Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start tracking 'canonical properties' #28

Merged
merged 56 commits into from
Apr 19, 2019
Merged

Conversation

LPGhatguy
Copy link
Member

This PR is a robust solution for #18, #14, and is a step towards #16.

This change introduces the idea of a canonical property more concretely. This idea helps us map from serialization-specific names onto common names that projects can agree on for referring to the same idea.

The concept for property canonicalization came out of Rojo, and seeks to handle these cases:

  1. Some properties have deprecated aliases, or aliases used only for serialization, like Fire.Size
  2. Some properties are scripted through special getters/setters, like CollectionService tags
  3. Some properties are not scriptable, but specified in the disk formats, like Lighting.Technology

A new file was added to generate_reflection called canonical-properties.toml that specifies these relationships. Previously, these ideas were present in rbx_xml's ad hoc reflection database, but they're needed for rbx_binary as well.

There are a couple of assumptions that this approach makes:

  1. Most properties in the API dump are scriptable and have the same name and type when serialized
  2. Most properties that don't follow (1) are just aliases of other properties

While I'm sure about (1), there are non-trivial cases where (2) doesn't hold up. The most notable example is part color, which used to be BrickColor, but is now Color3uint8, which requires a name and type conversion to Color3 before it can be reflected to Lua.

@LPGhatguy LPGhatguy marked this pull request as ready for review April 17, 2019 21:55
@LPGhatguy
Copy link
Member Author

It's mostly ready, I think!

@LPGhatguy LPGhatguy merged commit b05e16e into master Apr 19, 2019
@LPGhatguy LPGhatguy deleted the canonical-properties branch April 19, 2019 00:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant