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

Feature request: Allow texture indexing for TextureAtlases built through TextureAtlasBuilder #112

Closed
Seldom-SE opened this issue Nov 4, 2021 · 8 comments
Labels
enhancement New feature or request

Comments

@Seldom-SE
Copy link

Unless I am mistaken, when using a TextureAtlas built with a TextureAtlasBuilder as a tilemap's texture atlas, the Tiles' texture_indexes do not map to the TextureAtlasSprites' indexes of the same texture. This means that attempting to use indexes retrieved through texture_atlas.get_texture_index in a Tile results in unpredictable textures being displayed. And since the order of the sprites in a generated TextureAtlas cannot be known until runtime, using TextureAtlasBuilder with bevy_ecs_tilemap is infeasible. I wrote a simple example to demonstrate this issue: https://github.com/Seldom-SE/bevy_ecs_tilemap-TextureAtlas-demo.

Perhaps a method may be added to LayerBuilder to provide a Handle<TextureAtlas>, so the TextureAtlas's indexing may be used instead. Unfortunately, this could conflict with the material_handle passed into map_query.build_layer.

I am willing to contribute work on this, if it is desired. Thanks!

@MrGVSV
Copy link
Contributor

MrGVSV commented Nov 17, 2021

I actually just made something in my fork of this repo that might help. It adds a TileAtlasBuilder which can be used to generate a TextureAtlas that maintains order of insertion, so it should work with the current system.

If @StarArawn wants something like this merged, I can open a PR. Otherwise, feel free to just copy the TileAtlasBuilder code! I actually published this in its own crate for possibly broader usage and since it might only be tangentially related to this crate's purpose.

@Seldom-SE
Copy link
Author

This looks like it could help, thanks! I might use this once I inevitably refactor my level code again.

@MrGVSV
Copy link
Contributor

MrGVSV commented Nov 26, 2021

This looks like it could help, thanks! I might use this once I inevitably refactor my level code again.

@Seldom-SE I actually just published this in its own crate so it can be used in a more general sense (not just for bevy_ecs_tilemap). Feel free to check it out here!

@Seldom-SE
Copy link
Author

This is very helpful, thanks! I'm partway transitioned to using it now.

I'll leave this issue open, though, in case they want to come to their own solution.

@StarArawn
Copy link
Owner

The plan currently is to switch to a texture array. When I do that I can leverage mipmaps(when bevy comes up with a solution for these) and texture filtering. Things that you cannot easily use with an atlas.

@StarArawn StarArawn added the enhancement New feature or request label Dec 23, 2021
@StarArawn
Copy link
Owner

I'm going to close this as there is now a TileAtlasBuilder! 🎉

@MrGVSV
Copy link
Contributor

MrGVSV commented Dec 30, 2021

I'm going to close this as there is now a TileAtlasBuilder! 🎉

Wow I just realized that it was sneaked in via the rebase in #120. Sorry if you didn't actually want it, had no idea it did that 😅

@StarArawn
Copy link
Owner

I'm going to close this as there is now a TileAtlasBuilder! 🎉

Wow I just realized that it was sneaked in via the rebase in #120. Sorry if you didn't actually want it, had no idea it did that 😅

It's all good I think its still useful as I'm leaning towards making the texture array stuff all internal and users just use atlases with the API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants