Skip to content

Change the ambiguity_detection test to be more strict.#23846

Open
andriyDev wants to merge 1 commit intobevyengine:mainfrom
andriyDev:strict-amb
Open

Change the ambiguity_detection test to be more strict.#23846
andriyDev wants to merge 1 commit intobevyengine:mainfrom
andriyDev:strict-amb

Conversation

@andriyDev
Copy link
Copy Markdown
Contributor

@andriyDev andriyDev commented Apr 17, 2026 •
edited
Loading

Objective

Solution

  • Disable auto_insert_apply_deferred in the ambiguity_detection example. Now these stages won't accidentally resolve these ambiguities, so they will be detected!
  • Stop running the app in ambiguity_detection and just manually initialize the schedules.
    • We have to do this otherwise the schedules just panic because they depend on various resource initializations to be executed by commands (which were previously being applied by the automatically inserted apply_deferred).

Caveats:

  • The ambiguity_detection CI test now won't detect ambiguities in runtime-added systems.
    • This is not a pattern we use today, and I'd encourage us to delete systems instead so that tooling can grab the systems before the app runs.
    • To clarify, this is not ambiguity detection in general - just Bevy's CI test that is affected.

Testing

  • Ran the examples and all the ambiguities have been fixed in previous PRs.

This was referenced Apr 17, 2026
Merged
Merged
This was referenced Apr 17, 2026
Merged
Merged
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Apr 17, 2026
@github-project-automation github-project-automation bot moved this to Needs SME Triage in ECS Apr 17, 2026
@alice-i-cecile alice-i-cecile added the S-Blocked This cannot move forward until something else changes label Apr 17, 2026
alice-i-cecile
alice-i-cecile approved these changes Apr 17, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Mildly cursed, but I appreciate the robustness.

@alice-i-cecile alice-i-cecile added C-Testing A change that impacts how we test Bevy or how users test their apps A-Cross-Cutting Impacts the entire engine X-Contentious There are nontrivial implications that should be thought through D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes and removed C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Apr 17, 2026
@EmileGr
Copy link
Copy Markdown

EmileGr commented Apr 17, 2026

Not having ambiguity detection for runtime added systems seems like a pretty big downside tbh. I grant you that Bevy itself doesn't make use of that feature. However, users who are implementing scripting languages on top of Bevy, for example, will acutely feel the pain not having ambiguity detection anymore.

@andriyDev
Copy link
Copy Markdown
Contributor Author

andriyDev commented Apr 17, 2026

@EmileGr this only affects Bevy CI. This does not affect any ambiguity detection for downstream users. I just wanted to note that if we use runtime added systems within Bevy, that's not something that CI will detect.

github-merge-queue bot pushed a commit that referenced this pull request Apr 17, 2026
@andriyDev
Remove unused &Camera in update_gizmo_meshes. (#23853)
724bc10 
# Objective

- Removes an entirely unused component in `update_gizmo_meshes`.
- Resolve a "strict ambiguity" for #23846.

## Solution

- Remove the arg! Replace with `With<Camera>`.

## Testing

- Ran the `3d_gizmos` example. It still works!
github-merge-queue bot pushed a commit that referenced this pull request Apr 17, 2026
@andriyDev
Fix light bounding spheres being ambiguous with TransformSystems::Pro…
774e1fd 
…pagate. (#23847)

# Objective

- Resolves a "strict ambiguity" from #23843.
- TL;DR, adding systems before this schedule with `Commands` could cause
these systems being ambiguous.
- A step for #23846.

## Solution

- Explicitly order these lighting bounds updates after the transform
propagation.

## Testing

- This removes a "strict ambiguity" from #23846.
github-merge-queue bot pushed a commit that referenced this pull request Apr 17, 2026
@andriyDev
Fix some UI widgets accidentally being ambiguous with recomputing the…
7c5a1c3 
… ComputedNode::stack_index, which is not used. (#23854)

# Objective

- Resolves 2 "strict ambiguities" from #23843.

## Solution

- Make these two widget systems ambiguous with the Stack stage.
- This only contains the ui_stack_system which only updates the
stack_index of the `ComputedNode`, which is unused by these systems. So
we don't care about order here.

## Testing

- Fixes the ambiguities in #23846.
@andriyDev andriyDev marked this pull request as ready for review April 19, 2026 16:19
@andriyDev andriyDev removed the S-Blocked This cannot move forward until something else changes label Apr 19, 2026
@andriyDev
Copy link
Copy Markdown
Contributor Author

andriyDev commented Apr 19, 2026

BTW for motivation, this fixed an actual bug in #23847! Strict checking is valuable!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

Assignees

No one assigned

Labels

A-Cross-Cutting Impacts the entire engine A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change C-Testing A change that impacts how we test Bevy or how users test their apps D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Review Needs reviewer attention (from anyone!) to move forward X-Contentious There are nontrivial implications that should be thought through

Projects

ECS
Status: Needs SME Triage

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Ambiguous systems can accidentally be reported as non-ambiguous due to an automatically inserted apply_deferred.

3 participants

@andriyDev @EmileGr @alice-i-cecile