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.

Sign up for GitHub

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

Jump to bottom

Split up animated_fox example #17191

Merged
merged 2 commits into from
Jan 6, 2025
Merged

Split up animated_fox example #17191

merged 2 commits into from
Jan 6, 2025

Conversation

rparrett
Copy link
Contributor

@rparrett rparrett commented Jan 6, 2025 •
edited
Loading

Objective

Our animated_fox example used to be a bare-bones example of how to spawn an animated gltf and play a single animation.

I think that's a valuable example, and the current animated_fox example is doing way too much. Users who are trying to understand how our animation system are presented with an enormous amount of information that may not be immediately relevant.

Over the past few releases, I've been migrating a simple app of mine where the only animation I need is a single gltf that starts playing a single animation when it is loaded. It has been a slight struggle to wade through changes to the animation system to figure out the minimal amount of things required to accomplish this.

Somewhat motivated by this recent reddit thread where Bevy and animation got a mention.

Solution

  • Split animated_fox into three separate examples
    • animated_fox - Loads and immediately plays a single animation
    • animated_fox_control - Shows how to control animations
    • animated_fox_events - Shows fancy particles when the fox's feet hit the ground
  • Some minor drive-by tidying of these examples

I have created this PR after playing around with the idea and liking how it turned out, but the duplication isn't totally ideal and there's some slight overlap with other examples and inconsistencies:

  • animation_events is simplified and not specific to "loaded animated scenes" and seems valuable on its own
  • animation_graph also uses a fox

I am happy to close this if there's no consensus that it's a good idea / step forward for these examples.

Testing

cargo run --example animated_fox
cargo run --example animated_fox_control
cargo run --example animated_fox_events

@alice-i-cecile alice-i-cecile added this to the 0.16 milestone Jan 6, 2025
alice-i-cecile
alice-i-cecile approved these changes Jan 6, 2025
View reviewed changes
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

I like the split a lot, but I think we should rename these examples to not refer to foxes. That's an implementation detail that doesn't matter to learners :)

@alice-i-cecile alice-i-cecile added C-Examples An addition or correction to our examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it A-Animation Make things move and change over time labels Jan 6, 2025
@alice-i-cecile alice-i-cecile mentioned this pull request Jan 6, 2025
Closed
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jan 6, 2025
Merged via the queue into bevyengine:main with commit 6f68776 Jan 6, 2025
35 checks passed
greeble-dev added a commit to greeble-dev/bevy that referenced this pull request Jan 9, 2025
@greeble-dev
- Split the setup function in two for clarity.
01a212e 
  - One function does the animation setup that we're demonstrating.
  - The other function does the incidental things like spawning the environment.
- Changed the animation playing to use AnimationPlayer directly instead of creating AnimationTransitions.
  - This appears sufficient when only playing a single animation.
  - The animated_mesh_controls example will demonstrate multiple animations and AnimationTransitions.
- Changed the animation asset to be the run cycle.
  - The original animated_fox example used the run.
  - I think this got accidentally changed in bevyengine#17191, so reverting to the original is safest.
- Changed FOX_PATH to a more neutral ASSET_PATH.
- Removed a seemingly redundant AnimationGraphHandle component.
- Added and updated various comments.
greeble-dev added a commit to greeble-dev/bevy that referenced this pull request Jan 9, 2025
@greeble-dev
First pass at simplifying animated_mesh.rs.
eb8a95a 
- Split the setup function in two.
  - One function does the animation setup that we're demonstrating.
  - The other function does the incidental things like spawning the environment.
- Changed the animation playing to use AnimationPlayer directly instead of creating AnimationTransitions.
  - This appears sufficient when only playing a single animation.
  - The animated_mesh_controls example will demonstrate multiple animations and AnimationTransitions.
- Changed the animation asset to be the run cycle.
  - The original animated_fox example used the run.
  - I think this got accidentally changed in bevyengine#17191, so reverting to the original is safest.
- Changed FOX_PATH to a more neutral ASSET_PATH.
- Removed a seemingly redundant AnimationGraphHandle component.
- Added and updated various comments.
greeble-dev added a commit to greeble-dev/bevy that referenced this pull request Jan 9, 2025
@greeble-dev
First pass at simplifying animated_mesh.rs.
acb7dca 
- Split the setup function in two.
  - One function does the animation setup that we're demonstrating.
  - The other function does the incidental things like spawning the environment.
- Changed the animation playing to use AnimationPlayer directly instead of creating AnimationTransitions.
  - This appears sufficient when only playing a single animation.
  - The animated_mesh_controls example will demonstrate multiple animations and AnimationTransitions.
- Changed the animation asset to be the run cycle.
  - The original animated_fox example used the run.
  - I think this got accidentally changed in bevyengine#17191, so reverting to the original is safest.
- Changed FOX_PATH to a more neutral ASSET_PATH.
- Removed a seemingly redundant AnimationGraphHandle component.
- Added and updated various comments.
- Changed the example description to mention the fox.
  - This is a small hint that the example relates to character animation, as opposed to the more bare-bones examples.
greeble-dev added a commit to greeble-dev/bevy that referenced this pull request Jan 9, 2025
@greeble-dev
First pass at simplifying animated_mesh.rs.
ac3a923 
- Split the setup function in two.
  - One function does the animation setup that we're demonstrating.
  - The other function does the incidental things like spawning the environment.
- Changed the animation playing to use AnimationPlayer directly instead of creating AnimationTransitions.
  - This appears sufficient when only playing a single animation.
  - The animated_mesh_controls example will demonstrate multiple animations and AnimationTransitions.
- Changed the animation asset to be the run cycle.
  - The original animated_fox example used the run.
  - I think this got accidentally changed in bevyengine#17191, so reverting to the original is safest.
- Changed FOX_PATH to a more neutral ASSET_PATH.
- Removed a seemingly redundant AnimationGraphHandle component.
- Added and updated various comments.
- Changed the example description to mention the fox.
  - This is a small hint that the example relates to character animation, as opposed to the more bare-bones examples.
greeble-dev added a commit to greeble-dev/bevy that referenced this pull request Jan 9, 2025
@greeble-dev
First pass at simplifying animated_mesh.rs.
b7e8311 
- Split the setup function in two.
  - One function does the animation setup that we're demonstrating.
  - The other function does the incidental things like spawning the environment.
- Changed the animation playing to use AnimationPlayer directly instead of creating AnimationTransitions.
  - This appears sufficient when only playing a single animation.
  - The animated_mesh_controls example will demonstrate multiple animations and AnimationTransitions.
- Changed the animation asset to be the run cycle.
  - The original animated_fox example used the run.
  - I think this got accidentally changed in bevyengine#17191, so reverting to the original is safest.
- Changed FOX_PATH to a more neutral ASSET_PATH.
- Removed a seemingly redundant AnimationGraphHandle component.
- Added and updated various comments.
- Changed the example description to mention the fox.
  - This is a small hint that the example relates to character animation, as opposed to the more bare-bones examples.
greeble-dev added a commit to greeble-dev/bevy that referenced this pull request Jan 11, 2025
@greeble-dev
- Changed the animation to be the run cycle.
43ebaf8 
  - The original animated_fox example used the run.
  - I think this got accidentally changed in bevyengine#17191, so reverting to the original is safest.
  - Note that we can improve this to select the animation by name if bevyengine#16529 lands.
@greeble-dev greeble-dev mentioned this pull request Jan 12, 2025
Merged
github-merge-queue bot pushed a commit that referenced this pull request Jan 14, 2025
@greeble-dev @mockersf
Improve the animated_mesh example (#17328)
c96949d 
# Objective

Building upon #17191, improve the
`animated_mesh` example by removing code, adding comments, and making
the example more c&p'able.

## Solution

- Split the setup function in two to clarify what the example is
demonstrating.
    - `setup_mesh_and_animation` is the demonstration.
    - `setup_camera_and_environment` just sets up the example app.
- Changed the animation playing to use `AnimationPlayer` directly
instead of creating `AnimationTransitions`.
    - This appears sufficient when only playing a single animation.
- Added a comment pointing users to an example of multiple animations.
- Changed the animation to be the run cycle.
- I think it got accidentally changed to the idle in
[#17191](#17191), so this is
reverting back to the original.
- Note that we can improve it to select the animation by name if
[#16529](#16529) lands.
- Renamed `FOX_PATH` to a more neutral `GLTF_PATH`.
- Updated the example descriptions to mention the fox.
- This adds a little character and hints that the example involves
character animation.
- Removed a seemingly redundant `AnimationGraphHandle` component.
- Removed an unnecessary `clone()`.
- Added various comments.

## Notes

- A draft of this PR was discussed on Discord:
https://discord.com/channels/691052431525675048/1326910663972618302/1326920498663133348
- There was discord discussion on whether a component is "inserted
onto", "inserted into" or "added to" an entity.
- "Added to" is most common in code and docs, and seems best to me. But
it awkwardly differs from the name of `EntityCommands::insert`.
    - This PR prefers "added to".
- I plan to follow up this PR with similar changes to the
`animated_mesh_control` and `animated_mesh_events` examples.
    - But I could roll them into this PR if requested.

## Testing

`cargo run --example animated_mesh`

---------

Co-authored-by: François Mockers <mockersf@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BenjaminBrienen BenjaminBrienen BenjaminBrienen approved these changes

@Carter0 Carter0 Carter0 approved these changes

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

Assignees
No one assigned
Labels
A-Animation Make things move and change over time C-Examples An addition or correction to our examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Milestone
0.16
Development

Successfully merging this pull request may close these issues.
4 participants
@rparrett @alice-i-cecile @Carter0 @BenjaminBrienen