Skip to content
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.

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

Improve bevy ECS integration #14

Open
ErnWong opened this issue Sep 25, 2021 · 19 comments
Open

Improve bevy ECS integration #14

ErnWong opened this issue Sep 25, 2021 · 19 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@ErnWong
Copy link
Owner

ErnWong commented Sep 25, 2021

Right now, if we were to use the provided bevy plugin, we would need to write most of the game logic outside of bevy's ECS. It would be good to integrate crystalorb with bevy's ECS.

See other similar plugins for examples

Also investigate any new or upcoming bevy ECS features that would make the plugin more ergonomic.

@ErnWong ErnWong added the help wanted Extra attention is needed label Sep 25, 2021
@ErnWong ErnWong changed the title Improve bevy plugin Improve bevy ECS integration Sep 25, 2021
@ErnWong ErnWong added the enhancement New feature or request label Sep 25, 2021
@Shatur
Copy link
Contributor

Shatur commented Oct 1, 2021

Yes, It would be great to decouple world somehow. Because right now I should put all my networking logic into a single data structure.

@Shatur
Copy link
Contributor

Shatur commented Oct 2, 2021

@ErnWong, do you have any design comes to mind? I would like to help.

@ErnWong
Copy link
Owner Author

ErnWong commented Oct 6, 2021

Thanks for your interest! I've got some ideas that might be cool for us to try out. I'll see if I can do bit of investigation this weekend to flesh them out and will get back to you. (Feel free to ping me if I don't get back).

Ideally, it'll be nice to see if we can leave the core CrystalOrb crate more or less the same, and have the changes mostly reside on our bevy plugin crate side of things. CrystalOrb is designed around simulating two instances of the World at any given moment, which, if I understand correctly, is quite different to how backroll and ggrs does it, so it may be cleaner to structure our bevy integration approach differently to theirs.

The Bevy Sandwich Idea

I might as well share what I have in mind, but feel free to suggest other ideas too!

If we went with CrystalOrb's current style of doing things, then we might try and implement the crystalorb::world::World trait for an entire bevy_app::App (or bevy_ecs::world::World) and we'll end up with a delicious bevy sandwich (maybe we should call the plugin bevy_sandwich 😂):

  • We have an outer bevy_app::App, consisting of
    • e.g. the users' game rendering systems
    • other game logic systems that doesn't need to be networked
    • entities and components to be displayed to the screen
    • the bevy_crystalorb plugin, which supplies:
      • a crystalorb::client::Client<bevy_crystalorb::World> bevy resource, which:
        • internally spawns two instances of bevy_crystalorb::World, each of which consists of:
          • an inner bevy_app::App, which consists of
            • entities and components to be networked
            • update systems and setup systems for running your networked game simulation logic
      • a bevy_crystalorb::update_system, which invokes crystalorb::client::Client::update and syncs crystalorb::client::stage::Ready::display_state with the outer bevy app's components.

The game developer would probably set up their game code the usual way, and tag/register their components and systems that they want CrystalOrb to network. Then, the CrystalOrb bevy plugin would hook the inner bevy apps to the correct components and systems, and act as a bridge between the inner and outer bevy apps.

Hmm, that might be a bit confusing. Maybe I should try and draw it out:

image

Yikes, that looks even more confusing. This approach might sound a bit heavy-weight on paper, but so far, CrystalOrb's design decisions has been based around choosing the "cleaner" option over a more performant option. ("Clean" is very subjective though 😛 )

Bevy Subworlds

Having bevy apps inside a bevy app sounds a bit like the bevy subworlds RFC (bevyengine/rfcs#16). I haven't explored that RFC in too much detail, but I'm not sure if it'll be easy to make our ownership hierarchy clean using that approach (since I'm guessing we'll need some sort of circular reference between bevy and our crystalorb client?).

@Shatur
Copy link
Contributor

Shatur commented Oct 6, 2021

Sounds interesting :) But why we need to spawn two worlds?

You may also find it interesting that the Naia network engine is also trying to implement integration for Bevy: naia-lib/naia#22
I thought you might be interested in looking at this implementation. I talked a bit with the author on Discord, he plans to complete the integration in a few weeks. Naia advantage is that it doesn't need a nightly compiler.

@ErnWong
Copy link
Owner Author

ErnWong commented Oct 6, 2021

We simulate two worlds because whenever we want to perform a rollback:

  1. It can take multiple render frames to fast forward the world back to the current timestamp, so we need a second world to simulate and display during this time.
  2. Once rollback and fast-forward has completed, we don't want to abruptly show the corrected world to the player or it will be very noticeable. Instead, we want to gradually transition into the corrected world state, so we'd need to keep both worlds simulating and linearly interpolate the display state between the two worlds.

At least, that's the approach taken by CrystalOrb. It's like operating in some kind of higher level "World Algebra" where we treat the world as a black box. (I'm making these terms up btw)

@Shatur
Copy link
Contributor

Shatur commented Oct 6, 2021

We simulate two worlds because whenever we want to perform a rollback:

Got it, sounds reasonable!

@ErnWong
Copy link
Owner Author

ErnWong commented Oct 7, 2021

Hmm, looking at the top right quadrant of the diagram, using an outer bevy stage to mark which systems we want to run in the inner bevy app might not be a good idea:

  1. Low impact: Extra things we need to pass down
  2. Medium impact: Misleading design because the user would register systems in the outer bevy app even though they're only run in the inner bevy apps.
  3. High-ish impact: Users can't use other bevy plugins (like bevy_rapier) and incorperate it into their networked simulations because the user doesn't have control over what stage these plugins puts their systems into. It is also restrictive that we're only assuming that the user's custom systems should all fit into a single stage.

Rather than give the illusion that the user is adding systems to the outer bevy app, it might be better to expose the inner bevy app directly to the user to add things into, or accept an app bundle (or a set of app bundles) [EDIT: typo, should be PluginGroup or set of PluginGroups] that we use to initialise the inner bevy app.

@jamescarterbell
Copy link
Contributor

For anyone who comes across this, I'm currently working on a bevy plugin that makes the bevy sandwich.

https://github.com/jamescarterbell/crystalorb/tree/feature/bevy_plugin

@ErnWong
Copy link
Owner Author

ErnWong commented Oct 28, 2021

@jamescarterbell
Continuing on a discord discussion, if I understand correctly, there are two open questions. (There are probably more to come, but here are two for now).

  1. How do we pass in the necessary information to initialise the inner bevy worlds?
  2. How do we add or refer to the desired network resources?

I'll throw in some ideas - see if they help.

Regarding 1. Initialising the Client's inner bevy worlds

Initialising the Server is easy because it only needs on instance, but the Client requires two instances.

This might not be an exhaustive list, but I can think of two approaches:

Option A: Passing in a bevy app as a blueprint

The game developer passes in a bevy app when instantiating a BevyCrystalOrbClientPlugin, and we use this bevy app as a blueprint for creating our two instances of the inner bevy apps.

I'm not sure how easy it is to clone the bevy app this way. Systems might be fine, but it might not be possible for resources and components? (Feel free to disprove me - I'm not too familiar with the inner workings of bevy to answer this).

Option B: Passing in some sort of "recipe" for initialising the bevy app

The game developer passes in some sort of "recipe" for setting up the bevy app. For example, this "recipe" could be a closure, a struct implementing some sort of factory trait.

Bevy's Plugin trait is a good example, and I think it might be a good fit for our purposes because in my previous bevy game, I've already wrapped all my game systems in a plugin for organisational purposes anyway (I'm not a game dev though so I can't back up my claim that it's a good idea).

One possible downside of using Bevy's Plugin trait is that Bevy might want to go for a more structured Plugin management system in the future that makes it unsuitable for us, but when that time comes, it should be easy to swap out their Plugin trait with our own InnerWorldFactory trait.

The crystalorb client/server currently initialises their World/Worlds using the World's Default::default implementation, but that might be too restrictive. I'm happy to change that to something a little more flexible, for example, maybe something that takes a world factory lambda:

//////////////////////////////////////////////////////////////////////////////
// Dummy representation of a modified client.rs from the core crate:

// W no longer needs to impl Default
pub struct Client<W>(W, W);

impl<W> Client<W> {
    pub fn new<WorldFactory:Fn() -> W>(world_factory: WorldFactory) -> Self {
        // I can then create as many worlds as I like!
        let world1 = world_factory();
        let world2 = world_factory();
        Self(world1, world2)
    }
    
    // Rename the existing Client::new to Client::new_with_default_world
    pub fn new_with_default_world() -> Self
    where
        W: Default,
    {
        // Convenient but not necessary - what existing Client::new API does at the moment.
        let world1 = Default::default();
        let world2 = Default::default();
        Self(world1, world2)
    }
}

I hacked together a small proof of concept as an example: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=261239bf8a121a0bb37669661300b76c

But feel free to modify it to suit what you're doing, or completely disregard it if it doesn't play well with your code.

Regarding 2. Referring to the desired network resources?

I'll get back to you about this question later... 😄

@jamescarterbell
Copy link
Contributor

So there's two big issues at play:

  1. Schedules are non trivial to clone properly (and it may just be impossible)
  2. The builder idea is fairly unergonomic.

The core issue is that bevy schedules can't run on multiple worlds because they do some caching stuff apparently, so because of that we need two schedules and dedicated worlds for each schedule, which makes it hard to integrate with the two world architecture of crystal orb.

@ErnWong
Copy link
Owner Author

ErnWong commented Oct 28, 2021

Hmm, good point. The builder idea might not be the most ergonomic, although I'm not too sure I'm fully aware of why that is and would be curious to understand it better to help make better decisions.

I'm guessing you're referring to the extra boilerplate code needed to wrap around the systems we want for the inner worlds? (Which I agree is a valid concern btw). Would you happen to know if there are other concerns about ergonomic other than the extra boilerplate? E.g. Perhaps, does it restrict the game developer from using certain game designs? Perhaps, does it prevent certain existing games from being easily ported over to use crystalorb?

@jamescarterbell
Copy link
Contributor

jamescarterbell commented Oct 28, 2021 via email

@ErnWong
Copy link
Owner Author

ErnWong commented Oct 29, 2021

"most plugins people write for their game will have an inner world plugin and an outer world plugin"

Just double checking - did you mean systems rather than plugins? (since if the game developer has already organised their game into plugins, they could pass in a plugin or group of plugins into our crystalorb plugin to initialise the inner world without having to wrap it inside some special builder function, and we'll be sweet!).

I think using some sort of factory or Plugin brings across the connotation that multiple instances of the inner world are being created, which I'm hoping might help get the right mental model across, but I can see how that might be unintuitive at first if people assume that there should only be one instance of the inner world... 🤷 The name "Plugin" might also add to the confusion, but we can rename it with an alias if we want to.

Interestingly, the bevy getting-started book uses Plugins as a way of organising game code. Looking at some bevy games on the bevy assets page, I see several games (but not all games) also use plugins to organise their code. Maybe using plugins to separate the inner and outer game code is not as confusing as we think it is and might even be bevy-idiomatic? (Might be wrong - feel free to say otherwise)

"how often it's run"

I think it only runs twice and only on startup? Unless I misunderstood 😛

Regarding 2. Referring to the desired network resources

Might not be the most beautiful solution (although I wouldn't mind it 🤠 #notbiasedtowardsmyownideasatalliswear), but a potential solution nonetheless that we could add to our brainstorm of possible solutions:

// Some trait that packages all the necessary information to query and wrap the bevy resource into crystalorb's network resource:
pub trait SomethingLikeANetworkResourceBridge<'a> {
    type BevyResource;
    type CrystalOrbNetworkResource: crystalorb::network_resource::NetworkResource;
    fn into_crystalorb_network_resource(bevy_resource: &'a mut Self::BevyResource) -> Self::CrystalOrbNetworkResource;
}
// E.g. in crystalorb-bevy-networking-turbulence
impl SomethingLikeANetworkResourceBridge for crystalorb_bevy_networking_turbulence::WrappedNetworkResource<'a> {
   type BevyResource = bevy_networking_turbulence::NetworkResource;
   type CrystalOrbNetworkResource: Self;
   fn into_crystalorb_network_resource(bevy_resource: &'a mut Self::BevyResource) -> Self::CrystalOrbNetworkResource {
     Self(bevy_resource)
   }
}

// Then... in crystalorb-bevy
pub struct BevyCrystalOrbClientPlugin<T: SomethingLikeANetworkResourceBridge, ...>{
    // ...
}
// ...
fn client_update<T: SomethingLikeANetworkResourceBridge, ...>(mut client: ResMut<crystalorb::client::Client<...>>, mut network_resource: ResMut<T::BevyResource>, time: Res<Time>){
    client.update(time.delta_seconds_f64(), time.seconds_since_startup(), T::into_crystalorb_network_resource(network_resource.deref_mut()));
}

(Haven't tested it)
Of course, feel free to suggest other ideas!

@ErnWong
Copy link
Owner Author

ErnWong commented Oct 29, 2021

Regarding 2. Referring to the desired network resources - Continued

Might be nicer to have a conversion trait impl directly on the bevy resource, and register that bevy resource through the crystalorb plugin so that rust can deduce the types without writing out the generic parameters. (This is also an untested claim).

pub trait IntoCrystalOrbNetworkResource {
    type CrystalOrbNetworkResource: crystalorb::network_resource::NetworkResource;
    fn into_crystalorb_network_resource(bevy_resource: &'a mut Self::BevyResource) -> Self::CrystalOrbNetworkResource;
}

impl IntoCrystalOrbNetworkResource for bevy_networking_turbulence::NetworkResource {
   type CrystalOrbNetworkResource<'a> = crystalorb_bevy_networking_turbulence::WrappedNetworkResource<'a>;
   fn into_crystalorb_network_resource<'a>(bevy_resource: &'a mut Self) -> Self::CrystalOrbNetworkResource<'a> {
      Self::CrystalOrbNetworkResource(bevy_resource)
   }
}

// Then... in crystalorb-bevy
pub struct BevyCrystalOrbClientPlugin<P: Plugin, N: IntoCrystalOrbNetworkResource>{
    // ...
}

impl<P: Plugin, N: IntoCrystalOrbNetworkResource> BevyCrystalOrbClientPlugin<P, N> {
    pub fn new(inner_plugin: P, network_resource: N) -> Self {
        // ...
    }
}

// ...

fn client_update<N: IntoCrystalOrbNetworkResource, ...>(mut client: ResMut<crystalorb::client::Client<...>>, mut network_resource: ResMut<N>, time: Res<Time>){
    client.update(time.delta_seconds_f64(), time.seconds_since_startup(), N::into_crystalorb_network_resource(network_resource.deref_mut()));
}

We can use the builder pattern to make it look more familiar:

App::build()
    .add_plugin(BevyCrystalOrbClientPlugin::build()
        .add_plugin(inner_plugin)
        .insert_resource(NetworkResource)
        //...
    )
    //...

Wait no, sorry, that won't work :( because we're not usually the one to insert the network resource anyway (it's usually the network plugin that does it).

Also, all of this approach assumes that "Network Resource" corresponds 1-to-1 to a Bevy Resource, which might not be true. Hmm... we'll need to think of something different.

Regarding the idea of using Plugins

I think it only runs twice and only on startup?

Hmm... I wonder if calling .build twice on the same plugin struct would be problematic. I can see it being a problem for actual third-party bevy plugins that the game developer has less control of (like bevy_rapier or bey_networking_turbulence) if those plugins assume it is only built once (for example, if it needs to consume some kind of data in the process of building it). However, this is probably less of a problem for our case since the game developer would have control over the inner_plugin they pass in.

@jamescarterbell
Copy link
Contributor

I actually think I figured out number 2, I was just being dumb, but we'll see soon.

Just double checking - did you mean systems rather than plugins? (since if the game developer has already organised their game into plugins, they could pass in a plugin or group of plugins into our crystalorb plugin to initialise the inner world without having to wrap it inside some special builder function, and we'll be sweet!).

I do mean plugins! Since the inner world and outer world can essentially be thought of as different apps in this model, you really do need two plugins for any plugin that will interact with both the inner and outer worlds, since there's no way to directly access one from the other (they interact via displaystates, and eventually the outer world holds both inner worlds, but before then they're seperate).

I think the builder idea will work, but the only annoying thing is it will mean having to create your app kind of like this:

`
let app_factory = ||{
App::build()
.add_plugin(BevyCrystalOrbPhysicsSimulationPlugin)
.app
}

App::build()
.add_plugin(CrystalOrbClientPlugin::with_factory(app_factory))
`

Then the client plugin can run your builder twice, and take the world and scheduler from that app.

@ErnWong
Copy link
Owner Author

ErnWong commented Oct 30, 2021

I do mean plugins!

Ah, cool!

but the only annoying thing is it will mean having to create your app kind of like this:

I'm assuming we're wrapping it in a app_factory closure because we don't want to call the simulation plugin's .build() twice on the same plugin object? (I'm happy either way - just double checking the reason).

Btw, does this mean we need to refactor crystalorb::client::Client::new() to take in a world factory? (This is because we're currently initialising the world using crystalorb::world::World::default(), which I'm guessing won't play well with our app_factory?). Or, do you have other plans for injecting the plugin into the already created crystalorb::world::World?

Oh, I've got another idea! Rather than have an app_factory closure and refactoring crystalorb::client::Client::new(), we derive(Default) on the InnerSimulationPlugin and pass the type to our CrystalOrbClientPlugin as a generic parameter rather than by value. Then, we can do this in our crystalorb-bevy plugin:

pub struct CrystalOrbWorld<InnerSimulationPlugin: Plugin + Default, ...> {
    world: BevyWorld,
    schedule: Schedule,
    // ...
}
impl<InnerSimulationPlugin: Plugin + Default> Default for CrystalOrbWorld<InnerSimulationPlugin> {
    fn default() -> Self {
        let app = App::build()
            .add_plugin(InnerSimulationPlugin::default())
            .app;
        Self {
            world: app.world,
            schedule: app.schedule,
            // ...
        }
    }
}

This way, I think we could get rid of having the slightly annoying app_factory closure. I think for the game developer's perspective, their game code can become something like this:

#[derive(Default)]
pub struct InnerSimulationPlugin; // Most plugins are an empty struct anyway.

impl Plugin for InnerSimulationPlugin {
    fn build(&mut self, app_builder: AppBuilder) {
        app_builder.add_system(...); // etc...
    }
}

// later on

App::build()
    .add_plugin(OuterPlugin)
    .add_plugin(CrystalOrbClientPlugin::<InnerSimulationPlugin>::new())
    .run();

This does add a bit of restriction on what InnerSimulationPlugin can be, but I'm guessing for most games that won't be a problem.

Sorry I ended up dumping more ideas to you 😅 . Feel free to ignore it if you've already got a plan or what I said doesn't make sense, but feel free to check this idea out if you're stuck and want some inspiration.

@Shatur
Copy link
Contributor

Shatur commented Dec 24, 2021

@jamescarterbell are you planning to continue working on it?

@Shatur
Copy link
Contributor

Shatur commented Jan 17, 2022

@ErnWong are you planning to update your awesome crate to Bevy 0.6?

@ErnWong ErnWong mentioned this issue Jan 18, 2022
@ErnWong
Copy link
Owner Author

ErnWong commented Jan 18, 2022

@Shatur Sure, I can have a look, maybe this weekend. I've created #26 to track this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants