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

Better support fixed timesteps in newest Avian #61

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

janhohenheim
Copy link

@janhohenheim janhohenheim commented Sep 8, 2024

This primarily does the following things.

  • Update to breaking Avian changes
    • Notably, physics layers now require an explicit default instead of implicitly belonging to everything when not specified
  • Support Avian no longer running in PostUpdate, but in FixedUpdate.
    • The Tnua plugins for Avian no longer have a Default, as the TnuaControllerPlugin needs to be manually set in PhysicsSchedule as well
    • Change some scheduling stuff in examples in order to show idiomatic usage
  • Work with Position and Rotation instead of GlobalTransform to better support interpolation between fixed timesteps
    • GlobalTransform is in general just the place where the object in question will be rendered, not where it physically is.
  • Fix just_pressed in demos not working reliably with a fixed timestep
    • just_pressed in general does not work out of the box for fixed timesteps. LWIM would handle that, but I didn't want to add a new dependency. So, instead the demo now does its own just_pressed caching.
  • Make demos use PhysicsSchedule by default for avian.

I tested and verified that all examples and demos still work.
Some of these changes like the GlobalTransform stuff would be nice for rapier as well, but I'll leave that for a future follow-up PR.

@janhohenheim janhohenheim marked this pull request as ready for review September 8, 2024 06:00
Copy link
Owner

@idanarye idanarye left a comment

Choose a reason for hiding this comment

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

Wrote some comments. You don't have to fix them - I can fix them myself once I merge after Avian 0.2 is released.

avian2d/src/lib.rs Outdated Show resolved Hide resolved
@@ -132,6 +144,11 @@ fn update_proximity_sensors_system(
TnuaToggle::SenseOnly => {}
TnuaToggle::Enabled => {}
}
let transform = Transform {
Copy link
Owner

Choose a reason for hiding this comment

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

I think this transform is only used for computing the cast_origin. Maybe we can just compute it directly with the rotation and position instead of creating a transform matrix?

Copy link
Author

Choose a reason for hiding this comment

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

I don't know how we would do that. Creating the transform matrix is super cheap, so I also don't think this is pressing.

avian3d/src/lib.rs Outdated Show resolved Hide resolved
demos/src/app_setup_options.rs Outdated Show resolved Hide resolved
@janhohenheim
Copy link
Author

Good suggestions! Fixed the things that I knew how to fix :)

@janhohenheim
Copy link
Author

Looks like the CI ran into an infinite build. Could you rerun it if you happen to have time? :) Also, I'll try to keep this PR up to date so it's ready for Avian 0.2 when it releases.

@idanarye
Copy link
Owner

@janhohenheim I've updated the Avian integration crates to Avian 0.2, and since they've officially changed the default schedule I had do these changes on main - otherwise it'd mess up the demos.

I did not implement any of the other things you did in this PR - just the scheduling.

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

Successfully merging this pull request may close these issues.

2 participants