Skip to content

Conversation

@alice-i-cecile
Copy link
Member

@alice-i-cecile alice-i-cecile commented Oct 21, 2025

Objective

Despite initially advocating for its inclusion in #20204, I've been increasingly unconvinced by the edge cases and user-facing complexity and surprise that Internal brings.

Accidental queries are quite hard to write, and the entitiy-inspector concerns are really a UX problem for each tool to solve that Internal doesn't help with.

@cart feels similarly: as a result I'm marking this PR as X-Blessed.

Closes #21363.

Solution

  • Remove Internal as a type.
  • Follow the compiler errors to remove all references.
  • Write a migration guide.

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use X-Blessed Has a large architectural impact or tradeoffs, but the design has been endorsed by decision makers M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Oct 21, 2025
@alice-i-cecile alice-i-cecile added this to the 0.18 milestone Oct 21, 2025
@alice-i-cecile alice-i-cecile added the S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged label Oct 21, 2025
@alice-i-cecile alice-i-cecile 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 Oct 21, 2025
pub fn despawn_entities_on_exit_state<S: States>(
mut commands: Commands,
mut transitions: MessageReader<StateTransitionEvent<S>>,
// TODO: Use `AllowAll` once it exists: https://github.com/bevyengine/bevy/issues/21615
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this todo removed? reading the related issue/PR, it sounds like the intent to completely bypass the default query filters here once that becomes possible to express in a system remains.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think that we should be using AllowAll here even after #21615 is implemented. The behavior is too surprising and will mess with people's e.g. prefab systems that are built using disabling components.

@alice-i-cecile alice-i-cecile 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 Oct 21, 2025
@chescock
Copy link
Contributor

This should also close #21363, since Internal was the only case where we expected a disabling component to be used as a required component.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Oct 22, 2025
Merged via the queue into bevyengine:main with commit f35d22a Oct 22, 2025
40 checks passed
mate-h pushed a commit to mate-h/bevy that referenced this pull request Oct 22, 2025
# Objective

Despite initially advocating for its inclusion in bevyengine#20204, I've been
increasingly unconvinced by the edge cases and user-facing complexity
and surprise that `Internal` brings.

Accidental queries are quite hard to write, and the entitiy-inspector
concerns are really a UX problem for each tool to solve that `Internal`
doesn't help with.

@cart feels similarly: as a result I'm marking this PR as X-Blessed.

Closes bevyengine#21363.

## Solution

- Remove `Internal` as a type.
- Follow the compiler errors to remove all references.
- Write a migration guide.
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-Usability A targeted quality-of-life change that makes Bevy easier to use 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-Blessed Has a large architectural impact or tradeoffs, but the design has been endorsed by decision makers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Components with required disabling components are hard to query

4 participants