Bevy 0.10 - Plugin Rewrite #52

gschup · 2023-03-12T08:03:56Z

This is a rewrite of the plugin. So far, I am mostly experimenting with stuff, but the main goals are:

use bevy's new schedule v3
offer ScheduleLabels and SystemSets like PreLoad, PostLoad for better ergonomy and to cover cases like Add a RollbackEventHook Trait #33

Originally part of the rewrite, but maybe suited for another PR:

swap out the world snapshot with a new version that does not need reflect.

With the new infrastructure, a new world snapshot system can be swapped in and out much more easily.

johanhelsing · 2023-03-12T08:15:10Z

swap out the world snapshot with a new version that does not need reflect.

Really looking forward to this! The bevy_reflect part is currently the biggest performance bottleneck of my game.

donedgardo · 2023-03-12T08:15:45Z

I understand feeling dirty after using Reflect for saving/loading.

I've come across this issue that relates to our issue:
bevyengine/bevy#1442

In it, some pointed out these save plugin:
https://github.com/hankjordan/bevy_save
https://github.com/Zeenobit/moonshine_save

Ideally it be something included in bevy.

What are your ideas on the save/load world/scene approaches? @gschup

johanhelsing · 2023-03-12T08:20:47Z

Haha, yeah, I even factored out the snapping in this repo to a separate crate at one point: https://github.com/johanhelsing/bevy_snap since I needed it for snapshotting state in my physics engine experiments.

gschup · 2023-03-12T08:26:09Z

What are your ideas on the save/load world/scene approaches? @gschup

I am not sure how exactly this will work out, but I would like to make the saving/loading require only Clone.

Mark entities that are affected
register component types you want saved/loaded of these types
every registration adds a system to the save/load schedule that takes care of only those components
the systems in the save/load schedules clone stuff around, maybe into some hashmap-thingy that we then call a "snapshot"

johanhelsing · 2023-03-14T07:34:40Z

Would it make sense to look at how the extraction for bevy's rendering works for inspiration? Kind of feels like there would be some related concepts and patterns.

Add integration test and continue bevy 0.10 rewrite with Input system.

donedgardo · 2023-03-18T01:04:41Z

tests/integration.rs

+    let frame_count2 = app2.world.get_resource::<FrameCount>().unwrap();
+    assert!(frame_count1.frame > 0);
+    assert!(frame_count2.frame > 0);
+    assert_eq!(frame_count1.frame, frame_count2.frame);


Come to think of it, this assertion is wrong. It's ok if the frame count don't match. That is expected. This test should only test if the Advance frame stage systems run.

johanhelsing · 2023-03-18T09:08:39Z

tests/integration.rs

+    };
+    app1.insert_resource(inputs_resource);
+    for _ in 0..50 {
+        app1.update();


Does bevy_ggrs read from the Time resource? If so, I don't think this will actually advance the game 50 frames, probably just one frame.

I tried making some test for bevy_xpbd based on these ones, and I had to manually update the time resource, see https://github.com/Jondolf/bevy_xpbd/pull/18/files#diff-a0262fea108af44a98cc49e5eb72641963dd816513f2d0a22eb738fcfed55ebe

You are correct, it doesn't advance the game by 50 frames. So yeah in this case probably a few frames, depending on CI machine. To my understanding app.update() advances the advances systems by one cycle (whatever that means).

If you want to advance by frame/time I think a great example is taken from this ggrs integration test:

bevy_ggrs/tests/entity_maping.rs

Line 103 in 9542195

let sleep = || std::thread::sleep(Duration::from_secs_f32(1.0 / 60.0));

The hard part for me is that the time/cycles it needs to actually connect both apps to each peer varies between machines. So splitting the connection part and the frame advance systems by time can be tricky to run in a CI machine.

I really like what you did here: https://github.com/Jondolf/bevy_xpbd/blob/11499efaf5b039ba356f029d11f976449842cca2/src/tests.rs#L33

This is very useful for testing your system on a frame by frame scenario.

But to be honest if I would be testing a system in a frame by frame scenario I would probably isolate the FrameCount and the system under test by stubbing FrameCount and testing the system in a more unit-test like fashion.

Are the systems here demanding enough to take more than 16ms of cpu-time, though?

I don't know.
Why you ask?

Just that I think ggrs default update frequency is set to 60 fps, so if you don't run for at least 16 ms, you'll never get to the next frame, and you might not actually test what you think you're testing.

gschup · 2023-03-30T15:10:09Z

I did a little more work on the plugin rewrite. I have decided to keep the reflect-based world snapshotting in for a while, so the new scheduling API changes can be evaluated separately.

…s again

gschup · 2023-03-31T07:24:30Z

All three examples are now back in, but saving and loading is causing issues. I am sure I made a small mistake somewhere.

bushrat011899 · 2023-04-21T03:12:16Z

I've been experimenting with some major changes I think would suit a Bevy 0.10 rewrite. The big change I was focused on was allowing 3rd party save/load systems in the rollback process. If a user could create their own save/load systems, then the Reflection based system could be pushed into its own plugin for those that still need it, and then serialisation, cloning, and copying could be used where appropriate instead.

I have a branch I've made here which achieves this by creating two new public schedules, GGRSSaveSchedule and GGRSLoadSchedule. Each is appropriately called in a similar manner to how GGRSSchedule is currently called, as a response to GGRS requests. From there, I moved all of the existing save/load code into a pair of systems that live in these schedules.

Finally, this allowed me to make an example plugin, ResourceRollbackPlugin, which hooks into these schedules to allow custom save/load behaviour (in this example, I use Clone). Please see resource_snapshot.rs for the entire implementation of a resource snapshotting plugin.

/* snip from box_game_synctest.rs */

// start the GGRS session
let sess = sess_build.start_synctest_session()?;

let mut app = App::new();

GGRSPlugin::<GGRSConfig>::new()
    .with_update_frequency(FPS)
    .with_input_system(input)
    // register types of components AND resources you want to be rolled back
    .register_rollback_component::<Transform>()
    .register_rollback_component::<Velocity>()
    .build(&mut app);

// continue building/running the app like you normally would
app.insert_resource(opt)
    .add_plugins(DefaultPlugins)

    // NEW
    .add_plugin(ResourceRollbackPlugin.for_type::<FrameCount>().with_config::<GGRSConfig>())

    .add_startup_system(setup_system)
    .add_systems((move_cube_system, increase_frame_system).in_schedule(GGRSSchedule))
    .insert_resource(Session::SyncTestSession(sess))
    .insert_resource(FrameCount { frame: 0 })
    .run();

/* snip */

The exact same strategy could be used for components, and would allow an end-user to create highly custom rollback procedures based on their exact needs. For example, they may have a resource which stores a large amount of cached pre-computed values which don't actually need to be saved for rollback, but does have some elements that must be saved. They can simply create a pair of systems to save/load that custom resource from their own storage device.

If you think this is an interesting direction to move bevy_ggrs into, let me know and I'll polish up my branch and expand on the proof of concept for a pull request.

johanhelsing

I did a quick read-through and jotted down my thoughts a while back, but forgot to send it, but here it is

johanhelsing · 2023-04-12T14:15:57Z

examples/box_game/box_game.rs

@@ -57,23 +60,34 @@ pub struct FrameCount {
    pub frame: u32,
 }

-pub fn input(_handle: In<PlayerHandle>, keyboard_input: Res<Input<KeyCode>>) -> BoxInput {
-    let mut input: u8 = 0;
+pub fn read_local_inputs(


I think I like how this method changed 👍

johanhelsing · 2023-04-12T14:17:48Z

examples/box_game/box_game_p2p.rs

            ..default()
        }))
-        .add_startup_system(setup_system)
+        .add_plugin(GgrsPlugin::<GGRSConfig>::default())


Also nice to see it be a normal plugin :)

johanhelsing · 2023-04-12T14:19:08Z

examples/box_game/box_game_p2p.rs

+        // set the FPS we want our rollback schedule to run with
+        .set_rollback_schedule_fps(FPS)


Maybe this should be a field on the plugin instead? Or any reason not to?

johanhelsing · 2023-04-12T14:23:22Z

examples/box_game/box_game_p2p.rs

+        // register types we want saved and loaded when rollbacking
+        .register_rollback_component::<Transform>()
+        .register_rollback_component::<Velocity>()
+        .register_rollback_resource::<FrameCount>()


A thought that occurred to me. Maybe it makes sense to have a .init_rollback_resource::<Resource>() and .insert_rollback_resource(resource) instead that also registers? To lessen the boiler-plate somewhat and make it harder to forget it?

johanhelsing · 2023-04-12T14:25:26Z

src/lib.rs

-    }
+/// Keeps track of the current frame the rollback simulation is in
+#[derive(Resource)]
+pub struct RollbackFrameCount(i32);


Having this public will be really nice for writing plugins that integrate with bevy_ggrs (I might make an sfx plugin for instance)

Any reason it's an i32? wouldn't an unsigned type be more appropriate?

johanhelsing · 2023-04-12T14:26:15Z

src/lib.rs

+    SyncTestSession(SyncTestSession<T>),
+    P2PSession(P2PSession<T>),
+    SpectatorSession(SpectatorSession<T>),


Suggested change

SyncTestSession(SyncTestSession<T>),

P2PSession(P2PSession<T>),

SpectatorSession(SpectatorSession<T>),

SyncTest(SyncTestSession<T>),

P2P(P2PSession<T>),

Spectator(SpectatorSession<T>),

As recommended by rust api guidelines

johanhelsing · 2023-04-21T07:43:23Z

I haven't looked to closely at your branch, @bushrat011899 , but I really like the idea that reflection snapshotting is just a special-case of a generic snapshotting mechanism!

I guess then component type registrations will be move to the the reflection rollback plugin instead, which I think feels a lot more appropriate (and clear).

Save and load schedules also seem like a good idea.

johanhelsing · 2023-09-15T10:10:22Z

I find myself wanting many of the features in this PR... How is your capacity @gschup? Would it perhaps be better to just continue development in incremental bits, and not do the full rewrite?

gschup · 2023-09-15T10:57:07Z

I have been out of the loop for quite a bit now. I have been pursuing other hobbies in my free time. This branch is kind of obsolete now that we are already another bevy version ahead.

Please give me until the end of next week to get back into the code and try to finish this up or restart a new "rewrite" from the newest commit. I was pretty close to get this to work before my thesis got in the way :)

johanhelsing · 2023-09-17T14:26:28Z

I'd really like to get some variant of what @bushrat011899 was proposing in, toying with API ideas, I think the following would be possible:

A)

        .add_plugins((
            RollbackPlugin.for_component::<Player>().by_cloning(),
            RollbackPlugin.for_component::<BulletReady>().by_cloning().with_hash_checksum(), // callable on components implementing Hash
            RollbackPlugin.for_component::<Transform>().by_cloning(),
            RollbackPlugin.for_component::<Parent>().by_cloning().with_entity_mapping(),
            RollbackPlugin.for_component::<ForeignType>().by_reflecting(), // things not implementing clone etc.
            RollbackPlugin.for_resource::<Scores>().by_cloning().with_hash_checksum(),
        ))

or perhaps simpler/more explicit is better?

B)

        .add_plugins((
            CloneComponentSnapshotPlugin::<Player>::default(),
            CloneComponentSnapshotPlugin::<BulletReady>::default()
            HashComponentChecksumPlugin::<BulletReady>::default(),
            CloneComponentSnapshotPlugin::<Transform>::default()
            CloneComponentSnapshotPlugin::<Parent>::default(),
            ComponentEntityMappingPlugin::<Parent>::default(),
            ReflectComponentRollbackPlugin::<ForeignType>::default(), // if type reflects hash, use it for checksumming, if it reflects MapEntities, map entities
            CloneResourceSnapshotPlugin::<Scores>::default(),
            HashResourceChecksumPlugin::<Scores>::default(),
        ))

Thoughts?

gschup · 2023-10-10T11:34:06Z

We shifted strategy and now aim to pursue the goals of this outdated rewrite via individual, smaller commits.

foundation for plugin rewrite

b9b8eb0

gschup marked this pull request as draft March 12, 2023 08:04

johanhelsing mentioned this pull request Mar 12, 2023

Add rollback systems with app.add_system(system.in_schedule(GGRSSchedule)) #51

Merged

first draft of exclusive ggrs system

e4241ef

donedgardo mentioned this pull request Mar 12, 2023

Add bevy integration tests johanhelsing/bevy_ggrs#2

Merged

Add integrations test with Input integration started

2fd69a7

Merge pull request #53 from donedgardo/bevy10_rewrite

980a604

Add integration test and continue bevy 0.10 rewrite with Input system.

johanhelsing mentioned this pull request Mar 15, 2023

Update bevy_ggrs example for Bevy 0.10 johanhelsing/matchbox#130

Closed

donedgardo reviewed Mar 18, 2023

View reviewed changes

johanhelsing reviewed Mar 18, 2023

View reviewed changes

garryod mentioned this pull request Mar 25, 2023

Add a Bevy plugin johanhelsing/matchbox#157

Merged

reintroduce reflect-based snapshot setup

d7723d5

gschup changed the title ~~Bevy 0.10 - Complete Plugin Rewrite~~ Bevy 0.10 - Plugin Rewrite Mar 30, 2023

gschup added 2 commits March 30, 2023 21:49

save and load functionality restored (reflect-based), p2p example run…

5eee19e

…s again

reintroduce spectator and synctest example, synctest broken

f14729f

johanhelsing reviewed Apr 21, 2023

View reviewed changes

johanhelsing mentioned this pull request Sep 28, 2023

Make GgrsPlugin a regular Bevy plugin #64

Merged

gschup closed this Oct 10, 2023

johanhelsing mentioned this pull request Oct 15, 2023

Add SaveWorld/LoadWorld schedules #66

Merged

bushrat011899 mentioned this pull request Oct 19, 2023

Refactor into Modular Plugins #76

Merged

4 tasks

gschup deleted the bevy10_rewrite branch October 26, 2023 05:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bevy 0.10 - Plugin Rewrite #52

Bevy 0.10 - Plugin Rewrite #52

gschup commented Mar 12, 2023 •

edited

Loading

johanhelsing commented Mar 12, 2023

donedgardo commented Mar 12, 2023 •

edited

Loading

johanhelsing commented Mar 12, 2023

gschup commented Mar 12, 2023

johanhelsing commented Mar 14, 2023 •

edited

Loading

donedgardo Mar 18, 2023

johanhelsing Mar 18, 2023

donedgardo Mar 18, 2023 •

edited

Loading

johanhelsing Mar 18, 2023

donedgardo Mar 18, 2023

johanhelsing Mar 19, 2023 •

edited

Loading

gschup commented Mar 30, 2023

gschup commented Mar 31, 2023

bushrat011899 commented Apr 21, 2023

johanhelsing left a comment

johanhelsing Apr 12, 2023

johanhelsing Apr 12, 2023

johanhelsing Apr 12, 2023

johanhelsing Apr 12, 2023 •

edited

Loading

johanhelsing Apr 12, 2023

johanhelsing Apr 12, 2023

johanhelsing commented Apr 21, 2023 •

edited

Loading

johanhelsing commented Sep 15, 2023

gschup commented Sep 15, 2023 •

edited

Loading

johanhelsing commented Sep 17, 2023

gschup commented Oct 10, 2023

		// set the FPS we want our rollback schedule to run with
		.set_rollback_schedule_fps(FPS)

Bevy 0.10 - Plugin Rewrite #52

Bevy 0.10 - Plugin Rewrite #52

Conversation

gschup commented Mar 12, 2023 • edited Loading

johanhelsing commented Mar 12, 2023

donedgardo commented Mar 12, 2023 • edited Loading

johanhelsing commented Mar 12, 2023

gschup commented Mar 12, 2023

johanhelsing commented Mar 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

donedgardo Mar 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johanhelsing Mar 19, 2023 • edited Loading

Choose a reason for hiding this comment

gschup commented Mar 30, 2023

gschup commented Mar 31, 2023

bushrat011899 commented Apr 21, 2023

johanhelsing left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johanhelsing Apr 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johanhelsing commented Apr 21, 2023 • edited Loading

johanhelsing commented Sep 15, 2023

gschup commented Sep 15, 2023 • edited Loading

johanhelsing commented Sep 17, 2023

gschup commented Oct 10, 2023

gschup commented Mar 12, 2023 •

edited

Loading

donedgardo commented Mar 12, 2023 •

edited

Loading

johanhelsing commented Mar 14, 2023 •

edited

Loading

donedgardo Mar 18, 2023 •

edited

Loading

johanhelsing Mar 19, 2023 •

edited

Loading

johanhelsing Apr 12, 2023 •

edited

Loading

johanhelsing commented Apr 21, 2023 •

edited

Loading

gschup commented Sep 15, 2023 •

edited

Loading