Defining the Archivable property on an instance causes binary serializer to write false for Archivable properties on all other instances of the same class about rbx-dom HOT 4 OPEN

We probably don't want to serialize Archivable to begin with, but I don't want to mark properties unnecessarily as non-serializable, so 2 is the best option IMO.

I think the idea of Roblox deserializing Instance.archivable is really funny though. Under what circumstances would that even come up, ha ha?

from rbx-dom.

After giving this a shot, I've ran into a minor hiccup: the way we handle default properties is per-class right now. This means that in order to override a default for an abstract class like Instance, we'd need to override it for every class that inherits from it. That's a much bigger change than I was hoping for due to the nature of Instance.

I don't really have any good ideas to fix this. I have no problem cascading patches down to every inherited class, but it would mean we'd need to be careful with new patches to Instance's defaults since every one would result in a rather large addition to the database.

from rbx-dom.

It is a little weird how we that information duplicated... but maybe instead of adding the default value patches to all subclasses, we could change the way rbx_binary's serializer finds default properties? e.g., replace this:

with a call to a function like:

    fn find_default_value(
        database: &'db ReflectionDatabase,
        mut class: &'db ClassDescriptor<'db>,
        canonical_property_name: &str,
    ) -> Option<&'db Variant> {
        loop {
            match class.default_properties.get(canonical_property_name) {
                None => {
                    class = database
                        .classes
                        .get(class.superclass.as_deref()?)
                        .expect("superclass that is Some should exist in reflection database")
                }
                default_value => return default_value,
            }
        }
    }

We wouldn't have to move any existing defaults if we did this (although it's probably a good idea to deduplicate all the defaults in the future), but it would work with the obvious implementation of property default value patches

from rbx-dom.

Downside: more overhead for inherited properties (especially if we eventually do move default property values into their respective classes), since this does multiple hashmap lookups for each inherited property

from rbx-dom.