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

Loaded entity position ignores pivot #290

Open
Koopa1018 opened this issue Jan 12, 2024 · 2 comments · May be fixed by #294
Open

Loaded entity position ignores pivot #290

Koopa1018 opened this issue Jan 12, 2024 · 2 comments · May be fixed by #294

Comments

@Koopa1018
Copy link

In my LDtk project, I have defined a number of entities with a bottom pivot point:
image

Based on my work in other apps, I expect the pivot to change how entities are placed relative to the position shown--and indeed, in LDtk I see this exact thing happening.
image
Left: origin on the bottom. Right: origin in center, moved to fill the same space. Note the changed Y coordinate.

But when those entities are loaded into Bevy:
image
What the pfargtl?! The Transform component stores the object's center instead of its origin!

I've compensated in my code. But it's a huge bother to have to do complicated math instead of just reading a component which, frankly, ought to match LDtk's behavior in the first place (apart from Y-down coordinates).

@Koopa1018 Koopa1018 changed the title Loaded entity translation does not respect origin Loaded entity position ignores pivot Jan 12, 2024
@Trouv
Copy link
Owner

Trouv commented Jan 13, 2024

Yep, I agree this is bad behavior. It's a holdover from when this plugin was originally written, which was before the Anchor in bevy sprites existed. Sprites were just anchored to their center, and so to make spawned entities w/ sprites have the same visual location in LDtk, the transform was adjusted instead. Now, it's well past time we switch to using the Anchor to pivot entities instead of the transform.

Koopa1018 added a commit to Koopa1018/bevy_ecs_ldtk that referenced this issue Jan 16, 2024
This is the basic ingredient needed to fix Trouv#290. There's a lot of things which will potentially break if this is done in isolation, so there's a lot more work ahead.
@Koopa1018 Koopa1018 linked a pull request Jan 16, 2024 that will close this issue
@Koopa1018
Copy link
Author

Ahh, historical workarounds....that makes sense 🙂

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 a pull request may close this issue.

2 participants