Skip to content
12 changes: 7 additions & 5 deletions crates/bevy_dev_tools/src/states.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@ pub fn log_transitions<S: States>(mut transitions: MessageReader<StateTransition
let StateTransitionEvent {
exited,
entered,
same_state_enforced,
allow_same_state_transitions,
} = transition;
info!(
"{} transition: {:?} => {:?} | same state enforced: {:?}",
name, exited, entered, same_state_enforced
);
let skip_text = if exited == entered && !*allow_same_state_transitions {
" (disallowing same-state transitions)"
} else {
""
};
info!("{name} transition: {exited:?} => {entered:?}{skip_text}");
}
15 changes: 10 additions & 5 deletions crates/bevy_state/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ impl AppExtStates for SubApp {
self.world_mut().write_message(StateTransitionEvent {
exited: None,
entered: Some(state),
same_state_enforced: false,
// makes no difference: the state didn't exist before anyways
allow_same_state_transitions: true,
});
enable_state_scoped_entities::<S>(self);
} else {
Expand All @@ -125,7 +126,8 @@ impl AppExtStates for SubApp {
self.world_mut().write_message(StateTransitionEvent {
exited: None,
entered: Some(state),
same_state_enforced: false,
// makes no difference: the state didn't exist before anyways
allow_same_state_transitions: true,
});
enable_state_scoped_entities::<S>(self);
} else {
Expand All @@ -137,7 +139,9 @@ impl AppExtStates for SubApp {
self.world_mut().write_message(StateTransitionEvent {
exited: None,
entered: Some(state),
same_state_enforced: false,
// 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,
Comment on lines +142 to +144
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..

});
}

Expand All @@ -162,7 +166,7 @@ impl AppExtStates for SubApp {
self.world_mut().write_message(StateTransitionEvent {
exited: None,
entered: state,
same_state_enforced: false,
allow_same_state_transitions: S::ALLOW_SAME_STATE_TRANSITIONS,
});
enable_state_scoped_entities::<S>(self);
} else {
Expand Down Expand Up @@ -192,7 +196,8 @@ impl AppExtStates for SubApp {
self.world_mut().write_message(StateTransitionEvent {
exited: None,
entered: state,
same_state_enforced: false,
// makes no difference: the state didn't exist before anyways
allow_same_state_transitions: true,
});
enable_state_scoped_entities::<S>(self);
} else {
Expand Down
25 changes: 22 additions & 3 deletions crates/bevy_state/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,37 @@ pub trait CommandsStatesExt {
/// Note that commands introduce sync points to the ECS schedule, so modifying `NextState`
/// directly may be more efficient depending on your use-case.
fn set_state<S: FreelyMutableState>(&mut self, state: S);

/// Sets the next state the app should move to, skipping any state transitions if the next state is the same as the current state.
///
/// Internally this schedules a command that updates the [`NextState<S>`](crate::prelude::NextState)
/// resource with `state`.
///
/// Note that commands introduce sync points to the ECS schedule, so modifying `NextState`
/// directly may be more efficient depending on your use-case.
fn set_state_if_neq<S: FreelyMutableState>(&mut self, state: S);
}

impl CommandsStatesExt for Commands<'_, '_> {
fn set_state<S: FreelyMutableState>(&mut self, state: S) {
self.queue(move |w: &mut World| {
let mut next = w.resource_mut::<NextState<S>>();
if let NextState::Pending(prev) = &*next
&& *prev != state
{
if let NextState::PendingIfNeq(prev) = &*next {
debug!("overwriting next state {prev:?} with {state:?}");
}
next.set(state);
});
}

fn set_state_if_neq<S: FreelyMutableState>(&mut self, state: S) {
self.queue(move |w: &mut World| {
let mut next = w.resource_mut::<NextState<S>>();
if let NextState::PendingIfNeq(prev) = &*next
&& *prev != state
{
debug!("overwriting next state {prev:?} with {state:?} if not equal");
}
next.set_if_neq(state);
});
}
}
21 changes: 21 additions & 0 deletions crates/bevy_state/src/reflect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ pub struct ReflectFreelyMutableState(ReflectFreelyMutableStateFns);
pub struct ReflectFreelyMutableStateFns {
/// Function pointer implementing [`ReflectFreelyMutableState::set_next_state()`].
pub set_next_state: fn(&mut World, &dyn Reflect, &TypeRegistry),
/// Function pointer implementing [`ReflectFreelyMutableState::set_next_state_if_neq()`].
pub set_next_state_if_neq: fn(&mut World, &dyn Reflect, &TypeRegistry),
}

impl ReflectFreelyMutableStateFns {
Expand All @@ -77,6 +79,15 @@ impl ReflectFreelyMutableState {
pub fn set_next_state(&self, world: &mut World, state: &dyn Reflect, registry: &TypeRegistry) {
(self.0.set_next_state)(world, state, registry);
}
/// Tentatively set a pending state transition to a reflected [`ReflectFreelyMutableState`], skipping state transitions if the target state is the same as the current state.
pub fn set_next_state_if_neq(
&self,
world: &mut World,
state: &dyn Reflect,
registry: &TypeRegistry,
) {
(self.0.set_next_state_if_neq)(world, state, registry);
}
}

impl<S: FreelyMutableState + Reflect + TypePath> FromType<S> for ReflectFreelyMutableState {
Expand All @@ -92,6 +103,16 @@ impl<S: FreelyMutableState + Reflect + TypePath> FromType<S> for ReflectFreelyMu
next_state.set(new_state);
}
},
set_next_state_if_neq: |world, reflected_state, registry| {
let new_state: S = from_reflect_with_fallback(
reflected_state.as_partial_reflect(),
world,
registry,
);
if let Some(mut next_state) = world.get_resource_mut::<NextState<S>>() {
next_state.set_if_neq(new_state);
}
},
})
}
}
Expand Down
3 changes: 3 additions & 0 deletions crates/bevy_state/src/state/computed_states.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ pub trait ComputedStates: 'static + Send + Sync + Clone + PartialEq + Eq + Hash
/// For example, `(MapState, EnemyState)` is valid, as is `(MapState, Option<EnemyState>)`
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.


/// Computes the next value of [`State<Self>`](crate::state::State).
/// This function gets called whenever one of the [`SourceStates`](Self::SourceStates) changes.
///
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_state/src/state/freely_mutable_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ fn apply_state_transition<S: FreelyMutableState>(
current_state: Option<ResMut<State<S>>>,
next_state: Option<ResMut<NextState<S>>>,
) {
let Some((next_state, same_state_enforced)) = take_next_state(next_state) else {
let Some((next_state, allow_same_state_transitions)) = take_next_state(next_state) else {
return;
};
let Some(current_state) = current_state else {
Expand All @@ -63,6 +63,6 @@ fn apply_state_transition<S: FreelyMutableState>(
commands,
Some(current_state),
Some(next_state),
same_state_enforced,
allow_same_state_transitions,
);
}
53 changes: 52 additions & 1 deletion crates/bevy_state/src/state/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,7 @@ mod tests {
}

#[test]
fn same_state_transition_should_emit_event_and_not_run_schedules() {
fn same_state_transition_should_emit_event_and_run_schedules() {
let mut world = World::new();
setup_state_transitions_in_world(&mut world);
MessageRegistry::register_message::<StateTransitionEvent<SimpleState>>(&mut world);
Expand Down Expand Up @@ -533,6 +533,57 @@ mod tests {
world.insert_resource(NextState::Pending(SimpleState::A));
world.run_schedule(StateTransition);
assert_eq!(world.resource::<State<SimpleState>>().0, SimpleState::A);
assert_eq!(
*world.resource::<TransitionCounter>(),
TransitionCounter {
exit: 1,
transition: 1,
enter: 1
}
);
assert_eq!(
world
.resource::<Messages<StateTransitionEvent<SimpleState>>>()
.len(),
1
);
}

#[test]
fn same_state_transition_should_emit_event_and_not_run_schedules_if_same_state_transitions_are_disallowed(
) {
let mut world = World::new();
setup_state_transitions_in_world(&mut world);
MessageRegistry::register_message::<StateTransitionEvent<SimpleState>>(&mut world);
world.init_resource::<State<SimpleState>>();
let mut schedules = world.resource_mut::<Schedules>();
let apply_changes = schedules.get_mut(StateTransition).unwrap();
SimpleState::register_state(apply_changes);

let mut on_exit = Schedule::new(OnExit(SimpleState::A));
on_exit.add_systems(|mut c: ResMut<TransitionCounter>| c.exit += 1);
schedules.insert(on_exit);
let mut on_transition = Schedule::new(OnTransition {
exited: SimpleState::A,
entered: SimpleState::A,
});
on_transition.add_systems(|mut c: ResMut<TransitionCounter>| c.transition += 1);
schedules.insert(on_transition);
let mut on_enter = Schedule::new(OnEnter(SimpleState::A));
on_enter.add_systems(|mut c: ResMut<TransitionCounter>| c.enter += 1);
schedules.insert(on_enter);
world.insert_resource(TransitionCounter::default());

world.run_schedule(StateTransition);
assert_eq!(world.resource::<State<SimpleState>>().0, SimpleState::A);
assert!(world
.resource::<Messages<StateTransitionEvent<SimpleState>>>()
.is_empty());

world.insert_resource(TransitionCounter::default());
world.insert_resource(NextState::PendingIfNeq(SimpleState::A));
world.run_schedule(StateTransition);
assert_eq!(world.resource::<State<SimpleState>>().0, SimpleState::A);
assert_eq!(
*world.resource::<TransitionCounter>(),
TransitionCounter {
Expand Down
30 changes: 14 additions & 16 deletions crates/bevy_state/src/state/resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,29 +129,27 @@ pub enum NextState<S: FreelyMutableState> {
Pending(S),
/// There is a pending transition for state `S`
///
/// This will trigger state transitions schedules even if the target state is the same as the current one.
ForcedPending(S),
/// This will not trigger state transitions schedules if the target state is the same as the current one.
PendingIfNeq(S),
}

impl<S: FreelyMutableState> NextState<S> {
/// Tentatively set a pending state transition to `Some(state)`.
///
/// If `state` is the same as the current state, this will *not* trigger state
/// transition [`OnEnter`](crate::state::OnEnter) and [`OnExit`](crate::state::OnExit) schedules.
///
/// If [`set_forced`](Self::set_forced) has already been called in the same frame with the same state, its behavior is kept.
/// This will run the state transition schedules [`OnEnter`](crate::state::OnEnter) and [`OnExit`](crate::state::OnExit).
/// If you want to skip those schedules for the same where we are transitioning to the same state, use [`set_if_neq`](Self::set_if_neq) instead.
pub fn set(&mut self, state: S) {
if !matches!(self, Self::ForcedPending(s) if s == &state) {
*self = Self::Pending(state);
}
*self = Self::Pending(state);
}

/// Tentatively set a pending state transition to `Some(state)`.
///
/// If `state` is the same as the current state, this will trigger state
/// transition [`OnEnter`](crate::state::OnEnter) and [`OnExit`](crate::state::OnExit) schedules.
pub fn set_forced(&mut self, state: S) {
*self = Self::ForcedPending(state);
/// Like [`set`](Self::set), but will not run any state transition schedules if the target state is the same as the current one.
/// If [`set`](Self::set) has already been called in the same frame with the same state, the transition schedules will be run anyways.
pub fn set_if_neq(&mut self, state: S) {
if !matches!(self, Self::Pending(s) if s == &state) {
*self = Self::PendingIfNeq(state);
}
}

/// Remove any pending changes to [`State<S>`]
Expand All @@ -168,11 +166,11 @@ pub(crate) fn take_next_state<S: FreelyMutableState>(
match core::mem::take(next_state.bypass_change_detection()) {
NextState::Pending(x) => {
next_state.set_changed();
Some((x, false))
Some((x, true))
}
NextState::ForcedPending(x) => {
NextState::PendingIfNeq(x) => {
next_state.set_changed();
Some((x, true))
Some((x, false))
}
NextState::Unchanged => None,
}
Expand Down
8 changes: 7 additions & 1 deletion crates/bevy_state/src/state/state_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,13 @@ impl<S: InnerStateSet> StateSet for S {
None
};

internal_apply_state_transition(event, commands, current_state, new_state, false);
internal_apply_state_transition(
event,
commands,
current_state,
new_state,
T::ALLOW_SAME_STATE_TRANSITIONS,
);
};

schedule.configure_sets((
Expand Down
16 changes: 8 additions & 8 deletions crates/bevy_state/src/state/transitions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ pub struct StateTransitionEvent<S: States> {
pub exited: Option<S>,
/// The state being entered.
pub entered: Option<S>,
/// Enforce this transition even if `exited` and `entered` are the same
pub same_state_enforced: bool,
/// Allow running state transition events when `exited` and `entered` are the same
pub allow_same_state_transitions: bool,
}

/// Applies state transitions and runs transitions schedules in order.
Expand Down Expand Up @@ -137,7 +137,7 @@ pub(crate) fn internal_apply_state_transition<S: States>(
mut commands: Commands,
current_state: Option<ResMut<State<S>>>,
new_state: Option<S>,
same_state_enforced: bool,
allow_same_state_transitions: bool,
) {
match new_state {
Some(entered) => {
Expand All @@ -156,7 +156,7 @@ pub(crate) fn internal_apply_state_transition<S: States>(
event.write(StateTransitionEvent {
exited: Some(exited.clone()),
entered: Some(entered.clone()),
same_state_enforced,
allow_same_state_transitions,
});
}
None => {
Expand All @@ -166,7 +166,7 @@ pub(crate) fn internal_apply_state_transition<S: States>(
event.write(StateTransitionEvent {
exited: None,
entered: Some(entered.clone()),
same_state_enforced,
allow_same_state_transitions,
});
}
};
Expand All @@ -179,7 +179,7 @@ pub(crate) fn internal_apply_state_transition<S: States>(
event.write(StateTransitionEvent {
exited: Some(resource.get().clone()),
entered: None,
same_state_enforced,
allow_same_state_transitions,
});
}
}
Expand Down Expand Up @@ -223,7 +223,7 @@ pub(crate) fn run_enter<S: States>(
let Some(transition) = transition.0 else {
return;
};
if transition.entered == transition.exited && !transition.same_state_enforced {
if transition.entered == transition.exited && !transition.allow_same_state_transitions {
return;
}
let Some(entered) = transition.entered else {
Expand All @@ -240,7 +240,7 @@ pub(crate) fn run_exit<S: States>(
let Some(transition) = transition.0 else {
return;
};
if transition.entered == transition.exited && !transition.same_state_enforced {
if transition.entered == transition.exited && !transition.allow_same_state_transitions {
return;
}
let Some(exited) = transition.exited else {
Expand Down
Loading