Skip to content

make bevy_mesh optional for bevy_animate #20742

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
Merged
merged 1 commit into from
Oct 6, 2025
Merged

make bevy_mesh optional for bevy_animate #20742

merged 1 commit into from
Oct 6, 2025

Conversation

atlv24
Copy link
Contributor

@atlv24 atlv24 commented Aug 25, 2025

Objective

Solution

  • copy paste the morph weights stuff out into a module, gate it, then add the optional dep

@atlv24 atlv24 mentioned this pull request Aug 25, 2025
Closed
@alice-i-cecile alice-i-cecile added C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Animation Make things move and change over time M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Aug 25, 2025
@atlv24 atlv24 force-pushed the ad/meshless-animate branch from e26c392 to 4ceea29 Compare August 28, 2025 02:08
@greeble-dev greeble-dev added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Aug 31, 2025
@greeble-dev
Copy link
Contributor

I'm not sure M-Needs-Migration-Guide applies - change is backwards compatible with use bevy::animation::animation_curves::WeightsCurve and preludes.

@atlv24
Copy link
Contributor Author

atlv24 commented Sep 1, 2025

Migration guide is indeed not needed here.

@mockersf
Copy link
Member

mockersf commented Sep 1, 2025

Shouldn't there be a morph_weights feature that enables bevy_mesh? It feels a bit backward to do it the other way.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Sep 1, 2025
@alice-i-cecile alice-i-cecile removed this pull request from the merge queue due to a manual request Sep 1, 2025
@alice-i-cecile alice-i-cecile added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it labels Sep 1, 2025
@alice-i-cecile
Copy link
Member

Shouldn't there be a morph_weights feature that enables bevy_mesh? It feels a bit backward to do it the other way.

This feels clearer to me too.

@atlv24
Copy link
Contributor Author

atlv24 commented Sep 2, 2025

I think that's an orthogonal request: bevy_mesh does not have a morph_weights feature. If it did, it would make sense to gate this on bevy_mesh/morph_weights, but right now this is about bevy_animation supporting bevy_mesh. The specific part of bevy_mesh it supports is morph weights, but this could just as easily be gating skinning code or other things.

@atlv24
Copy link
Contributor Author

atlv24 commented Sep 2, 2025

I'd be in favor of adding a morph_weights feature to bevy_mesh which gates it there and also in bevy_animate, anyhow. I just dont think its relevant to this PR

@atlv24 atlv24 mentioned this pull request Sep 4, 2025
Open
@atlv24 atlv24 mentioned this pull request Oct 5, 2025
Merged
@atlv24 atlv24 force-pushed the ad/meshless-animate branch from 4ceea29 to f4cb356 Compare October 5, 2025 01:01
@atlv24 atlv24 added S-Blocked This cannot move forward until something else changes and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Oct 5, 2025
@atlv24
Copy link
Contributor Author

atlv24 commented Oct 5, 2025

Blocked on #21389

github-merge-queue bot pushed a commit that referenced this pull request Oct 6, 2025
@atlv24
bevy_mesh optional morph (#21389)
c233772 
# Objective

- make morph support optional on bevy_mesh
- unblock #20742
- another step towards #20867

## Solution

- feature

## Testing

- morph targets example works
@atlv24 atlv24 force-pushed the ad/meshless-animate branch from f4cb356 to 951a7af Compare October 6, 2025 20:33
@atlv24 atlv24 removed the S-Blocked This cannot move forward until something else changes label Oct 6, 2025
@atlv24 atlv24 added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Oct 6, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Oct 6, 2025
Merged via the queue into bevyengine:main with commit b7a90ae Oct 6, 2025
42 checks passed
tigregalis pushed a commit to tigregalis/bevy that referenced this pull request Oct 7, 2025
@atlv24 @tigregalis
bevy_mesh optional morph (bevyengine#21389)
6be49fd 
# Objective

- make morph support optional on bevy_mesh
- unblock bevyengine#20742
- another step towards bevyengine#20867

## Solution

- feature

## Testing

- morph targets example works
tigregalis pushed a commit to tigregalis/bevy that referenced this pull request Oct 7, 2025
@atlv24 @tigregalis
make bevy_mesh optional for bevy_animate (bevyengine#20742)
8c418da 
# Objective

- make bevy_mesh optional for bevy_animate as mentioned in
bevyengine#20714 (comment)

## Solution

- copy paste the morph weights stuff out into a module, gate it, then
add the optional dep
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@tychedelia tychedelia tychedelia approved these changes

@greeble-dev greeble-dev greeble-dev approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A-Animation Make things move and change over time C-Usability A targeted quality-of-life change that makes Bevy easier to use M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide 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

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@atlv24 @greeble-dev @mockersf @alice-i-cecile @tychedelia