bevyengine / bevy

A refreshingly simple data-driven game engine built in Rust
https://bevyengine.org
Apache License 2.0
35.15k stars 3.46k forks source link

Port Specs' SystemData to Bevy #800

Closed mvlabat closed 3 years ago

mvlabat commented 3 years ago

What problem does this solve or what need does it fill?

1. Reducing boilerplate

In specs SystemData solves the problem of re-using sets of resources and components, eliminating the need to copy-paste common storage declarations among different systems, that work with the same components or resources.

2. Attaching implementation to a set of resources or archetypes

Apart from that, SystemData enables attaching implementation to a set of resources. One may argue that attaching functionality to components or resources goes against the ECS paradigm, but I can personally see this useful for implementing helper methods, that could be re-used by many systems.

In the game that I was building on Amethyst I used this pattern to "extend" (more like to compose together) the engine's Time resource with additional data, that I used for counting frames spent playing a level: https://github.com/amethyst/grumpy_visitors/blob/f12b85d0dac07c101b43e83711b89777dc69dd58/libs/core/src/ecs/system_data/time.rs This allowed me to create a useful abstraction, that helped me to conveniently access both game time and engine time from any system.

3. Working around the limit of the number of elements one can pass to a system

I haven't been able to look into Bevy's implementation of passing resources/components into systems yet, but I guess there's a limit of the number of elements one can pass to a system. SystemData can also be used for working this limitation around.

4. Enhances the UX of accessing components

Users would be able to access components like they would access a struct field, without the need to destructure a tuple to get meaningful identifiers.

for attacker in attackers.iter() {
  let damage_dealt = attacker.damage * *attacker.damage_modifier;
}
// instead of
for (damage, damage_modifier) in attackers.iter() {
  let damage_dealt = damage * *damage_modifier;
}
// or
for attacker in attackers.iter() {
  let damage_dealt = attacker.0 * *attacker.1;
}

If a query is ever edited, the first loop is much easier to refactor. For instance, if we add a new component somewhere in the middle, a developer won't need to add a new variable to a destructuring pattern, also keeping in mind the order of components.

Describe the solution would you like?

I believe that we could implement two derive macros: one for resources, one for components. Ideally, I'd imagine using them would look something like this:

#[derive(SystemResources, Clone, Copy)]
pub struct MyResources {
    resource_a: &ResourceA,
    resource_b: &mut ResourceB,
}

#[derive(SystemComponents, Clone, Copy)]
pub struct MyArchetype {
    component_a: &ComponentA,
    component_b: &mut ComponentB,
}

fn for_each_system(my_resources: MyResources, my_archetype: MyArchetype) {
    // ...
}

fn query_system(my_resources: MyResources, my_archetype_query: Query<MyArchetype>) {
    for my_archetype in my_archetype_query.iter() {
        // ...
    }
}

Describe the alternative(s) you've considered?

1. Reducing boilerplate

Speaking of system declarations this can be solved with introducing type aliases for some common queries. Though this solution introduces more cognitive complexity because developers will have to keep the order of components in mind. Introducing new components to a query is more painful to refactor.

2. Attaching implementation to a set of resources or archetypes

Attaching implementation is likely possible with implementing custom traits for tuples of components/resources references, but the UX of this method seems to have obvious drawbacks, that are covered in other sections as well.

3. Working around the limit of the number of elements one can pass to a system

Introducing a feature flag that would enable implementations of necessary traits for larger tuples: (A, B, C, ..., Z).

4. Enhances the UX of accessing components

I don't think there are any, though one can dispute the importance of this. I guess this point can be perceived as one of the positive side-effects if we implement SystemData in Bevy.

Additional context

A couple of weeks ago I made an attempt to implement this for Legion, which resulted in SystemResources derive macro: https://github.com/amethyst/legion/pull/211. This PR enables this pattern only for resources, but not components (hopefully, yet).

mvlabat commented 3 years ago

Oh, I've just realized that's something that #9 and #786 already suggest. Though this issue extends these proposals to resources as well

entropylost commented 3 years ago

I don't think its really necessary to have bundles for resources; resources can already have names as they are function arguments, compaired to queries which don't.

mvlabat commented 3 years ago

@iMplode-nZ that's only one of the use-cases for resource bundles. I agree that there's probably more demand for query bundles, but I don't think that means that resource bundles are useless.

I see this feature most useful in scenarios when there's a resource provided by an engine and you want to extend it with some additional data, which you obviously can't do without making changes to the engine. You can always have just two resources, but that would require passing them both all over the code, and attaching some implementation to them isn't very convenient.

One option would be to introduce a new resource that would encapsulate all data of the original resource and data that you want to add. But this would also require copy-pasting all the engine systems that interact with the resource and adapt them to your new one.

Having a resource bundle pretty much solves all these problems.

This is a real use-case I had in my own game, I think this example is a pretty good demonstration: https://github.com/amethyst/grumpy_visitors/blob/f12b85d0dac07c101b43e83711b89777dc69dd58/libs/core/src/ecs/system_data/time.rs

cart commented 3 years ago

This should be resolved by #798, which will have a SystemParam derive (which will work for resources, queries, querysets, commands, and whatever params we add in the future).

The derive is implemented, but I haven't pushed it yet.

mvlabat commented 3 years ago

@cart wow, this is awesome! The fact that it supports resources and components, and command buffers is even above my expectations.

cart commented 3 years ago

Just merged #798. The #[derive(Query)] issue (#786) handles the query side, so I'm closing this out.