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 does not build model pivots correctly when no PrimaryPart is set #628

Closed
ThoughtSpinnr opened this issue Aug 16, 2022 · 6 comments
Closed
Assignees
Labels
impact: medium Moderate issue for Rojo users or a large issue with a reasonable workaround. scope: cli Relevant to the Rojo CLI size: small status: external An external project is the cause of this type: bug Something happens that shouldn't happen

Comments

@ThoughtSpinnr
Copy link

When attempting to build a project file that includes a model saved as a .rbxm with:

  1. A pivot point offset from its center
  2. No PrimaryPart set

Rojo will not set the pivot point of the model correctly in the resulting .rbxl

Below is a zip file containing a .rbxm which contains 2 identical models that demonstrate the issue. One has a PrimaryPart set and one does not. Inserting this file into studio directly, you'll see 2 identical models. Including it in a rojo build, you'll see 2 identical models with 2 different pivots.

FloatingRock.zip

@LPGhatguy LPGhatguy added the type: bug Something happens that shouldn't happen label Sep 6, 2022
@LPGhatguy
Copy link
Contributor

Thanks for the report!

I think this issue comes down to Rojo not supporting pivots as well as it should. It should be just a matter of figuring out how they fit into the reflection database. We've been putting this off for a bit because the feature was unstable for a long time, but it sounds like we should prioritize it now.

@kennethloeffler kennethloeffler added scope: cli Relevant to the Rojo CLI size: small impact: medium Moderate issue for Rojo users or a large issue with a reasonable workaround. status: external An external project is the cause of this status: in-progress Someone is working on this last time we checked. labels Feb 15, 2024
@kennethloeffler kennethloeffler self-assigned this Feb 15, 2024
@kennethloeffler
Copy link
Member

@Dekkonot would you be okay if we just hack a Model.NeedsPivotMigration insertion into RojoTree::insert_instance until we address rojo-rbx/rbx-dom#385?

@Dekkonot
Copy link
Member

Dekkonot commented Feb 15, 2024

I would be more than okay with that for Rojo 7.4.1. It's important to get it working and we'll have it fixed properly soon enough anyway.

@kennethloeffler
Copy link
Member

I would be more than okay with that for Rojo 7.4.1. It's important to get it working and we'll have it fixed properly soon enough anyway.

Should we just put this change on the 7.4.x branch or do we want it on master too? I'd like to square away the rbx-dom issue before 7.5, so I think it should just be on 7.4.x

@Dekkonot
Copy link
Member

Just 7.4.x seems fine! Makes it less pressing to fix it too since it won't be code we'll potentially forget.

@kennethloeffler
Copy link
Member

This is closed by #865 (I properly linked this issue with the correct keyword in the PR, what are you doing github)

@kennethloeffler kennethloeffler removed the status: in-progress Someone is working on this last time we checked. label Feb 20, 2024
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: small status: external An external project is the cause of this type: bug Something happens that shouldn't happen
Projects
None yet
Development

No branches or pull requests

4 participants