Skip to content

Patch ScaleFactor to Scale in reflection database #252

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

Merged
merged 2 commits into from
Mar 9, 2023

Conversation

Dekkonot
Copy link
Member

@Dekkonot Dekkonot commented Mar 8, 2023

At the moment, we do not serialize ScaleFactor. This is mostly fine, except Roblox has begun emitting models with ScaleFactor so Rojo occasionally merges them when building models. This has the result of us producing files with ScaleFactor set to 0, which is not good.

This is not critical right now because model scaling is in beta and most people don't have it enabled. It might become critical in the future though. So, this PR patches it so we won't have that problem.

I also added Scale as an alias for it because that's what it's exposed to users as in Studio and it's more intuitive than ScaleFactor.


This is my first time adding a patch to the database and I probably made a mistake, but I did my best. Please let me know if anything needs to change.

* Adds ScaleFactor via reflection patch
* Marks Scale as alias for ScaleFactor
* Add custom setter and no getter for Scale

Signed-off-by: Dekkonot <[email protected]>
@nezuo
Copy link
Contributor

nezuo commented Mar 8, 2023

I think you need to set Scale's Scriptability to Custom

Copy link
Contributor

@LPGhatguy LPGhatguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, let's get this in before it becomes a problem.

@LPGhatguy LPGhatguy merged commit ce4c5bf into rojo-rbx:master Mar 9, 2023
@Dekkonot Dekkonot deleted the scalefactor-patch branch July 23, 2024 17:05
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.

4 participants