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

Tracking Issue: MVP Animation #4026

Open
9 of 14 tasks
james7132 opened this issue Feb 23, 2022 · 7 comments
Open
9 of 14 tasks

Tracking Issue: MVP Animation #4026

james7132 opened this issue Feb 23, 2022 · 7 comments
Labels
A-Animation Make things move and change over time A-Meta About the project itself C-Tracking-Issue An issue that collects information about a broad development initiative

Comments

@james7132
Copy link
Member

james7132 commented Feb 23, 2022

This issue is meant to be a more actionable/scoped version of #91 and #95, specifically scoped to the goal of providing a minimum low level API for managing asset based animations in Bevy.

RFCs

The overall design for these systems are detailed in the following RFCs. Later parts of the implementation are blocked on the design feedback on these individual RFCs.

Implementation/Merge Strategy

This is a fairly large area that touches multiple core areas in the engine. Even with a complete prototype, introducing the system into Bevy in one large PR is likely difficult or impossible (see #1429). The following individual segments will likely need to be individually addressed to piecemeal introduce parts of the system into Bevy proper.

@james7132 james7132 added A-Meta About the project itself A-Animation Make things move and change over time labels Feb 23, 2022
@cart
Copy link
Member

cart commented Feb 23, 2022

A full property based animation system is largely enabled by the designs in the RFCs listed; however, at the time or writing, there currently exists no way to get references to components that implement Reflect via the component name. This blocks the ability to apply sampled and blended values to arbitrary component properties.

This is already possible. We use this functionality for our scene system. TypeRegistry::get_with_name gets you the type registration, which can then be used to get the ReflectComponent impl, which will return you an Option<&dyn Reflect> (or mutable variant) for a given entity.

This was referenced Feb 23, 2022
@james7132
Copy link
Member Author

@cart: updated the main issue to reflect (haha) this. I'll need to dive deeper into the actual reflection APIs to devise an implementation strategy though.

@Type1J
Copy link

Type1J commented Feb 24, 2022

This is a very welcomed addition!

@Barugon
Copy link

Barugon commented Mar 14, 2022

What about animation blending (like Unity's Mecanim)?

@james7132
Copy link
Member Author

What about animation blending (like Unity's Mecanim)?

You probably won't see a full animation controller like API anytime soon. However, the proposed animation graph API is low-level enough like Unity's Playables API, which supports arbitrary blending control, and you could potentially build a full state machine driven animation system on top of it.

@SamPruden
Copy link

Is the eventual intention to allow designers (i.e. people using the Bevy Editor, not coders) to apply arbitrary animations that drive arbitrary component values?

This is something that @james7132 brought up in the Schematics discussion #3877 (comment). I'm wary of that, for reasons that overlap with the arguments made in favour of Schematics.

Firstly, there's the standard issue of tying animation assets too closely to ECS implementation details, meaning that refactorings to the ECS layout would break compatibility with authored animation (and other) assets. This is the problem that Schematics aim to solve, and may be addressable by some design in which animations drive Schematic values instead of component values directly. There's a little discussion of that in #3877.

There's also an ECS philosophy question that I'd be interested to hear the official stance on:

Should ECS Systems be expected to gracefully handle all typesafe Component values, even those that don't make semantic sense? Or alternatively, is it acceptable to rely on engineering knowledge about what values get set on the Component fields by the code? For example, should Systems processing a Component holding a direction vector be expected to have well defined and sensible behaviour if that vector is not in fact normalized?1

On the one hand, in ECS it's hard to keep track of all of the places where a value is mutated, so one could argue that it's dangerous to make assumptions about data values. On the other hand, actually covering every corner case would have both engineering and runtime overhead. Inevitably, cases would be missed. In practice, I've never taken this rigorous approach.

The reason that this is relevant to animation is that if we allow arbitrary animations to make arbitrary changes to entity data, it becomes incredibly easy for these implicit invariants to be broken. The animations may be composed by people who are completely separate from the code, and who don't have any understanding of the restrictions and dangers. We don't want a scenario where somebody tries to animate a value and causes undefined behaviour because the value they applied was unexpected.

For these reasons, I think it's a bad idea for animations to directly touch ECS data without going through some validating mediation layer that only allows semantically valid animations. The ideas around Schematics may work for this, although I don't really know what that looks like yet.

What's Bevy's general philosophy to these problems around data validation and trust, and how that interacts with animations?

Footnotes

  1. This particular example might be a good candidate for newtype typesafety.

@james7132
Copy link
Member Author

The intent is to have it work on Component that implements Reflect. For the most part, this usually implies that what you're writing to is mostly just read-only data that has no invariant to enforce (i.e. materials).

@james7132 james7132 mentioned this issue Apr 28, 2022
50 tasks
@nicopap nicopap added the C-Tracking-Issue An issue that collects information about a broad development initiative label Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Animation Make things move and change over time A-Meta About the project itself C-Tracking-Issue An issue that collects information about a broad development initiative
Projects
None yet
Development

No branches or pull requests

6 participants