Eager system parameter validation causes issues with combinator and piped systems

[alice-i-cecile]

For some history, see #16638 (comment):

In a combination system, like a.and(b) or a.pipe(b) we're running param checks for all systems before running any of them. This results in resource_exists::<MyRes>.and(do_stuff) failing params, because do_stuff doesn't have MyRes even though it wouldn't run because of failed condition.
...
Since combination systems are incomplete, let's just ensure the first system (or the system, if no combinations are used) works correctly with fallible params, do not check params for subsequent ones and just let app crash like it did in 0.14. In 0.16 we can ship the correct param checking behavior, where we do it directly before running a system.

Originally posted by @chescock in #18755

The same problem occurs with combinator systems, particularly when using the short-circuiting and behavior. The same bug as in #18755 exists for combinator systems, but fixing it breaks much more normal use cases (and one of our doc tests!).

Proposed solution

In order to resolve this (and other issues, see #8857, #2381), we need to retain the unique identity of systems even when piped.

we can ship the correct param checking behavior, where we do it directly before running a system.

This is the ideal solution, but can't work with the current architecture. These systems, from the perspective of the schedule (and thus the executor), are only one system.

[chescock]

I see two other possible approaches here that might be worth considering:

1. Embrace Result! If PipeSystem returned a Result, then it could validate the second system just-in-time and return a SystemParamValidationError that indicates it was skipped. The executors could inspect the error and not report it if it was caused by skipping. (I'm glossing over a lot of tricky details here, but I think the general idea is at least possible.)
2. Just avoid the bad cases. We could have resource_equals use Option<Res<T>> and treat a missing resource as never equal, and then nobody would have to write resource_exists::<R>.and(resource_equals(R(0)). More complex run conditions could use something like When<Res<T>> from Create a When system param wrapper for skipping systems that fail validation #18765 to avoid needing to call resource_exists().and().

[alice-i-cecile]

Embrace Result

Interesting! If you (or anyone else) can get that working I'm very down for that as a fix. I still think the way we handle system piping is too inflexible, but that's a much bigger can of worms that I don't have the appetite to open right now.

Just avoid the bad cases. We could have resource_equals use Option<Res> and treat a missing resource as never equal, and then nobody would have to write resource_exists::.and(resource_equals(R(0)). More complex run conditions could use something like When<Res> from #18765 to avoid needing to call resource_exists().and().

I think this is a good thing to do in general, but doesn't solve the issue more broadly, especially for user-defined run conditions.