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

Port Specs' SystemData to Bevy #800

Closed
vladbat00 opened this issue Nov 6, 2020 · 6 comments
Closed

Port Specs' SystemData to Bevy #800

vladbat00 opened this issue Nov 6, 2020 · 6 comments
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible

Comments

@vladbat00
Copy link
Contributor

vladbat00 commented Nov 6, 2020

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: amethyst/legion#211. This PR enables this pattern only for resources, but not components (hopefully, yet).

@vladbat00 vladbat00 added the C-Feature A new feature, making something new possible label Nov 6, 2020
@vladbat00 vladbat00 changed the title Port specs' SystemData to bevy Port specs' SystemData to Bevy Nov 6, 2020
@vladbat00 vladbat00 changed the title Port specs' SystemData to Bevy Port Specs' SystemData to Bevy Nov 6, 2020
@karroffel karroffel added the A-ECS Entities, components, systems, and events label Nov 6, 2020
@vladbat00
Copy link
Contributor Author

vladbat00 commented Nov 6, 2020

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
Copy link
Contributor

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.

@vladbat00
Copy link
Contributor Author

@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
Copy link
Member

cart commented Nov 7, 2020

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.

@vladbat00
Copy link
Contributor Author

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

@cart
Copy link
Member

cart commented Nov 8, 2020

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

@cart cart closed this as completed Nov 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible
Projects
None yet
Development

No branches or pull requests

4 participants