Skip to content

Make default state change trigger self state transitions#21792

Merged
alice-i-cecile merged 12 commits intobevyengine:mainfrom
janhohenheim:naming
Dec 9, 2025
Merged

Make default state change trigger self state transitions#21792
alice-i-cecile merged 12 commits intobevyengine:mainfrom
janhohenheim:naming

Conversation

@janhohenheim
Copy link
Member

@janhohenheim janhohenheim commented Nov 9, 2025

Objective

  • enable same state transitions #19363 finally allows self-state transitions
  • The naming is a bit meh (set_forced)
  • Many people expect this to work out of the box
  • That PR also missed a few spots where this distinction applies

Solution

  • Expand on enable same state transitions #19363 by making it the default
  • Rename set -> set_if_neq
  • Rename set_forced -> set
  • Add the two variants to
    • commands
    • reflection
    • computed states
  • make the transition logs a bit less chatty in the common case
  • add a test for the new behavior

Testing

  • CI

@janhohenheim janhohenheim added M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward A-States App-level states machines labels Nov 9, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2025

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Please review the instructions for writing migration guides, then expand or revise the content in the migration guides directory to reflect your changes.

@alice-i-cecile alice-i-cecile added the X-Contentious There are nontrivial implications that should be thought through label Nov 9, 2025
@alice-i-cecile alice-i-cecile added this to the 0.18 milestone Nov 9, 2025
@alice-i-cecile
Copy link
Member

So the 4 areas of access:
App.insert_state : Always transitions. (could add a insert_state_if_neq but meh)
Commands.set_state/set_state_if_neq: Always transitions, unless you dont set it.
NextState.set/set_if_neq: Always transitions, unless you dont set it.
ComputedStates.compute: Always transitions, unless the const flag makes you not set it.

In most/all these scenerios, the StateTransitionEvent is always doing same-state transitioning if current == next, but only differs in whether or not you actually provide a next == current.
So I think it should be possible to drop the field from the Event and just run the schedules by default, with the API of states supporting this set_if_neq ability.

@mockersf
Copy link
Member

mockersf commented Nov 9, 2025

We shouldn't change the default on only one of the method, they should all behave the same 👍

@hukasu
Copy link
Contributor

hukasu commented Nov 9, 2025

Nit: nowhere in std and bevy "not equal" is written as neq, it is always written as ne

Comment on lines +142 to +144
// Not configurable for the moment. This controls whether inserting a state with the same value as a pre-existing state should run state transitions.
// Leaving it at `true` makes state insertion idempotent. Neat!
allow_same_state_transitions: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confused here, if allow_same_state_transitions is true, then same state transitions are allowed, and the state transition systems should be run? Wouldn't state insertion be idempotent then when allow_same_state_transitions is false?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, since when deactivated, inserting the state without it pre-existing will trigger transitions, and then inserting it a second time won't.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, too sleepy..

@ickshonpe
Copy link
Contributor

ickshonpe commented Nov 10, 2025

Nit: nowhere in std and bevy "not equal" is written as neq, it is always written as ne

Component change detection has a set_is_neq function. It's not idiomatic rust, but I don't see a problem with it. Both APIs should match, so if we want to use ne instead of neq that should be a separate PR anyway I think.

@janhohenheim
Copy link
Member Author

I believe the blocking feedback was based on a misconception. This PR changes the default on all methods, not just on one.

type SourceStates: StateSet;

/// Whether state transition schedules should be run when the state changes to the same value. Default is `true`.
const ALLOW_SAME_STATE_TRANSITIONS: bool = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is unclear to me why this const exists.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did discovery. This exists because of this discord conversation: https://discord.com/channels/691052431525675048/749335865876021248/1437152386345996430

Which is intended to be a computed_states on/off flag for either always or never triggering. In this case the default is "always trigger".

It is used in state/state_set, which is updated in this PR.

@janhohenheim janhohenheim added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Nov 20, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Dec 9, 2025
Merged via the queue into bevyengine:main with commit 3d700c4 Dec 9, 2025
38 checks passed
@janhohenheim janhohenheim deleted the naming branch December 9, 2025 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-States App-level states machines D-Straightforward Simple bug fixes and API improvements, docs, test and examples M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Contentious There are nontrivial implications that should be thought through

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants