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

Event-dependent run criteria for systems #1272

Closed
alice-i-cecile opened this issue Jan 20, 2021 · 7 comments
Closed

Event-dependent run criteria for systems #1272

alice-i-cecile opened this issue Jan 20, 2021 · 7 comments
Labels
A-ECS Entities, components, systems, and events C-Enhancement A new feature

Comments

@alice-i-cecile
Copy link
Member

alice-i-cecile commented Jan 20, 2021

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

Some systems should only run when there are events to be processed.

This is a common pattern, but annoying

What solution would you like?

A simple run criteria helper function that checks whether an event queue of the specified type is empty.

Because run criteria apply to multiple systems at once, I don't think you can have it automatically inspect the parameters of the systems its modifying.

What alternative(s) have you considered?

Users can write their own run criteria by hand. This is tedious and error-prone.

Event-handling systems can return early if they have no work today. This combines internal logic with scheduling logic, and forces scheduling of a system when it's not needed, possibly blocking other systems' access to archetype-components or resources.

Additional context

This functionality relates to but is distinct from triggered systems, as discussed in #1273. Something morally similar might be helpful as part of a scheduler for a dedicated UI stage.

@tigregalis
Copy link
Contributor

Previous work/discussion that is in a similar vein: #1041

@alice-i-cecile
Copy link
Member Author

I think that the event_system API shown here is the clearest of the available options, and captures the desired functionality well.

You'll want to configure it so then the system runs if any of its event queues are non-empty, to properly handle the case of systems that read more than one event type.

This is mostly relevant at the system level, so losing the ability to generalize it in an ultra-convenient way to a SystemSet or Stage is not very concerning to me. You can always manually write a run-criteria.

The only other caveat is that by expanding the .system() syntax for other use cases, we're limiting our ability to remove it again in the future if we can find appropriate magic to do so.

@Moxinilian Moxinilian added A-ECS Entities, components, systems, and events C-Enhancement A new feature labels Jan 25, 2021
@alice-i-cecile
Copy link
Member Author

A similar pattern emerges for processing component data, where you only want to run a system if a query is non-empty, typically on the basis of Changed.

This is also related to the idea of ChangedRes, discussed in #1186.

Ideally we could handle all this in a single abstraction?

@alice-i-cecile
Copy link
Member Author

alice-i-cecile commented Jan 25, 2021

So to capture the idea from the comment above, what I think we actually want is a run criteria of:

RunCriteria::Processing: the system will run if and only if at least one of the change-tracked resources has been changed, its EventReader buffer has events to process, or a query that contains a Changed (or similar query filter) is non-empty.

The name could use some workshopping, but I think this is an abstraction that captures a lot of patterns. In particular, I think that this could seriously mitigate the need for hard run criteria at all (see the infamous #1144). I think this also requires some thinking about how exactly to handle ChangedRes (see #1313 as well) to really capture well.

Edit: I think this is too magical. Instead, we should have more explicit run criteria helpers based on change detecting queries and event readers.

@alice-i-cecile
Copy link
Member Author

The problem is that System s are opaque, and this data isn't very accessible. The problem also gets worse once nested system sets come into play.
Detecting a query with Changed is messy - it could be interminled with Or statements, or and-ed with a diffrent Changed, or just be arbitrarily deeply nested in with tuples.
ChangedRes is on its way out, and I don't think this is enough motivation to keep it around
With all three of these, you end up special-casing for specific types in a lot of places. I don't think this is the way to go

@TheRawMeatball, Discord

@alice-i-cecile
Copy link
Member Author

Blocked on a better design for run criteria in #2801.

@alice-i-cecile
Copy link
Member Author

Closing this out; I don't think this is the right direction (at least for now).

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-Enhancement A new feature
Projects
None yet
Development

No branches or pull requests

3 participants