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

Examples cleanup #356

Merged
merged 15 commits into from
Dec 18, 2022
Merged

Examples cleanup #356

merged 15 commits into from
Dec 18, 2022

Conversation

rparrett
Copy link
Collaborator

@rparrett rparrett commented Nov 24, 2022

I feel like there is a lot left to do here, but this branch is getting large and I'm running out of time.

I tested all examples and their keyboard functionality. texture_vec texture_container were already broken and the problem wasn't obvious so I didn't touch them.

My main goal was to center a bunch of tilemaps and improve consistency of tilemap spawning code between examples, but I stumbled on a few other little cleanups.

I'd recommend reviewing individual commits.

Copy link
Collaborator

@bzm3r bzm3r left a comment

Choose a reason for hiding this comment

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

This is a welcome standardization fix. Thanks for taking the time to do it.

My main question is around the removal of map_transform in the multiplication with the tile center's transform. Can you comment?

examples/hex_neighbors.rs Outdated Show resolved Hide resolved
examples/hex_neighbors.rs Outdated Show resolved Hide resolved
examples/hex_neighbors.rs Show resolved Hide resolved
examples/mouse_to_tile.rs Show resolved Hide resolved
examples/mouse_to_tile.rs Outdated Show resolved Hide resolved
@rparrett
Copy link
Collaborator Author

rparrett commented Nov 28, 2022

Note: there is currently (perhaps a) system ordering issue in hexagon_generation. On some runs, switching map types results in empty tiles in the maps. On other runs, it works fine. This behavior was also present prior to this PR, but I'm looking into it.

edit: documented that here: #362

@StarArawn
Copy link
Owner

@rparrett Is this ready for review again? Also the conflicts need to be fixed.

@rparrett
Copy link
Collaborator Author

rparrett commented Dec 11, 2022

Rebased. Looks like there's consensus on removing tile labels from bevy's hierarchy. I should be able to get that done later today.

@bzm3r bzm3r merged commit 56e48a2 into StarArawn:main Dec 18, 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.

3 participants