Skip to content

Use Position and Rotation instead of GlobalTransform (Based off PR #61) #81

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

Merged
merged 15 commits into from
Mar 11, 2025
Merged

Use Position and Rotation instead of GlobalTransform (Based off PR #61) #81

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
a0743b8
Remove default schedule
janhohenheim Sep 8, 2024
d890cd9
Use physical transform instead of rendered
janhohenheim Sep 8, 2024
31dafba
Patch to newest avian
janhohenheim Sep 8, 2024
6afb0a6
Add docs
janhohenheim Sep 8, 2024
2c955b5
Update 2D and examples
janhohenheim Sep 8, 2024
3bc32f8
Fix various fixed timestep issues
janhohenheim Sep 8, 2024
a10433c
Revert removal of running demos in physics schedule
janhohenheim Sep 8, 2024
5492484
Make examples a bit more idiomatic
janhohenheim Sep 8, 2024
0f28eb6
Use direct into impl
janhohenheim Sep 8, 2024
e8425b0
Remove useless line
janhohenheim Sep 8, 2024
e4d1d3b
Fix compile error
janhohenheim Sep 8, 2024
6099a73
Run cargo fmt
janhohenheim Sep 8, 2024
c4d7909
Merge branch 'main' into fix-minor-avian-stuff
janhohenheim Sep 19, 2024
009e3d1
fix demos
Alpyg Jan 16, 2025
152fce4
Use right schedule
Alpyg Jan 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 36 additions & 13 deletions avian2d/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,20 @@ use bevy_tnua_physics_integration_layer::*;
/// Add this plugin to use avian2d as a physics backend.
///
/// This plugin should be used in addition to `TnuaControllerPlugin`.
/// Note that you should make sure both of these plugins use the same schedule.
/// This should usually be `PhysicsSchedule`, which by default is `FixedUpdate`.
Copy link
Owner

@idanarye idanarye Jan 17, 2025

Choose a reason for hiding this comment

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

Isn't the PhysicsSchedule a schedule of its own, separate from the FixedUpdate?

Copy link
Contributor Author

@Alpyg Alpyg Jan 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure what @janhohenheim meant by PhysicsSchedule being FixedUpdate by default

Copy link
Owner

@idanarye idanarye Jan 19, 2025

Choose a reason for hiding this comment

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

Maybe that the default in Avian is FixedUpdate but usually you'd want to use PhysicsSchedule?

This is kind of important, both because the documentation needs to be correct and because it affects the ScheduleToUse::PhysicsSchedule option in the demos. I'll try to ping him on Discord.

Copy link
Contributor

@janhohenheim janhohenheim Jan 21, 2025

Choose a reason for hiding this comment

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

Hey hey everyone. I was not active much in the Bevy space for some months due to academia, so what I'm writing now might not be accurate. But as I remember it, I was looking at PhysicsSchedule as basically a generic label / wrapper / "variable" for the actual schedule. What the actual schedule was, I determined by looking at PhysicsSchedulePlugin .schedule which back then was FixedUpdate by default (now FixedPostUpdate).
It's very very possible that I misunderstood / misunderstand how the schedule hierarchy precisely works in Bevy, so if you think this sounds somewhat wrong, it probably is :D

Copy link
Owner

@idanarye idanarye Jan 21, 2025

Choose a reason for hiding this comment

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

That's weird. I thought the point of the PhysicsSchedule was supposed to run in its own frequency, which means its runner was supposed to run it multiple times, or not at all, depending on how much time has passed since the last time it was ran. In other words - even if it runs in PostUpdate the PhysicsSchedule itself still gets fixed frame time.

But not it looks like PhysicsSchedule is mostly about not running it if Avian is paused and about replacing the Time resource while it runs?

Copy link

@Jondolf Jondolf Jan 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@idanarye The point of the PhysicsSchedule is just to advance the whole physics simulation by one step, using the delta time stored in the Time resource. The schedule is independent of how often and in what way it is run; you could run it with a fixed time step, a variable time step, or even run it manually for e.g. networking purposes.

Avian 0.2 did indeed change how the default scheduling is set up. Previously, Avian managed its own fixed time step in PostUpdate, but this had unnecessary complexity, footguns (ex: duplicate fixed time steps when running physics in FixedUpdate, and having to use Time::fixed_once_hz), several bugs, and it was overall strange to emulate a custom fixed time step when Bevy already has a first-party one. Running physics after Update was also kind of strange; both Unity and Godot run it before, near the end of FixedUpdate/_physics_process.

With the new scheduling, the PhysicsSchedule is run in FixedPostUpdate by default, making FixedUpdate the natural place for most user gameplay logic and movement systems, except of course for things like camera movement or interpolation which should typically run every frame for smooth and responsive results.

You can still run physics in e.g. PostUpdate too, but it will use a variable time step with Time<Virtual> since that is the active time resource there. And of course custom scheduling is possible too for additional control.

The new scheduling and rationale is covered in all of these:

Copy link
Owner

@idanarye idanarye Jan 21, 2025

Choose a reason for hiding this comment

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

My scheduling goals are:

  • I want Tnua's internal systems to get the correct frame duration - that is, the same one Avian uses - because it's crucial for their calculations.
  • I prefer Tnua's internal systems to run in lockstep with Avian's physics simulation, so that they can respond to the position and velocity changes Avian make.
  • User control systems don't need to use Avian's frame duration - but they do need to run in lockstep with Tnua's systems because of how actions work.

Base on these goals and your description, I'd say the recommended way is to have both TnuaControllerPlugin and TnuaAvian#dPlugin use PhysicsSchedule and to put the user control systems under FixedUpdate?

Also, regarding the demos, this means that ScheduleToUse::PhysicsSchedule is meaningless. Instead, for Avian, ScheduleToUse::Update should register Avian itself under PostUpdate and the user control systems under Update while ScheduleToUse::FixedUpdate should register Avian itself under FixedPostUpdate and the user control systems under FixedUpdate - and either way the Tnua plugins should be registered under PhysicsSchedule.

Copy link

@BrainBacon BrainBacon Mar 10, 2025

Choose a reason for hiding this comment

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

@idanarye Is this still blocked by the scheduling choices? I've been using this fork for use with lightyear with success for the last couple months.

Copy link
Owner

@idanarye idanarye Mar 10, 2025

Choose a reason for hiding this comment

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

@BrainBacon
Pretty much, yes. Not sure if "blocked" is the correct word, since this mainly is about the docs and about the demos and I can always just merge it and fix it myself, but I do want to at least understand it before doing so.

Copy link
Owner

@idanarye idanarye Mar 11, 2025

Choose a reason for hiding this comment

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

I think I'll just merge it and fix the scheduling (and doc comments)

///
/// # Example
///
/// ```ignore
/// App::new()
/// .add_plugins((
/// DefaultPlugins,
/// PhysicsPlugins::default(),
/// TnuaControllerPlugin::new(PhysicsSchedule),
/// TnuaAvian2dPlugin::new(PhysicsSchedule),
/// ));
/// ```
pub struct TnuaAvian2dPlugin {
schedule: InternedScheduleLabel,
}
Expand All @@ -39,8 +53,7 @@ impl Plugin for TnuaAvian2dPlugin {
app.configure_sets(
self.schedule,
TnuaSystemSet
.before(PhysicsSet::Prepare)
.before(PhysicsStepSet::First)
.in_set(PhysicsStepSet::First)
.run_if(|physics_time: Res<Time<Physics>>| !physics_time.is_paused()),
);
app.add_systems(
Expand All @@ -65,24 +78,25 @@ pub struct TnuaAvian2dSensorShape(pub Collider);
fn update_rigid_body_trackers_system(
gravity: Res<Gravity>,
mut query: Query<(
&GlobalTransform,
&Position,
&Rotation,
&LinearVelocity,
&AngularVelocity,
&mut TnuaRigidBodyTracker,
Option<&TnuaToggle>,
)>,
) {
for (transform, linaer_velocity, angular_velocity, mut tracker, tnua_toggle) in query.iter_mut()
for (position, rotation, linaer_velocity, angular_velocity, mut tracker, tnua_toggle) in
query.iter_mut()
{
match tnua_toggle.copied().unwrap_or_default() {
TnuaToggle::Disabled => continue,
TnuaToggle::SenseOnly => {}
TnuaToggle::Enabled => {}
}
let (_, rotation, translation) = transform.to_scale_rotation_translation();
*tracker = TnuaRigidBodyTracker {
translation: translation.adjust_precision(),
rotation: rotation.adjust_precision(),
translation: position.adjust_precision().extend(0.0),
rotation: Quaternion::from(*rotation).adjust_precision(),
velocity: linaer_velocity.0.extend(0.0),
angvel: Vector3::new(0.0, 0.0, angular_velocity.0),
gravity: gravity.0.extend(0.0),
Expand All @@ -96,7 +110,9 @@ fn update_proximity_sensors_system(
collisions: Res<Collisions>,
mut query: Query<(
Entity,
&GlobalTransform,
&Position,
&Rotation,
&Collider,
&mut TnuaProximitySensor,
Option<&TnuaAvian2dSensorShape>,
Option<&mut TnuaGhostSensor>,
Expand All @@ -105,7 +121,7 @@ fn update_proximity_sensors_system(
)>,
collision_layers_entity: Query<&CollisionLayers>,
other_object_query: Query<(
Option<(&GlobalTransform, &LinearVelocity, &AngularVelocity)>,
Option<(&Position, &LinearVelocity, &AngularVelocity)>,
Option<&CollisionLayers>,
Has<TnuaGhostPlatform>,
Has<Sensor>,
Expand All @@ -114,7 +130,9 @@ fn update_proximity_sensors_system(
query.par_iter_mut().for_each(
|(
owner_entity,
transform,
position,
rotation,
collider,
mut sensor,
shape,
mut ghost_sensor,
Expand All @@ -126,6 +144,11 @@ fn update_proximity_sensors_system(
TnuaToggle::SenseOnly => {}
TnuaToggle::Enabled => {}
}
let transform = Transform {
translation: position.adjust_precision().extend(0.0),
rotation: Quaternion::from(*rotation).adjust_precision(),
scale: collider.scale().adjust_precision().extend(1.0),
};
let cast_origin = transform.transform_point(sensor.cast_origin.f32());
let cast_direction = sensor.cast_direction;
let cast_direction_2d = Dir2::new(cast_direction.truncate())
Expand Down Expand Up @@ -195,14 +218,14 @@ fn update_proximity_sensors_system(

let entity_linvel;
let entity_angvel;
if let Some((entity_transform, entity_linear_velocity, entity_angular_velocity)) =
if let Some((entity_position, entity_linear_velocity, entity_angular_velocity)) =
entity_kinematic_data
{
entity_angvel = Vector3::new(0.0, 0.0, entity_angular_velocity.0);
entity_linvel = entity_linear_velocity.0.extend(0.0)
+ if 0.0 < entity_angvel.length_squared() {
let relative_point = intersection_point
- entity_transform.translation().truncate().adjust_precision();
let relative_point =
intersection_point - entity_position.adjust_precision();
// NOTE: no need to project relative_point on the
// rotation plane, it will not affect the cross
// product.
Expand Down
50 changes: 36 additions & 14 deletions avian3d/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,20 @@ use bevy_tnua_physics_integration_layer::TnuaSystemSet;
/// Add this plugin to use avian3d as a physics backend.
///
/// This plugin should be used in addition to `TnuaControllerPlugin`.
/// Note that you should make sure both of these plugins use the same schedule.
/// This should usually be `PhysicsSchedule`, which by default is `FixedUpdate`.
///
/// # Example
///
/// ```ignore
/// App::new()
/// .add_plugins((
/// DefaultPlugins,
/// PhysicsPlugins::default(),
/// TnuaControllerPlugin::new(PhysicsSchedule),
/// TnuaAvian3dPlugin::new(PhysicsSchedule),
/// ));
/// ```
pub struct TnuaAvian3dPlugin {
schedule: InternedScheduleLabel,
}
Expand All @@ -47,8 +61,7 @@ impl Plugin for TnuaAvian3dPlugin {
app.configure_sets(
self.schedule,
TnuaSystemSet
.before(PhysicsSet::Prepare)
.before(PhysicsStepSet::First)
.in_set(PhysicsStepSet::First)
.run_if(|physics_time: Res<Time<Physics>>| !physics_time.is_paused()),
);
app.add_systems(
Expand All @@ -73,23 +86,24 @@ pub struct TnuaAvian3dSensorShape(pub Collider);
fn update_rigid_body_trackers_system(
gravity: Res<Gravity>,
mut query: Query<(
&GlobalTransform,
&Position,
&Rotation,
&LinearVelocity,
&AngularVelocity,
&mut TnuaRigidBodyTracker,
Option<&TnuaToggle>,
)>,
) {
for (transform, linaer_velocity, angular_velocity, mut tracker, tnua_toggle) in query.iter_mut()
for (position, rotation, linaer_velocity, angular_velocity, mut tracker, tnua_toggle) in
query.iter_mut()
{
match tnua_toggle.copied().unwrap_or_default() {
TnuaToggle::Disabled => continue,
TnuaToggle::SenseOnly => {}
TnuaToggle::Enabled => {}
}
let (_, rotation, translation) = transform.to_scale_rotation_translation();
*tracker = TnuaRigidBodyTracker {
translation: translation.adjust_precision(),
translation: position.adjust_precision(),
rotation: rotation.adjust_precision(),
velocity: linaer_velocity.0.adjust_precision(),
angvel: angular_velocity.0.adjust_precision(),
Expand All @@ -104,7 +118,9 @@ fn update_proximity_sensors_system(
collisions: Res<Collisions>,
mut query: Query<(
Entity,
&GlobalTransform,
&Position,
&Rotation,
&Collider,
&mut TnuaProximitySensor,
Option<&TnuaAvian3dSensorShape>,
Option<&mut TnuaGhostSensor>,
Expand All @@ -113,7 +129,7 @@ fn update_proximity_sensors_system(
)>,
collision_layers_entity: Query<&CollisionLayers>,
other_object_query: Query<(
Option<(&GlobalTransform, &LinearVelocity, &AngularVelocity)>,
Option<(&Position, &LinearVelocity, &AngularVelocity)>,
Option<&CollisionLayers>,
Has<TnuaGhostPlatform>,
Has<Sensor>,
Expand All @@ -122,7 +138,9 @@ fn update_proximity_sensors_system(
query.par_iter_mut().for_each(
|(
owner_entity,
transform,
position,
rotation,
collider,
mut sensor,
shape,
mut ghost_sensor,
Expand All @@ -134,6 +152,11 @@ fn update_proximity_sensors_system(
TnuaToggle::SenseOnly => {}
TnuaToggle::Enabled => {}
}
let transform = Transform {
translation: position.0,
rotation: rotation.0,
scale: collider.scale(),
};

// TODO: is there any point in doing these transformations as f64 when that feature
// flag is active?
Expand Down Expand Up @@ -203,14 +226,14 @@ fn update_proximity_sensors_system(

let entity_linvel;
let entity_angvel;
if let Some((entity_transform, entity_linear_velocity, entity_angular_velocity)) =
if let Some((entity_position, entity_linear_velocity, entity_angular_velocity)) =
entity_kinematic_data
{
entity_angvel = entity_angular_velocity.0.adjust_precision();
entity_linvel = entity_linear_velocity.0.adjust_precision()
+ if 0.0 < entity_angvel.length_squared() {
let relative_point = intersection_point
- entity_transform.translation().adjust_precision();
let relative_point =
intersection_point - entity_position.adjust_precision();
// NOTE: no need to project relative_point on the
// rotation plane, it will not affect the cross
// product.
Expand Down Expand Up @@ -252,10 +275,9 @@ fn update_proximity_sensors_system(

let query_filter = SpatialQueryFilter::from_excluded_entities([owner_entity]);
if let Some(TnuaAvian3dSensorShape(shape)) = shape {
let (_, owner_rotation, _) = transform.to_scale_rotation_translation();
let owner_rotation = Quat::from_axis_angle(
*cast_direction,
owner_rotation.to_scaled_axis().dot(*cast_direction),
rotation.to_scaled_axis().dot(*cast_direction),
);
spatial_query_pipeline.shape_hits_callback(
shape,
Expand Down
13 changes: 12 additions & 1 deletion demos/src/app_setup_options.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,18 @@ impl AppSetupConfiguration {
} else if cfg!(feature = "avian") {
ScheduleToUse::FixedUpdate
} else {
ScheduleToUse::Update
#[cfg(feature = "avian")]
{
ScheduleToUse::PhysicsSchedule
}
#[cfg(feature = "rapier")]
{
ScheduleToUse::Update
}
#[cfg(all(not(feature = "avian"), not(feature = "rapier")))]
{
panic!("No schedule was specified, but also no physics engine is avaible. Therefore, there is no fallback.")
}
},
level_to_load: url_params.get("level"),
}
Expand Down
16 changes: 10 additions & 6 deletions demos/src/bin/platformer_2d.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#[cfg(feature = "avian2d")]
use avian2d::{prelude as avian, prelude::*, schedule::PhysicsSchedule};
use avian2d::{prelude as avian, prelude::*};
use bevy::ecs::schedule::ScheduleLabel;
use bevy::prelude::*;
#[cfg(feature = "rapier2d")]
Expand All @@ -22,6 +22,7 @@ use tnua_demos_crate::app_setup_options::{AppSetupConfiguration, ScheduleToUse};
use tnua_demos_crate::character_control_systems::info_dumpeing_systems::character_control_info_dumping_system;
use tnua_demos_crate::character_control_systems::platformer_control_systems::{
apply_platformer_controls, CharacterMotionConfigForPlatformerDemo, FallingThroughControlScheme,
JustPressedCachePlugin,
};
use tnua_demos_crate::character_control_systems::Dimensionality;
use tnua_demos_crate::level_mechanics::LevelMechanicsPlugin;
Expand Down Expand Up @@ -101,10 +102,10 @@ fn main() {
app.add_plugins(TnuaControllerPlugin::new(FixedUpdate));
app.add_plugins(TnuaCrouchEnforcerPlugin::new(FixedUpdate));
}
#[cfg(any(feature = "avian", feature = "avian"))]
#[cfg(feature = "avian")]
ScheduleToUse::PhysicsSchedule => {
app.add_plugins(TnuaControllerPlugin::new(PhysicsSchedule));
app.add_plugins(TnuaCrouchEnforcerPlugin::new(PhysicsSchedule));
app.add_plugins(TnuaControllerPlugin::new(FixedUpdate));
Copy link
Owner

@idanarye idanarye Jan 17, 2025

Choose a reason for hiding this comment

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

Wait what?

I thought Avian's PhysicsSchedule is a schedule of its own which has its frequency controlled by Avian. Was it changed to just be equivalent to the FixedUpdate? I don't see anything about it in the release notes.

Also, if PhysicsSchedule really does mean the exact same thing as FixedUpdate - to the point where when we run the demo with PhysicsSchedule it gets switched to FixedUpdate behind our back - maybe we should just remove ScheduleToUse::PhysicsSchedule?

Copy link
Contributor Author

@Alpyg Alpyg Jan 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think I messed up when I rebased, I reverted it

app.add_plugins(TnuaCrouchEnforcerPlugin::new(FixedUpdate));
}
}

Expand All @@ -129,11 +130,14 @@ fn main() {
ScheduleToUse::Update => Update.intern(),
ScheduleToUse::FixedUpdate => FixedUpdate.intern(),
#[cfg(feature = "avian")]
ScheduleToUse::PhysicsSchedule => PhysicsSchedule.intern(),
// `PhysicsSchedule` is `FixedPostUpdate` by default, which allows us
// to run user code like the platformer controls in `FixedUpdate`,
// which is a bit more idiomatic.
ScheduleToUse::PhysicsSchedule => FixedUpdate.intern(),
},
apply_platformer_controls.in_set(TnuaUserControlsSystemSet),
);
app.add_plugins(LevelMechanicsPlugin);
app.add_plugins((LevelMechanicsPlugin, JustPressedCachePlugin));
#[cfg(feature = "rapier2d")]
{
app.add_systems(Startup, |mut cfg: Single<&mut RapierConfiguration>| {
Expand Down
18 changes: 12 additions & 6 deletions demos/src/bin/platformer_3d.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#[cfg(feature = "avian3d")]
use avian3d::{prelude as avian, prelude::*, schedule::PhysicsSchedule};
use avian3d::{prelude as avian, prelude::*};
use bevy::ecs::schedule::ScheduleLabel;
use bevy::prelude::*;
#[cfg(feature = "rapier3d")]
Expand All @@ -19,9 +19,6 @@ use bevy_tnua_avian3d::*;
use bevy_tnua_rapier3d::*;

use tnua_demos_crate::app_setup_options::{AppSetupConfiguration, ScheduleToUse};
use tnua_demos_crate::character_animating_systems::platformer_animating_systems::{
animate_platformer_character, AnimationState,
};
#[cfg(feature = "egui")]
use tnua_demos_crate::character_control_systems::info_dumpeing_systems::character_control_info_dumping_system;
use tnua_demos_crate::character_control_systems::platformer_control_systems::{
Expand All @@ -41,6 +38,12 @@ use tnua_demos_crate::ui::plotting::PlotSource;
#[cfg(feature = "egui")]
use tnua_demos_crate::ui::DemoInfoUpdateSystemSet;
use tnua_demos_crate::util::animating::{animation_patcher_system, GltfSceneHandler};
use tnua_demos_crate::{
character_animating_systems::platformer_animating_systems::{
animate_platformer_character, AnimationState,
},
character_control_systems::platformer_control_systems::JustPressedCachePlugin,
};

fn main() {
tnua_demos_crate::verify_physics_backends_features!("rapier3d", "avian3d");
Expand Down Expand Up @@ -137,13 +140,16 @@ fn main() {
ScheduleToUse::Update => Update.intern(),
ScheduleToUse::FixedUpdate => FixedUpdate.intern(),
#[cfg(feature = "avian")]
ScheduleToUse::PhysicsSchedule => PhysicsSchedule.intern(),
// `PhysicsSchedule` is `FixedPostUpdate` by default, which allows us
// to run user code like the platformer controls in `FixedUpdate`,
// which is a bit more idiomatic.
ScheduleToUse::PhysicsSchedule => FixedUpdate.intern(),
},
apply_platformer_controls.in_set(TnuaUserControlsSystemSet),
);
app.add_systems(Update, animation_patcher_system);
app.add_systems(Update, animate_platformer_character);
app.add_plugins(LevelMechanicsPlugin);
app.add_plugins((LevelMechanicsPlugin, JustPressedCachePlugin));
app.run();
}

Expand Down
Loading
Loading