API to add simulation to the specific step

[Shatur]

When developing multiplayer, physics ticks need to be controlled manually. In this case we not only have to run physics on network tickrate, but sometimes also have to perform additional ticks in a single frame in case of rollback.

Rollback is needed in fast-paced games when all inputs are predicted. When an desync occurs, the client rollbacks the entire world to the last synchronized position, reapplies all user input, and re-simulates the entire world forward by the number of frames rolled back. In some games rollback and re-simulation happens on each network tick.

To achieve this we could add an option to disable stages creation (but still add systems to that labels):

bevy_rapier/src/plugin/plugin.rs

Lines 84 to 93 in 4eeffe4

	app.add_stage_before(
	CoreStage::Update,
	PhysicsStages::StepSimulation,
	SystemStage::parallel(),
	);
	app.add_stage_before(
	CoreStage::Last,
	PhysicsStages::DetectDespawn,
	SystemStage::parallel(),
	);

In this case user will need to create stages with this labels manually. This will allow to run them in its own exclusive system as many times as needed (he will also need to update SimulationToRenderTime to apply all changes to Bevy components immediately).

Relevant PR in heron: jcornaz/heron#171

[sebcrozet]

Sounds good! In that case, we should define more stage labels in bevy_rapier, for better granularity. I’m thinking something like:

pub enum PhysicsStages {
    SyncBackend,      // All systems from `ApplyScale` to `InitJoints`.
    StepSimulation,   // Only the `StepSimulation` system.
    Writeback,        // Only the `WritebackRigidBodies` (may add a `WritebackJoints` later).
    DetectDespawn,
}

Could there be any configuration where the user would want to run our systems within their own stages instead of using our labels?

Note that there is #141, which suggests an events-based stepping scheme: you generate events that tells StepSimulation to execute one physics step per event received. It is simpler but less flexible than the solution suggested here (so having both options sound useful).

[Shatur]

In that case, we should define more stage labels in bevy_rapier, for better granularity. I’m thinking something like

Sounds good!

Could there be any configuration where the user would want to run our systems within their own stages instead of using our labels?

I would say no. Not sure when user may want to create its own labels. So I think an option to disable stages creation should be enough.

Note that there is #141, which suggests an events-based stepping scheme

Yes, I discussed with @ggaast in discord and his use case is also networking :) But we came to the conclusion that events are not suitable for this user case. Because it is necessary to run other systems after each physical step. So we think that for networking we need only this feature.

[mvlabat]

Does this design imply that we also need to export system set constructors themselves? To let users avoid adding all the systems to the stages manually.

[Shatur]

No, no, you just create stages with exported labels yourself (SyncBackend, StepSimulation, etc.) and the plugin will just put all systems at this labels.

[mvlabat]

Ah, that makes sense. Yeah, the solution sounds pretty good!

[mvlabat]

Oh, wait, I'm actually not sure about this. Bevy also has a notion of nested stages (a user can pass a Schedule instead of a SystemSet, which also can have its own stages). If a user wants to add rapier stages to a sub-stage of their app, they need an ability to somehow pass a mutable reference to their sub-stage to the plugin builder. I'm not sure that it's something that Bevy's current API allows to do.

[Shatur]

Can you create a sub-stage you need and just label it using the exported labels? This plugin will just put its systems as usual.

[mvlabat]

Yes, I can create a sub-stage with the rapier's label, but a plugin won't be able to access it via the application builder. Consider the following example:

pub use bevy::prelude::*;

pub fn main() {
    let mut test_schedule = Schedule::default();
    test_schedule.add_stage("sub_stage", SystemStage::parallel());

    App::new()
        .add_stage("test_schedule", test_schedule)
        .add_stage("normal_stage", SystemStage::parallel())
        .add_system_to_stage("normal_stage", test_system)
        .add_system_to_stage("sub_stage", test_system);
}

fn test_system() {}

This will panic with 'Stage '"sub_stage"' does not exist or is not a SystemStage'.

If I want to add a system to the "sub_stage" stage, I need to call the add_system_to_stage method on test_schedule, which rapier won't know anything about.

[Shatur]

Oh, I see... Not sure if we have a good solution for it until stageless lands.
The only thing I can suggest is to avoid using nested stages...

[mvlabat]

Yeah, well, in my case I've no other choice than re-schedule all the systems manually. :) ... unless bevy_rapier provides some kind of a helper function that would return a SystemSet for each stage. Users will be able to use the add_system_set_to_stage method then, which would help to keep track of the changes a lot (i.e. I wouldn't need to be worried about missing an addition of a new system or any system changing its order).

[Shatur]

Curious, why do you need a sub-stage?

[mvlabat]

As I mentioned in #148, my multiplayer game may need to run the simulation more than once per frame (for example, if its state is rewinded due to a client mispredicting updates, I need to run re-run all the simulations since the newest update). Using custom schedules and run criteria makes it really convenient to implement:
https://github.com/mvlabat/muddle-run/blob/c86396135e8ea779143d84d6dfba3e263b51151b/libs/shared_lib/src/lib.rs#L173

[Shatur]

Using custom schedules and run criteria makes it really convenient to implement

I would suggest you to consider using an exclusive system which runs needed schedules required number of times. When stageless lands, you will need to migrate to exclusive system anyway because you won't be able to re-run your systems using run criteria (ShouldRun::YesAndCheckAgain will be removed).

[mvlabat]

As far as I understand, the proposed alternative to a looping run criteria is running a schedule inside an exclusive system. Basically, an exclusive system will contain almost the same code that a run criteria system does. Which still means there needs to be a way to add systems not to the App's main schedule but to a custom one.

[Shatur]

Oh, you are right, I forgot that such stage won't be a part of a global schedule. Then a system set for each stage looks like the only way to go. With an option to disable all stages creation (to load init only Rapier's resources).

[cscorley]

I think it is possible to currently hack together something that accomplishes this by doing exactly what the RapierPhysicsPlugin.build() does. However, I have not yet found it fruitful in rollback libraries (e.g. GGRS) since they require full determinism. I am often easily able to cause de-sync between two running versions of the same game on the same machine. I think in many scenarios, I may be better off fully syncing the state across the network instead of rolling back to a previous local state (that may already have incorrect simulations, but were correct per inputs -- I may also simply have an init-order problem).

Follows is an example of how I was able to put Rapier physics into bevy_ggrs:

    app.insert_resource(RapierConfiguration::default())
        .insert_resource(SimulationToRenderTime::default())
        .insert_resource(RapierContext::default())
        .insert_resource(Events::<CollisionEvent>::default())
        .insert_resource(PhysicsHooksWithQueryResource::<NoUserData>(Box::new(())));

    let physics_pipeline = SystemStage::parallel()
        .with_system(systems::init_async_shapes)
        .with_system(systems::apply_scale.after(systems::init_async_shapes))
        .with_system(systems::apply_collider_user_changes.after(systems::apply_scale))
        .with_system(
            systems::apply_rigid_body_user_changes.after(systems::apply_collider_user_changes),
        )
        .with_system(
            systems::apply_joint_user_changes.after(systems::apply_rigid_body_user_changes),
        )
        .with_system(systems::init_rigid_bodies.after(systems::apply_joint_user_changes))
        .with_system(
            systems::init_colliders
                .after(systems::init_rigid_bodies)
                .after(systems::init_async_shapes),
        )
        .with_system(systems::init_joints.after(systems::init_colliders))
        .with_system(systems::sync_removals.after(systems::init_joints))
        .with_system(systems::step_simulation::<NoUserData>.after(systems::sync_removals))
        .with_system(
            systems::update_colliding_entities.after(systems::step_simulation::<NoUserData>),
        )
        .with_system(systems::writeback_rigid_bodies.after(systems::step_simulation::<NoUserData>));

    GGRSPlugin::<GGRSConfig>::new()
        .with_update_frequency(FPS)
        .with_input_system(round::input)
        .register_rollback_type::<Transform>()
        .register_rollback_type::<Velocity>()
        .register_rollback_type::<components::FrameCount>()
        .with_rollback_schedule(
            Schedule::default()
                .with_stage(
                    ROLLBACK_SYSTEMS,
                    SystemStage::parallel()
                        .with_system(apply_inputs)
                        .with_system(update_velocity.after(apply_inputs))
                        .with_system(increase_frame_count),
                )
                .with_stage_after(ROLLBACK_SYSTEMS, PHYSICS_SYSTEMS, physics_pipeline),
        )
        .build(&mut app);

I'm still new to this, but it seems less stressful to implement something on RapierPhysicsPlugin that returns a SystemStage and allow the user to plug that wherever instead of relying on labels. (I believe this is the same as what @Shatur was suggesting)

A hypothetical view:

    app.add_plugin(RapierDebugRenderPlugin::default());

    let physics_plugin = RapierPhysicsPlugin::new()
        .without_stage_setup()
        .build(&mut app);  // Just need to insert resources, doesn't need to use the same entry point as regular plugin initialization

    let physics_pipeline = physics_plugin.physics_pipeline_stage();

    GGRSPlugin::<GGRSConfig>::new()
       ... // Same as before

(Not pictured above, hooking into the despawn system, which bevy_ggrs could also possibly do during rollbacks. I think since bevy_ggrs only hooks into CoreStage::Update, it would be fine to leave it in CoreStage::Last)

[Shatur]

I'm still new to this, but it seems less stressful to implement something on RapierPhysicsPlugin that returns a SystemStage and allow the user to plug that wherever instead of relying on labels. (I believe this is the same as what @Shatur was suggesting)

Yep.

A hypothetical view:

Exactly, but instead of builder interface we could just have a separate struct which implements Plugin or a boolean flag in RapierPhysicsPlugin.

[cscorley]

I was successfully able to make the above ideas work with bevy_ggrs. I have a working example here: https://github.com/cscorley/bevy_ggrs_rapier_example.

(It turns out my earlier problems were because I just needed to switch to TimestepMode::Fixed. Something that allows a "manual mode" I think would be helpful since in this scenario, we'd be turning over the stepping to the other plugin. I'm just guessing that the dt value I set is good, but unsure of it's implications.)

Time allowing, I should be able to do a PR for this week, but would like guidance on what the ultimate API should look like.

[Shatur]

I was successfully able to make the above ideas work with bevy_ggrs.

Nice to hear!

It turns out my earlier problems were because I just needed to switch to TimestepMode::Fixed

How this config is used by Rapier when you step systems manually?

Time allowing, I should be able to do a PR for this week, but would like guidance on what the ultimate API should look like.

I think it would be cool to have enum like this:

pub enum PhysicsStages {
    SyncBackend,      // All systems from `ApplyScale` to `InitJoints`.
    StepSimulation,   // Only the `StepSimulation` system.
    Writeback,        // Only the `WritebackRigidBodies` (may add a `WritebackJoints` later).
    DetectDespawn,
}

And a function which returns a stage with such systems.

[cscorley]

How this config is used by Rapier when you step systems manually?

I am still using bevy_rapier and using its systems directly. Straight copy and paste. The TimestepMode in all cases overrides the dt parameter of the integration_parameters. It is a simple division here, but I think (going on an old GGRS example that did have direct integration with Rapier) it would may be more exact to avoid this and simply pass through integration_parameters without modification.

And a function which returns a stage with such systems.

Would the idea here be something like:

impl<PhysicsHooksData> RapierPhysicsPlugin<PhysicsHooksData> {

    pub fn get_stage(stage: PhysicsStages) -> SystemStage {
        match stage {
            SyncBackend => {
                SystemStage::parallel()
                    .with_system(systems::init_async_shapes)
                    .with_system(systems::apply_scale.after(systems::init_async_shapes))
                    // ... etc
            }
            _ => SystemStage::parallel()
        }

    }
}

Or is the desire to have a function per stage?

[Shatur]

dt parameter of the integration_parameters

Thanks for the clarification!

Would the idea here be something like:

Yep, that's what I meant.

Or is the desire to have a function per stage?

But this suggestion is probably better :)