resolve all internal ambiguities

[jakobhellermann]

• ignore all ambiguities that are not a problem
• remove .before(Assets::<Image>::track_assets), that points into a different schedule (-> should this be caught?)
• add some explicit orderings:
  • run poll_receivers and update_accessibility_nodes after window_closed in bevy_winit::accessibility
  • run bevy_ui::accessibility::calc_bounds after CameraUpdateSystem
  • run bevy_text::update_text2d_layout and bevy_ui::text_system after font_atlas_set::remove_dropped_font_atlas_sets
• add app.ignore_ambiguity(a, b) function for cases where you want to ignore an ambiguity between two independent plugins A and B
• add IgnoreAmbiguitiesPlugin in DefaultPlugins that allows cross-crate ambiguities like bevy_animation/bevy_ui
• Fixes Bevy contains internal system-order ambiguities #9511

Before

Render

PostUpdate

After

Render

PostUpdate

[alice-i-cecile]

maybe we can add a regression test for not introducing new ones.

Strongly in favor of this.

I'm fine with either solution 1 or 2 for fixing the outstanding ambiguity: I think we may actually want both.

I like splitting that work out of this PR though, and merging this ASAP.

[alice-i-cecile]

Changed my opinion on the best path forward following discussion on Discord by @hymm :)

[alice-i-cecile]

@jakobhellermann can you update the PR description and title to match the latest changes?

[hymm]

can we make this just sugar for configure_sets(a.ambiguous_with(b))?

[jakobhellermann]

thread 'main' panicked at /home/jakob/dev/rust/bevy/crates/bevy_ecs/src/schedule/config.rs:533:38:
configuring system type sets is not allowed

[hymm]

I wonder if we're being too strict here and should just deny using system type sets with in_set instead of any configuration. @maniwani any opinion?

edit: this is non-blocking and can be done in a separate pr.

[alice-i-cecile]

🤔 I think that's a pretty reasonable way to increase the flexibility of the API. in_set is definitely a mistake, but being able to configure public systems without an explicit label seems safe.

[maniwani]

I don't understand what's being asked here. in_set should already fail if given a system type set.

[alice-i-cecile]

Should configure_set(my_system.before(ArbitraryLabel)) work?

[maniwani]

Last I remember, if you call configure_set or in_set with a system type set, it should immediately error at runtime. Maybe somebody changed that, but the error was intentional.

It'd be too easy to call that with a system that's in many places like apply_deferred.

You can pass a system type set into the ambiguity-related methods. However, for similar reasons, if there are multiple instances of that system in the schedule, you'll get a schedule build error.

[maniwani]

The error on in_set should be obvious. Users shouldn't be adding any other kind of system to the type set.

The immediate error on configure_set is directed at stopping users from calling before, after, run_if, and in_set and it affecting every single instance of a system.

It'd be a little more complicated, but someone could tweak that so that configure_set doesn't error if you only call ambiguity methods. I don't know if that's generally "safe" to use tho. I think you'd have to get rid of that schedule build check too for this to work.

[alice-i-cecile]

Sounds good, let's leave it as is for now.

[hymm]

This looks good to me, but someone from rendering should probably double check that the new before/after constraints in the render schedule make sense.

[hymm]

Can you resolve conflicts? Since we're earlier in the cycle now, we'll probably find any problems caused by this pr before release, so I'm fine with merging even if someone from rendering doesn't approve.

[alice-i-cecile]

Resolved conflicts, fingers crossed that CI still passes and we can finally merge this <3

[hymm]

looks like the failure is real. Something about this line when no default features is used https://github.com/bevyengine/bevy/pull/10411/files#diff-f6e740177ada590c3b70e737730d436f3314a45a6455ce5f5efe6d9bdbfebe98R154