ScheduleBuildPass are not overwritten or removed, only combined or disabled

[urben1680]

Objective

Fixes #18790.

Solution

Adding schedule build setting clears all configurations regarding edges ignoring deferred parameters until then:

• If the new setting has auto_insert_apply_deferred = true then currently the setting gets overwritten with an empty set of ignoring edges.
• If the new setting has auto_insert_apply_deferred = false then currently the ignoring edges are lost even when re-enabling the build pass.

The fix is that build passes are never removed or overwritten, only combined with new values or simply disabled. I think this follows the idea of #11094.

The implementation makes use of rust's 1.86 down-casting that is properly brought into the bevy code by #18984 so it might make sense to merge that one first before this PR.

Alternatives

Instead of this PR, there are three other ways to solve the issue:

1. Just point out in the docs that set_build_pass should not be used after configurations were added. This could be enforced with an assertion that panics.
2. Make build passes independent to the configurations that are currently in the schedule. They instead pull the information they need during building. This requires then another collection in the graph that keeps the pass-specific dependencies around so they can be applied when building, not when added.
3. Just check if the currently auto_insert_apply_deferred is already true and in this case do not add the pass which is only a solution for this specific pass in particular, but a very simple one.

So far I explored only 2. and found it too complex for a bug fix where there is no use case yet that requires passes to see dependencies only at build time. For 1. it seems not better than now except the problem causes a panic. I likely will check 3. soon.

Testing

I added a new test. I am not sure if you like that it is more like 3 tests in one but I did not want to pollute the module with the types for this "single" test. But I can split it up if wanted.

Migration Guide

Added one: rendered

[urben1680]

I am not sure this is how it should work like. (with or without the if condition)

With this, when a pass is disabled, it is not updated by configs. When enabling it, changes keep being updated.

This way you could add specific system/set configurations that behave differently than the other you added before you disabled a pass and after you enabled the pass again.

Maybe instead this should still be updated even when disabled so, if enabled again, it is up to date with all configs, not the one it witnessed enabled?

What speaks for this behavior here is that it is consistent with when the pass is not added at all yet.

Same matter to the change in add_system_inner. Or maybe not because it is called by build.

[urben1680]

I want to explore the second alternative idea I wrote about. I either then open both PRs to decide or one that seems obviously better to me.

[urben1680]

I think I want to stick to this variant. It is more minimal compared to storing dependencies in yet another Vec<TypeIdMap<...>> in ScheduleGraph. Doing that makes more sense if there is more of a need to delaying adding dependencies until the building.

[PixelDust22]

A better and easier to implement idea might be that, in set_build_settings, we check the current settings and skip reinserting the build pass if it's already enabled.

[urben1680]

That would have the downside that disabling and re-enabling still clears the edge list that do not insert sync points.

But of course, one could say that is what you get for changing settings around more than once.

[PixelDust22]

Yeah, honestly I don't see why someone would want to flip-flop on their decision to enable auto sync point insertion.

I would just add a documentation there clarifying that disabling auto sync point insertion will remove all existing edge settings related to auto sync.

[urben1680]

Closed in favor of #19217