Rename `StateScoped` to `DespawnOnStateExit` and add `DespawnOnStateEnter` #16284

BenjaminBrienen · 2024-11-07T20:53:08Z

Objective

Solution

Rename StateScoped to DespawnOnStateExit (and related function).
Add a copy of the type named DespawnOnStateEnter (and modify related function to use .entered)
Add the system.

Showcase

state_scoped.mp4

Testing

Tested manually with an example

Migration Guide

The StateScoped component has been renamed to DespawnOnStateExit. A corresponding DespawnOnStateEnter component has also been added.

BenjaminBrienen · 2024-11-07T20:53:29Z

@Jaso333 does this meet your needs?

Jaso333 · 2024-11-07T20:56:50Z

@Jaso333 does this meet your needs?

absolutely! thanks!

BenjaminBrienen · 2024-11-07T21:00:54Z

@Jaso333 does this meet your needs?

absolutely! thanks!

Could review the PR, please? :)

ghost

The code looks fine and is a means to an end. I think a larger discussion about world object management needs to happen tho.

benfrankel · 2024-11-09T01:59:47Z

I haven't looked at the code, but I approve of this direction and the names chosen. In pyri_state I renamed StateScope to DespawnOnExit already. I like the OnStateExit disambiguation, though, especially if more variants are added. If we get entity disabling and state disabling eventually, DisableOnStateDisable makes more sense than DisableOnDisable :P

Can ping me for a review if another approve is needed.

mockersf · 2024-11-09T06:56:04Z

you should also rename App::enable_state_scoped_entities to something else if we want to get ride of "state scoped"

BenjaminBrienen · 2024-11-09T10:17:02Z

you should also rename App::enable_state_scoped_entities to something else if we want to get ride of "state scoped"

I think "state scoped" is a great name for the feature itself.

github-actions · 2024-11-12T14:18:40Z

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

Could you add some context on what users should update when this change get released in a new version of Bevy?
It will be used to help writing the migration guide for the version. Putting it after a ## Migration Guide will help it get automatically picked up by our tooling.

alice-i-cecile

I think this is clearer, and the functionality is useful. If you add tests and a migration guide you have my approval. You probably want to do something similar for events too.

I've removed X-Controversial, as this isn't a particularly high-impact change, and the various states devs are broadly in favor of this style of mechanism.

BenjaminBrienen · 2024-11-12T14:51:16Z

That should work for the migration guide, I think. I'll add a test tonight.

alice-i-cecile · 2024-11-12T15:05:00Z

Tweaked the migration guide wording slightly for clarity :)

MiniaczQ

Looks good, one suggestion for followup if we want to have even more variants

crates/bevy_state/src/app.rs

BenjaminBrienen · 2024-11-13T01:48:22Z

I didn't have time tonight. Definitely tomorrow or the next day.

Cargo.toml

examples/ecs/state_scoped.rs

alice-i-cecile

I like the rename and new feature, but I think we can clean up the documentation story a bit :)

…into rename-StateScoped

mgi388 · 2025-04-24T02:45:45Z

I've adopted this in #18818, so have nominated to close this.

…tate` (#18818) # Objective - Alternative to and builds on top of #16284. - Fixes #15849. ## Solution - Rename component `StateScoped` to `DespawnOnExitState`. - Rename system `clear_state_scoped_entities` to `despawn_entities_on_exit_state`. - Add `DespawnOnEnterState` and `despawn_entities_on_enter_state` which is the `OnEnter` equivalent. > [!NOTE] > Compared to #16284, the main change is that I did the rename in such a way as to keep the terms `OnExit` and `OnEnter` together. In my own game, I was adding `VisibleOnEnterState` and `HiddenOnExitState` and when naming those, I kept the `OnExit` and `OnEnter` together. When I checked #16284 it stood out to me that the naming was a bit awkward. Putting the `State` in the middle and breaking up `OnEnter` and `OnExit` also breaks searching for those terms. ## Open questions 1. Should we split `enable_state_scoped_entities` into two functions, one for the `OnEnter` and one for the `OnExit`? I personally have zero need thus far for the `OnEnter` version, so I'd be interested in not having this enabled unless I ask for it. 2. If yes to 1., should we follow my lead in my `Visibility` state components (see below) and name these `app.enable_despawn_entities_on_enter_state()` and `app.enable_despawn_entities_on_exit_state()`, which IMO says what it does on the tin? ## Testing Ran all changed examples. ## Side note: `VisibleOnEnterState` and `HiddenOnExitState` For reference to anyone else and to help with the open questions, I'm including the code I wrote for controlling entity visibility when a state is entered/exited. <details> <summary>visibility.rs</summary> ```rust use bevy_app::prelude::*; use bevy_ecs::prelude::*; use bevy_reflect::prelude::*; use bevy_render::prelude::*; use bevy_state::{prelude::*, state::StateTransitionSteps}; use tracing::*; pub trait AppExtStates { fn enable_visible_entities_on_enter_state<S: States>(&mut self) -> &mut Self; fn enable_hidden_entities_on_exit_state<S: States>(&mut self) -> &mut Self; } impl AppExtStates for App { fn enable_visible_entities_on_enter_state<S: States>(&mut self) -> &mut Self { self.main_mut() .enable_visible_entities_on_enter_state::<S>(); self } fn enable_hidden_entities_on_exit_state<S: States>(&mut self) -> &mut Self { self.main_mut().enable_hidden_entities_on_exit_state::<S>(); self } } impl AppExtStates for SubApp { fn enable_visible_entities_on_enter_state<S: States>(&mut self) -> &mut Self { if !self .world() .contains_resource::<Events<StateTransitionEvent<S>>>() { let name = core::any::type_name::<S>(); warn!("Visible entities on enter state are enabled for state `{}`, but the state isn't installed in the app!", name); } // We work with [`StateTransition`] in set // [`StateTransitionSteps::ExitSchedules`] as opposed to [`OnExit`], // because [`OnExit`] only runs for one specific variant of the state. self.add_systems( StateTransition, update_to_visible_on_enter_state::<S>.in_set(StateTransitionSteps::ExitSchedules), ) } fn enable_hidden_entities_on_exit_state<S: States>(&mut self) -> &mut Self { if !self .world() .contains_resource::<Events<StateTransitionEvent<S>>>() { let name = core::any::type_name::<S>(); warn!("Hidden entities on exit state are enabled for state `{}`, but the state isn't installed in the app!", name); } // We work with [`StateTransition`] in set // [`StateTransitionSteps::ExitSchedules`] as opposed to [`OnExit`], // because [`OnExit`] only runs for one specific variant of the state. self.add_systems( StateTransition, update_to_hidden_on_exit_state::<S>.in_set(StateTransitionSteps::ExitSchedules), ) } } #[derive(Clone, Component, Debug, Reflect)] #[reflect(Component, Debug)] pub struct VisibleOnEnterState<S: States>(pub S); #[derive(Clone, Component, Debug, Reflect)] #[reflect(Component, Debug)] pub struct HiddenOnExitState<S: States>(pub S); /// Makes entities marked with [`VisibleOnEnterState<S>`] visible when the state /// `S` is entered. pub fn update_to_visible_on_enter_state<S: States>( mut transitions: EventReader<StateTransitionEvent<S>>, mut query: Query<(&VisibleOnEnterState<S>, &mut Visibility)>, ) { // We use the latest event, because state machine internals generate at most // 1 transition event (per type) each frame. No event means no change // happened and we skip iterating all entities. let Some(transition) = transitions.read().last() else { return; }; if transition.entered == transition.exited { return; } let Some(entered) = &transition.entered else { return; }; for (binding, mut visibility) in query.iter_mut() { if binding.0 == *entered { visibility.set_if_neq(Visibility::Visible); } } } /// Makes entities marked with [`HiddenOnExitState<S>`] invisible when the state /// `S` is exited. pub fn update_to_hidden_on_exit_state<S: States>( mut transitions: EventReader<StateTransitionEvent<S>>, mut query: Query<(&HiddenOnExitState<S>, &mut Visibility)>, ) { // We use the latest event, because state machine internals generate at most // 1 transition event (per type) each frame. No event means no change // happened and we skip iterating all entities. let Some(transition) = transitions.read().last() else { return; }; if transition.entered == transition.exited { return; } let Some(exited) = &transition.exited else { return; }; for (binding, mut visibility) in query.iter_mut() { if binding.0 == *exited { visibility.set_if_neq(Visibility::Hidden); } } } ``` </details> --------- Co-authored-by: Benjamin Brienen <Benjamin.Brienen@outlook.com> Co-authored-by: Ben Frankel <ben.frankel7@gmail.com>

BenjaminBrienen self-assigned this Nov 7, 2024

ghost approved these changes Nov 8, 2024

View reviewed changes

alice-i-cecile requested review from MiniaczQ and alice-i-cecile November 12, 2024 14:17

alice-i-cecile reviewed Nov 12, 2024

View reviewed changes

BenjaminBrienen added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Nov 12, 2024

MiniaczQ approved these changes Nov 13, 2024

View reviewed changes

crates/bevy_state/src/app.rs Show resolved Hide resolved

BenjaminBrienen added 4 commits November 16, 2024 22:32

initial implementation of feature

63296a4

fix clear_despawn_on_state_enter_entities

475e68c

update documentation and fmt

f98eb2b

example

1a3273b

BenjaminBrienen force-pushed the rename-StateScoped branch from 4b8684b to 1a3273b Compare November 16, 2024 22:51

Merge branch 'main' into rename-StateScoped

4f3ef5c

BenjaminBrienen requested review from MiniaczQ and alice-i-cecile January 2, 2025 00:15

alice-i-cecile reviewed Jan 2, 2025

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

alice-i-cecile reviewed Jan 2, 2025

View reviewed changes

examples/ecs/state_scoped.rs Outdated Show resolved Hide resolved

alice-i-cecile reviewed Jan 2, 2025

View reviewed changes

BenjaminBrienen added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jan 2, 2025

feedback on documentation

c390276

BenjaminBrienen requested a review from alice-i-cecile January 3, 2025 22:10

BenjaminBrienen added S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Jan 3, 2025

BenjaminBrienen and others added 4 commits January 3, 2025 17:11

Merge branch 'main' into rename-StateScoped

f9dd13c

update examples readme

02aa213

Merge branch 'rename-StateScoped' of github.com:BenjaminBrienen/bevy …

0382a79

…into rename-StateScoped

clippy

4478d2e

BenjaminBrienen requested review from mockersf, MrGVSV and BD103 January 3, 2025 22:26

BenjaminBrienen added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jan 22, 2025

BenjaminBrienen removed their assignment Mar 5, 2025

BenjaminBrienen added S-Adopt-Me The original PR author has no intent to complete this work. Pick me up! and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Mar 5, 2025

mgi388 mentioned this pull request Apr 12, 2025

Rename StateScoped to DespawnOnExitState and add DespawnOnEnterState #18818

Merged

mgi388 added the S-Nominated-To-Close A triage team member thinks this PR or issue should be closed out. label Apr 24, 2025

alice-i-cecile closed this Apr 24, 2025

BenjaminBrienen deleted the rename-StateScoped branch April 24, 2025 06:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename `StateScoped` to `DespawnOnStateExit` and add `DespawnOnStateEnter` #16284

Rename `StateScoped` to `DespawnOnStateExit` and add `DespawnOnStateEnter` #16284

BenjaminBrienen commented Nov 7, 2024 •

edited by alice-i-cecile

Loading

BenjaminBrienen commented Nov 7, 2024

Jaso333 commented Nov 7, 2024 •

edited

Loading

BenjaminBrienen commented Nov 7, 2024

ghost left a comment

benfrankel commented Nov 9, 2024 •

edited

Loading

mockersf commented Nov 9, 2024

BenjaminBrienen commented Nov 9, 2024

github-actions bot commented Nov 12, 2024

alice-i-cecile left a comment

BenjaminBrienen commented Nov 12, 2024

alice-i-cecile commented Nov 12, 2024

MiniaczQ left a comment

BenjaminBrienen commented Nov 13, 2024

alice-i-cecile left a comment

mgi388 commented Apr 24, 2025

Rename StateScoped to DespawnOnStateExit and add DespawnOnStateEnter #16284

Rename StateScoped to DespawnOnStateExit and add DespawnOnStateEnter #16284

Conversation

BenjaminBrienen commented Nov 7, 2024 • edited by alice-i-cecile Loading

Objective

Solution

Showcase

Testing

Migration Guide

BenjaminBrienen commented Nov 7, 2024

Jaso333 commented Nov 7, 2024 • edited Loading

BenjaminBrienen commented Nov 7, 2024

ghost left a comment

Choose a reason for hiding this comment

benfrankel commented Nov 9, 2024 • edited Loading

mockersf commented Nov 9, 2024

BenjaminBrienen commented Nov 9, 2024

github-actions bot commented Nov 12, 2024

alice-i-cecile left a comment

Choose a reason for hiding this comment

BenjaminBrienen commented Nov 12, 2024

alice-i-cecile commented Nov 12, 2024

MiniaczQ left a comment

Choose a reason for hiding this comment

BenjaminBrienen commented Nov 13, 2024

alice-i-cecile left a comment

Choose a reason for hiding this comment

mgi388 commented Apr 24, 2025

Rename `StateScoped` to `DespawnOnStateExit` and add `DespawnOnStateEnter` #16284

Rename `StateScoped` to `DespawnOnStateExit` and add `DespawnOnStateEnter` #16284

BenjaminBrienen commented Nov 7, 2024 •

edited by alice-i-cecile

Loading

Jaso333 commented Nov 7, 2024 •

edited

Loading

benfrankel commented Nov 9, 2024 •

edited

Loading