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

AnimatedTile add/remove bug #507

Open
richchurcher opened this issue Jan 21, 2024 · 2 comments
Open

AnimatedTile add/remove bug #507

richchurcher opened this issue Jan 21, 2024 · 2 comments

Comments

@richchurcher
Copy link

richchurcher commented Jan 21, 2024

[dependencies]
bevy = { version = "0.12.1", features = ["wayland"] }
bevy_ecs_tilemap = { git = "https://github.com/StarArawn/bevy_ecs_tilemap", branch = "main" }

Hello! Enjoying using bevy_ecs_tilemap for a game jam, and thanks so much for your hard work.

I think I may have found a related bug to #474. I've been adding and removing AnimatedTile to support various action animations on a tile. (It's possible this isn't great practice, it's "jamcode" so we'll leave efficiency aside for now!) However, I notice that some of the time the animation doesn't stop when the component is removed.

I'd be happy to submit a PR but I'm a little new to the library, and not completely certain what's causing this inconsistent behaviour. I note that the workaround described in #473 solves the issue:

                      commands
                          .entity(character_entity)
                          .remove::<AnimatedTile>()
                          .insert(SeekingStorage { tile_pos: None });
                      if let Ok((_, mut color)) =
                          character_animation_q.get_mut(character_entity)
                      {
                          color.set_changed();
                      }

So presumably removing AnimatedTile just needs to trigger changed state? If you can point me in the right direction, I can have a crack at fixing it.

@richchurcher richchurcher changed the title AnimatedTile add/remove AnimatedTile add/remove bug Jan 21, 2024
@rparrett
Copy link
Collaborator

Thanks for the report. Glad that workaround worked for you.

So presumably removing AnimatedTile just needs to trigger changed state? If you can point me in the right direction, I can have a crack at fixing it.

Sounds about right to me. I think this might be slightly more involved than the previous fix. It has been a while since I've looked at this code and I'm 100% sure this would work in an "extract system" but at a glance, I would try:

  • Splitting changed_tiles_query into
    • A query that just grabs the Entity with the same big "or changed" filter.
    • A separate query that actually retrieves data
  • Adding an Extract<RemovedComponents<AnimatedTile>> system param
  • Use iter_many to grab data tile data for entities that were either changed, or had that component removed.

@richchurcher
Copy link
Author

Thanks! I'll take a look after recovering from the jam 😆

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

No branches or pull requests

2 participants