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

Rojo drops Ref properties #142

Closed
LPGhatguy opened this issue Mar 28, 2019 · 1 comment · Fixed by #217
Closed

Rojo drops Ref properties #142

LPGhatguy opened this issue Mar 28, 2019 · 1 comment · Fixed by #217
Labels
impact: medium Moderate issue for Rojo users or a large issue with a reasonable workaround. scope: cli Relevant to the Rojo CLI size: medium type: bug Something happens that shouldn't happen

Comments

@LPGhatguy
Copy link
Contributor

The instance snapshot system that Rojo uses to avoid bugs with incremental updates doesn't support the concept of instance IDs. When the snapshots are reified as real instances (and eventually written to disk) they're given IDs, but any Ref properties are left untouched.

This is easy to demonstrate. Make a file that has a ref property, like an ObjectValue whose Value property is set to itself or a Model with a PrimaryPart set: https://github.com/LPGhatguy/rbx-dom/blob/3ab3f2dfd0f6753e2c5d7e25482e56cac468a68b/rbx_xml/test-files/part-referent.rbxmx

If you create a minimal project that reads in this model, then build that project with Rojo master to a new rbxmx model file, you'll notice that the Ref value ends up pointing to a referent that isn't present in the file!

This problem seems fairly simple to fix: keep a map from the old IDs to new IDs and rewrite Ref properties as we find them. This should work well as long as we make sure that every time we deserialize a model file that it has unique IDs, or else we'll run into collisions if paths alias.

@LPGhatguy LPGhatguy added type: bug Something happens that shouldn't happen scope: cli Relevant to the Rojo CLI size: medium impact: medium Moderate issue for Rojo users or a large issue with a reasonable workaround. labels Mar 28, 2019
@LPGhatguy LPGhatguy changed the title Rojo drops Ref properties Rojo snapshots drop Ref properties Mar 28, 2019
@LPGhatguy LPGhatguy added this to the 0.5.0 blockers milestone May 31, 2019
@LPGhatguy LPGhatguy changed the title Rojo snapshots drop Ref properties Rojo drops Ref properties Jun 17, 2019
@LPGhatguy LPGhatguy removed this from the 0.5.0 blockers milestone Aug 23, 2019
@LPGhatguy
Copy link
Contributor Author

This should be fixed as part of #217, as 0.6.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact: medium Moderate issue for Rojo users or a large issue with a reasonable workaround. scope: cli Relevant to the Rojo CLI size: medium type: bug Something happens that shouldn't happen
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant