`Time<Physics>` pause is delayed by one frame

[Pascualex]

From what I'm seeing, unpausing Time<Physics> takes effect immediately, but pausing it is delayed by one frame.

A simplified extract from my logs:

# fixed schedule 1 (already paused)
after FixedPostUpdate, body position: [6.0856233, -7.507883] (same as before)
# fixed schedule 2 (already paused)
after FixedPostUpdate, body position: [6.0856233, -7.507883] (same as before)
# fixed schedule 3
on FixedUpdate, unpause Time<Physics>
after FixedPostUpdate, body position: [5.954998, -7.2947235] (updated)
# fixed schedule 4
on FixedUpdate, pause Time<Physics>
after FixedPostUpdate, body position: [5.824373, -7.081564] (updated)
# fixed schedule 5 (already paused)
after FixedPostUpdate, body position: [5.824373, -7.081564] (same as before)

I would expect the physics simulation to only run one time in this situation, but it seems to have run twice.

[Pascualex]

Took a look at the code and was able to work around it like this:

physics_time.pause();
physics_time.advance_by(Duration::ZERO);

I'm pretty sure the issue lies in run_physics_schedule. Shouldn't Time<Physics> be reset before advancing the simulation?:

// Advance the physics clock by the timestep if not paused.
if !is_paused {
    world.resource_mut::<Time<Physics>>().advance_by(timestep);

    // Advance the substep clock already so that systems running
    // before the substepping loop have the right delta.
    let SubstepCount(substeps) = *world.resource::<SubstepCount>();
    let sub_delta = timestep.div_f64(substeps as f64);
    world.resource_mut::<Time<Substeps>>().advance_by(sub_delta);
}

// Set the generic `Time` resource to `Time<Physics>`.
*world.resource_mut::<Time>() = world.resource::<Time<Physics>>().as_generic();

// Advance the simulation.
if !world.resource::<Time>().delta().is_zero() {
    trace!("running PhysicsSchedule");
    schedule.run(world);
}

// If physics is paused, reset delta time to stop the simulation
// unless users manually advance `Time<Physics>`.
if is_paused {
    world
        .resource_mut::<Time<Physics>>()
        .advance_by(Duration::ZERO);
}

Could create a simple PR if you want.

Edit: now I see that if you did that then users wouldn't be able to manually advance Time<Physics> 🤔. Maybe the trick then is to always do .advance_by(Duration::ZERO) regardless of if the physics time is paused?

// Reset delta time to stop the simulation unless
// `Time<Physics>` is unpaused or manually advanced.
world
    .resource_mut::<Time<Physics>>()
    .advance_by(Duration::ZERO);