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

fix: flame_tiled - add centered anchor point for tile rotation #1570

Merged
merged 8 commits into from
Apr 28, 2022

Conversation

nathanaelneveux
Copy link
Contributor

@nathanaelneveux nathanaelneveux commented Apr 25, 2022

Description

Added a centered anchor point to SpriteBatch creation so that rotations won't move tiles into the wrong position.

Checklist

  • The title of my PR starts with a Conventional Commit prefix (fix:, feat:, docs: etc).
  • I have read the Contributor Guide and followed the process outlined for submitting PRs.
  • I have updated/added tests for ALL new/updated/fixed functionality.
  • I have updated/added relevant documentation in docs and added dartdoc comments with ///.
  • I have updated/added relevant examples in examples.

Breaking Change

  • Yes, this is a breaking change.
  • No, this is not a breaking change.

Related Issues

I'm assuming this fixes the errant behavior described at the top of the project readme? There wasn't a lot of detail. With this patch rotations work in my project.

@nathanaelneveux nathanaelneveux changed the title fix: add centered anchor point for tile rotation fix: flame_tiled - add centered anchor point for tile rotation Apr 25, 2022
@spydon
Copy link
Member

spydon commented Apr 25, 2022

Some tests are failing, due to the update. It looks like the code change is sound, but that the tests need to be updated.

@nathanaelneveux
Copy link
Contributor Author

nathanaelneveux commented Apr 27, 2022

I guess the only other question I have is whether this solves the issue described at the top of the README for this package and if I should remove that note with this PR?

Changing:

⚠️ Under the current sprite batch implementation, Flips are not working properly.
Any help is appreciated looking into this issue.

You might also experience extra lines while rendering due to a bug in Flutter,
see this issue.

to something like:

⚠️ Under the current sprite batch implementation you might experience extra
lines while rendering due to a bug in Flutter, see this issue.

@spydon
Copy link
Member

spydon commented Apr 27, 2022

I guess the only other question I have is whether this solves the issue described at the top of the README for this package and if I should remove that note with this PR?

Changing:

warning Under the current sprite batch implementation, Flips are not working properly.
Any help is appreciated looking into this issue.
You might also experience extra lines while rendering due to a bug in Flutter,
see this issue.

to something like:

warning Under the current sprite batch implementation you might experience extra
lines while rendering due to a bug in Flutter, see this issue.

Have you tried whether flips work? If they do agree the readme change sounds good.

@nathanaelneveux
Copy link
Contributor Author

Have you tried whether flips work? If they do agree the readme change sounds good.

It doesn't fix flips - just rotations. I had a map with random flips and rotations - the rotations stuck out as broken because they were drawn in the wrong location. I assumed that flips were broken similarly but a quick test with just flipped tiles shows that is a different problem.

@spydon
Copy link
Member

spydon commented Apr 28, 2022

Good to know, then we'll merge this and see if someone fixes flips in the future

@spydon spydon merged commit f64d526 into flame-engine:main Apr 28, 2022
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

3 participants