gschup / bevy_ggrs

Bevy plugin for the GGRS P2P rollback networking library.
Other
302 stars 41 forks source link

Integration Tests incorrect #54

Closed gschup closed 1 year ago

gschup commented 1 year ago

I deleted the integration tests I previously merged. While I like the idea of having more testing not only on ggrs, but also the plugin, these tests did not correctly test what should happen. This should be fixed at some point.

donedgardo commented 1 year ago

I'd love to contribute for this. In general can we get a conversation going on what should be tested instead?

As a person integrating this library into their bevy game I'd like to see that:

The test in the link provided would be a good start, I would remove line https://github.com/gschup/bevy_ggrs/blob/de326196cd484bbe74a0c0b7d25219fe304d7d19/tests/example_integration.rs#L29 It is not expected that the two systems are completely in sync only that both system are being scheduled and executed.

Additionally the second test would be a good candidate for the integration test and rollback behavior's: https://github.com/gschup/bevy_ggrs/blob/de326196cd484bbe74a0c0b7d25219fe304d7d19/tests/example_integration.rs#L35

Now with thanks to this helper function or something very similar, we can now also test frame by frame scenarios/edge cases:

https://github.com/johanhelsing/bevy_xpbd_jondolf/blob/0d2582fc9fb6a5324d50f922d933612a76461d30/src/tests.rs#L33

fn tick_60_fps(app: &mut App) {
    let mut update_strategy = app.world.resource_mut::<TimeUpdateStrategy>();
    let TimeUpdateStrategy::ManualInstant(prev_time) = *update_strategy else { unimplemented!() };
    *update_strategy =
        TimeUpdateStrategy::ManualInstant(prev_time + Duration::from_secs_f64(1. / 60.));
    app.update();
}
gschup commented 1 year ago

solved in #69 . we now have a nice basis to extend integration tests from if we wish to do so!