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

allow ordering systems registered in computed and source state transition events #17708

Open
bananaturtlesandwich opened this issue Feb 6, 2025 · 0 comments
Labels
C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled

Comments

@bananaturtlesandwich
Copy link

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

  • If a source state changes a computed state, transition events for both transitions will be triggered
  • however the order in which they trigger cannot be configured
  • therefore transition systems which should run after each other but trigger in different transitions which can happen at the same time don't work half the time

example

i have a source state containing jump and fall which when computed give the computed state air variant (as opposed to ground)
(transitions between jump and fall variants and any other new air states don't trigger transitions in the computed state which is great)
on entering the air state i set air velocity based on ground velocity (ground velocity has no y component) and on jumping i skew this based on the ground normal

app.add_systems(OnEnter(T::Air), air::transfer).add_systems(OnEnter(M::Jump), jump::skew)
  • skewing should act on the air velocity after setting so should run after
  • but arbitrarily it runs before because the schedule labels are different
  • system ordering e.g .before doesn't work here because they're in different schedule labels

What solution would you like?

The OnEnter, OnExit and OnTransition types should only indicate how the systems are triggered not the order they are triggered in
I think unifying all state transition events under one label and using run conditions like we already do with states would be the best solution
so something like

app.add_systems(Transition, (air::transfer.run_if(transition(OnEnter(T::Air)), jump::skew.run_if(transition(OnEnter(M::Jump))).chain())

or

app.add_systems(Transition, (air::transfer.run_if(entering(T::Air)), jump::skew.run_if(entering(M::Jump))).chain())

although the run condition is schedule specific which might be confusing not sure on the stance for that so maybe

app.add_transitions((OnEnter::new(T::Air, air::transfer), OnEnter::new(M::Jump, jump::skew)).chain())

which adds the run condition under the hood - there's probably plenty of alternate solutions

What alternative(s) have you considered?

in this case this works but also defeats the point of the state infrastructure

 .add_systems(OnEnter(T::Air), (air::transfer, jump::skew.run_if(in_state(M::Jump))).chain())

and in more complex cases this will continue to grow more convoluted

@bananaturtlesandwich bananaturtlesandwich added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled
Projects
None yet
Development

No branches or pull requests

1 participant