rbx-dom doesn't care about select properties important for correctness and least surprise about rbx-dom HOT 7 OPEN

The Terrain.ShorelinesUpgraded one is especially annoying (because of the popup). I think the defaults should be implemented in rbx-dom as well.

from rbx-dom.

A few more questions:

• Should rbx-dom populate all properties with their defaults, or only some?
• Should rbx_dom_weak populate properties?
  • This is a little hairy, because then rbx_dom_weak needs access to reflection
• Or should the rbx_xml and rbx_binary (de)serializers populate properties instead?
  • This would be more straightforward since rbx_xml and rbx_binary already have access to reflection

from rbx-dom.

I think populating all default properties is probably a bad idea since it would massively increase the memory consumption for some Instances (I'm thinking about Part, as an example).

Having the parsers do it is weird because it means that they become opinionated instead of neutral. I'm thinking we may want a method or two in rbx_dom_weak to apply the default properties of an Instance when given a reflection database. That means the DOM would only need rbx_reflection as a dependency, which we update far less frequently than we do rbx_reflection_database and doesn't complicate things too much.

from rbx-dom.

Having the parsers do it is weird because it means that they become opinionated instead of neutral. I'm thinking we may want a method or two in rbx_dom_weak to apply the default properties of an Instance when given a reflection database. That means the DOM would only need rbx_reflection as a dependency, which we update far less frequently than we do rbx_reflection_database and doesn't complicate things too much.

This sounds reasonable and would work alright, but I remain concerned about correctness, and making the pit of success as wide and easy to fall into as possible. Model.NeedsPivotMigration shows that some properties must be present, or else Roblox won't load the file properly. It's probably not the only property like this. The fact that rbx-dom's implementation can leave out properties is a divergence from Roblox. Populating all properties would not just be our opinion - it's Roblox's standard. Also, I think it's just a little strange for consumers to have to call a special method for everything to work, just as it's strange to have to insert obscure properties.

I think populating all default properties is probably a bad idea since it would massively increase the memory consumption for some Instances (I'm thinking about Part, as an example).

Considering that rbx-dom is predominantly used to read files saved by Roblox Studio - which does include all the properties - I'm not sure if this is an actual problem. As far as I know, it is not possible to get Roblox to save files that have missing properties, and you must use a tool like Rojo or Lune (or rbx-dom manually, of course). I guess we can measure the real impact, combine this with the previous observation, and go from there?

from rbx-dom.

I've ran into an instance while implementing Syncback for Rojo where information on 'correctness' would be useful. I'm curious what your thoughts would be on including properties like MeshPart.InitialSize and Model.ScaleFactor which are important to the actual function of these Instances?

For syncback, I'm stripping out as many properties as possible to ease diffing two DOMs, but there's no easy metric for this. Right now I'm just using "scriptable" and "not scriptable" but I would like to differentiate between "important" and "not important" too since there are a fair few unscriptable properties that are critical to being able to build a place file correctly.

from rbx-dom.

from rbx-dom.

My concern with that strategy is that I imagine a lot of people will want to run syncback on their game and then just use the output for serving into Studio. If there's a bunch of properties that can't be scripted, it's a bad UX, especially for services like Lighting and Workspace, which have a fair few unscriptable properties. These are important if you're using the project for rojo build, but they aren't important if you're just syncing and having a bunch of sync errors obscures actual problems.

I'm also trying to avoid blacklisting or whitelisting properties because it isn't sustainable and feels out of place in Rojo. I'd ideally like rbx-dom to track the information I need for this implementation instead, even if that information is written manually.

from rbx-dom.