bevy icon indicating copy to clipboard operation
bevy copied to clipboard

One-shot systems via Commands for scripting-like logic

Open alice-i-cecile opened this issue 4 years ago • 17 comments
trafficstars

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

Some operations are only done very rarely: scripting-like game logic that modifies levels, unlocks achievements or handles a boss's special attack come to mind.

These systems clutter the schedule, waste resources as they constantly spin, and are harder to extend.

What solution would you like?

There are two parts to my proposed solution:

  1. Add Commands::run_system(system: impl Into<SystemDescriptor>). This runs the specified system on the World the next time commands are processed.
  2. Create a pattern where we store references to system functions on components, and dynamically run those systems when the appropriate trigger is met, allowing for scripting-like flexibility with low-overhead and seamless ergonomic Bevy syntax.

What alternative(s) have you considered?

Writing custom commands types each time will be onerous and have much increased boilerplate, and because they take exclusive access to World, force the users to learn a new syntax and reduce our ability to parallelize commands of this sort in the future.

This same functionality could also be framed as "dynamically inserting systems", but that may complicate the API somewhat when you often don't end up caring a lot about overhead.

Other approaches to reactivity may be able to fill this niche in some form instead.

Additional context

Making sure the ordering on this is correct enough is going to be one of the larger challenges. May be better to wait until after #1375. Chaining also needs some careful thought.

Brought up in response to the following user request:

Is it possible / practical / good practice to pass a function around inside an event? My thought process is an NPC could have an interaction with a very specific procedure following it like opening a secret door across the map or unlocking a new dialogue option with another NPC so much so that having a system that contains every one of these niche interactions inside a match would be rather hideous.

Depending on the efficiency and ergonomics of this, could also be used to support push data-flow for UI (see #254).

Related to #1273.

alice-i-cecile avatar May 17 '21 04:05 alice-i-cecile

Create a pattern where we store references to system functions on components [...]

I'm not sure what this entails on Bevy side. A mention in the book(s) and an example?

This same functionality could also be framed as "dynamically inserting systems", but that may complicate the API somewhat when you often don't end up caring a lot about overhead.

I'm not sure what you mean by that. Inserting systems dynamically is not an API problem: it can't be done without forcing the systems executor to recalculate its cached data, which is necessarily slow.

Making sure the ordering on this is correct enough is going to be one of the larger challenges. May be better to wait until after #1375.

Commands' order is entirely dependant on the order of the systems that issue them. This reads like proposing we add another layer of ordering on top of that, which is bound to get ugly. Stageless won't change anything here.

Chaining also needs some careful thought.

Why? Chaining two systems is essentially making them into one system, no special considerations needed.


I don't think there are any obstacles for a built-in arbitrary closure command (if we don't have one already). Dressing said closure up as a system shouldn't be a challenge, at first glance, but: systems are stateful - they have their own local variables. How should that be handled here?

This might be once again us running into the trap of trying to jam everything and anything into our system abstraction.

I think it's best to instead consider the accessor API for this. It's semantically more correct: a thing that's not a normally scheduled system but needs potentially-shared access to world data.

Ratysz avatar May 17 '21 15:05 Ratysz

I think pre-registering a system with the executor, retrieving a handle from that, then triggering a run of the system via commands (or some other abstraction) is probably the way to go if we want to support this.

It definitely has synergies with "reactivity", which entails running a pre-defined system on a trigger. The only difference is "manual trigger" vs "data access trigger".

The executor is already "greedy" (it just loops over systems that need to run and sees if they can run), so this feels like a natural extension of that. Reactions and manual system triggers should both probably run before the source system's dependencies run, so I think its probably just a matter of only flipping the "finished" state for the source system once all triggered systems have finished.

cart avatar May 18 '21 00:05 cart

I think pre-registering a system with the executor, retrieving a handle from that, then triggering a run of the system via commands (or some other abstraction) is probably the way to go if we want to support this.

I'm on board with this strategy: should be quite clean and support the scripting-like use cases very nicely. As long as we can store the handles on components we should be good.

alice-i-cecile avatar May 18 '21 00:05 alice-i-cecile

I'm not sure what this entails on Bevy side. A mention in the book(s) and an example?

Yes, likely as part of an example game of some sort plus a page in the book. This sort of design pattern is really important to communicate IMO, but very challenging to understand without context.

alice-i-cecile avatar May 18 '21 00:05 alice-i-cecile

One other useful bit of functionality that would be helpful: the ability to pass in new data to the system via the Input type on system chaining. This should probably be split from the default no-input form for ergonomic reasons.

alice-i-cecile avatar May 19 '21 04:05 alice-i-cecile

That's not what chaining is for. We don't have a centralized way to pipe data directly between systems (like in e.g. run criteria), and I don't think we need one - events and other kinds of channels should work just fine.

Ratysz avatar May 19 '21 17:05 Ratysz

there's also the config method on FunctionSystem that can be used to setup locals

mockersf avatar May 19 '21 18:05 mockersf

For posterity, my current thinking is that this pattern is less than optimal. It creates a lot of spaghetti in terms of dependencies and ordering, and has a serious but largely hidden performance impact. It's also very painful to refactor away from.

Instead, I think there are two, better alternatives:

  • create a well-scoped event type, which stores a FnMut with the appropriate data types, and then process it in its own system. Possibly in combination with #2070.
  • work to dynamically add-and-remove systems based on world state in order to disable specialized scripting systems when they're not needed. Blocked on #279, and by extension #2801.

alice-i-cecile avatar Dec 20 '21 22:12 alice-i-cecile

To add to this: I think we should have a way to queue runs of systems (in the same stage as the queuing system or some future stage) via their labels. This is slightly different than disabling and enabling at runtime, because queuing twice should result in two runs of the system.

cart avatar Dec 20 '21 22:12 cart

Here's an approach to a one-shot style scheme.

https://gist.github.com/hlb8122/33ef96079aa3ba9d5cbd3611f3e4e27c

hlb8122 avatar Jan 23 '22 15:01 hlb8122

Reopening this: we need something along these lines still, and this is a nice discussion of what will and will not work.

alice-i-cecile avatar Jan 27 '22 19:01 alice-i-cecile

I was discussing a workaround with a user today.

  1. Store a HashMap<MyLabel, BoxedSystem> in a resource.
  2. Add systems to this resource, initializing them when they're added.
  3. Use World::resource_scope to run these in an exclusive system.

This is (somewhat) ergonomic, and avoids the reinitialization pain. #2777 would make this pattern even cleaner.

alice-i-cecile avatar Feb 16 '22 18:02 alice-i-cecile

@alice-i-cecile, I've been thinking about infrequent events and ECS for a while now and every time I dig deeper I find you've already been there (both here and on Reddit 😅). Given that you've changed your stance on the topic over the months I'd like to know what is your current thinking. Do you still find this a non-optimal pattern? Do you have thoughts on the direction to follow after closing #4090?

I believe this pattern fills some legitimate needs not well supported by classic ECS, but I find it hard to implement without creating confusion around when it should be used or avoided. Even after making a proposal to have ergonomic system params in commands, I still fear an easy-to-use API that completely defeats the parallelism of ECS.

With an ergonomic command API I'm tempted to use them for a bunch of things. Some could be handled with events, but the constant polling for infrequent actions and the explosion of EventWriters that a system might need for very divergent logic just don't feel right. Others could be handled with simple function calls, but in complex systems that forces me to add a lot of boilerplate to fetch the params on the outer system and to pass them to the inner functions.

But I know that by relying too heavily on them I'm bypassing schedules and all the good parallelization stuff. I've been thinking of ways of mitigating this command drawback but it clearly is a very complex topic and so far I've only reached dead ends.

Pascualex avatar Mar 08 '23 18:03 Pascualex

I think that this is an important pattern: polling isn't a good fit for everything.

I think that redoing #4090 on the new scheduler should be straightforward and valuable. I also think we can make them very fast. I just closed out that PR as the scheduling internals had / were about to change too dramatically. If you'd like, I'd be happy to mentor you on redoing the PR.

alice-i-cecile avatar Mar 08 '23 18:03 alice-i-cecile

That sounds great, I'll start tomorrow with the redo. I'll start with the same approach as #4090, but I'd like to discuss #7707 (mainly my last comment) before merging. Using commands instead of labels opens the possibility of passing arguments to the one-shot systems and they require almost none additional boilerplate.

Pascualex avatar Mar 08 '23 21:03 Pascualex

I'd be okay to have them take basically an impl System object, and then cache if and only if we can :) I don't think that should use commands though: there's no need for &mut World in most cases.

alice-i-cecile avatar Mar 08 '23 21:03 alice-i-cecile

I'll first redo #4090 to fully understand your proposal and then reevaluate my reasoning for #7707.

Pascualex avatar Mar 08 '23 22:03 Pascualex