How to execute logic when the animation reaches a specific frame?

[XBagon]

I'm currently trying to trigger some logic at a specific frame of the animation, but it seems SpriteSheetAnimationState::current_frame isn't public, is there are reason for that?
Also I think a more powerful system which automatically sends events for tagged frames (which you would tag when constructing the animation), would be very helpful for many cases.

[jcornaz]

it seems SpriteSheetAnimationState::current_frame isn't public, is there are reason for that?

Yes, and it should stay private so that we can change internal representation without breaking the API.

But eventually, we may expose a getter.

Also I think a more powerful system which automatically sends events for tagged frames (which you would tag when constructing the animation), would be very helpful for many cases.

Interesting, that sounds like a good idea. Let's explore how the API could look like. I will think about it on my side, feel free to share your ideas about how you would like to use it ;-)

[XBagon]

Yes, and it should stay private so that we can change internal representation without breaking the API.

I agree, this makes a lot of sense for a crate >1.0. Sadly it's really hard to use the crate, when missing features can only be achieved by hacks or forking it. Generally exposing these kind of fields seems like a good idea to me.

Let's explore how the API could look like.

I probably would just add something like

SpriteSheetAnimation::<Event>::with(frame_tag_map: BTreeMap<usize, Event>)

which sends the according event when "starting" a frame.

[jcornaz]

Maybe worth noting: The sprite sheet frame is publicly accessible. So it is already possible to observe changes of the sprite-sheet itself instead of observing changes of the animation state.

[jcornaz]

I probably would just add something like

SpriteSheetAnimation::<Event>::with(frame_tag_map: BTreeMap<usize, Event>)

which sends the according event when "starting" a frame.

It looks like bevy assets cannot be generic. So that wouldn't work, sadly :-/

[XBagon]

For example, we may provide a getter that gives the user access to the data they need without exposing the internal representation.

Sorry by exposing I meant in any way, not necessarily by making it public, which we agree is not a good option here. And I think being able to set the frame might also be important for some things?

[XBagon]

What about something like this:

sure the BTreeMap is internal representation, I have nothing against a nicer API :).

It looks like bevy assets cannot be generic. So that wouldn't work, sadly :-/

Oh I didn't know that. That's a bummer. I guess we would need to work with Box<dyn Any> then?

[jcornaz]

I guess we would need to work with Box then?

I am not yet sure how to approach the implementation of dispatching the event if the assets contain a Box<dyn Any>. If you have an idea feel free to share it.

A simple solution for now may be to add a fn current_frame_index(&self) -> usize getter to the animation state.

[XBagon]

It looks like bevy assets cannot be generic. So that wouldn't work, sadly :-/

We should be able to implement TypeUuid ourselves. Also an PR is planned for adding generic support to the macro.

A simple solution for now may be to add a getter to the animation state.

I would do that regardless, but maybe directly also allow setting it?

[jcornaz]

We should be able to implement TypeUuid ourselves.

The trait definition is:

pub trait TypeUuid {
    const TYPE_UUID: Uuid;
}

So we have to return a constant uuid associated for the type. It's not very easy to implement, we probably have to do it via macro.

If you feel confident about a possible implementation, you may share code (either in this issue, as a conceptual implementation, or in the form of a pull request)

I would do that regardless, but maybe directly also allow setting it?

What would be the use-case for setting it? My approach is usually to wait for the need before adding a new public member (aka YAGNI). This is somehow an echo of what I was saying about breaking changes. The more public members, the higher chances are that I find myself in the difficult situation of having the break the API, therefore I make sure they actually bring value. For the getter, I am okay because the need arose (and is documented in the present issue), for the setter I would like to hear what's the use-case first.

[jcornaz]

For now, we can go with the current_frame_index getter (available in version 2.2.0). We can still open another issue if we want to continue designing an event-based solution.

[XBagon]

Great! Yes I would just wait for mentioned bevyengine/bevy/pull/4118 to hit stable and then we should be able to implement the discussed API easily.

My approach is usually to wait for the need before adding a new public member

I understand that idiom, I guess I'm just coming from a different perspective. I took part in the Bevy Jam 1 last week and just wanted to be able to do stuff rather than having to start (and keep) my own fork around which exposes the frame number. So that was just thinking into the future where a need for setting the frame number could arise. But for any other project this should be a totally fine and probably better idiom.

[XBagon]

bevyengine/bevy#4118 was merged and should be included in the now released bevy 0.8.
So we could try this again :P