Skip to content

Add constraints to AppLabel#23377

Open
Zeophlite wants to merge 10 commits intobevyengine:mainfrom
Zeophlite:app-label2
Open

Add constraints to AppLabel#23377
Zeophlite wants to merge 10 commits intobevyengine:mainfrom
Zeophlite:app-label2

Conversation

@Zeophlite
Copy link
Copy Markdown
Contributor

@Zeophlite Zeophlite commented Mar 16, 2026

Objective

  • While working on Extract extract to bevy_extract #22852 , I found AppLabel needed multiple additional bounds ( the largest being L: AppLabel + Default + Clone + Eq + Copy , in total 44 times) - this greatly reduces the readability of that PR

e.g. I want to change ExtractComponentPlugin to be generic over L: AppLabel + Default, so that I can do the following:

-        if let Some(render_app) = app.get_sub_app_mut(RenderApp) {
+        if let Some(render_app) = app.get_sub_app_mut(L::default()) {

Eq is needed to by EntityEquivalent to make

unsafe impl<L: AppLabel> EntityEquivalent for RenderEntity<L> {}

Clone is needed to make SyncToRenderWorld generic

Copy is needed to make Plugin for SyncWorldPlugin generic

If its desired, I can remove the extra constraints, and just have pub trait AppLabel: AppLabelInterior {} as a placeholder, and add requirements as... required.

  • Note: I tried adding this to AppLabel directly, but AppLabel is created by bevy_ecs::define_label! , and I was unable to add the above there

Solution

  • Use AppLabelInterior as a derived label, that have a super trait AppLabel with the additional constraints.

Testing

  • CI

@Zeophlite Zeophlite added A-Rendering Drawing game state to the screen D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Mar 16, 2026
@github-project-automation github-project-automation bot moved this to Needs SME Triage in Rendering Mar 16, 2026
@Zeophlite Zeophlite changed the title Refactor AppLabel Add constraints to AppLabel Mar 16, 2026
@Zeophlite Zeophlite marked this pull request as ready for review March 16, 2026 09:29
@Zeophlite Zeophlite 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 Mar 16, 2026
@mockersf
Copy link
Copy Markdown
Member

I don't understand the need to add an intermediate AppLabelBase trait.

If we need it, we should find a better name for it

@Zeophlite
Copy link
Copy Markdown
Contributor Author

I don't understand the need to add an intermediate AppLabelBase trait.

The original AppLabel uses the bevy_ecs::define_label! macro to create that type. I'm unable to add Default to that (might be a skill issue)

If we need it, we should find a better name for it

Yeah I don't love it

@alice-i-cecile alice-i-cecile added X-Contentious There are nontrivial implications that should be thought through S-Needs-Help The author needs help finishing this PR. and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Mar 18, 2026
@Zeophlite Zeophlite added the S-Needs-Review Needs reviewer attention (from anyone!) to move forward label Apr 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Rendering Drawing game state to the screen D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Help The author needs help finishing this PR. S-Needs-Review Needs reviewer attention (from anyone!) to move forward X-Contentious There are nontrivial implications that should be thought through

Projects

Status: Needs SME Triage

Development

Successfully merging this pull request may close these issues.

3 participants