Logging for system parameter validation should be configurable

[alice-i-cecile]

What problem does this solve or what need does it fill?

Following #15276, systems are now skipped when they cannot run due to missing data, rather than panicking and crashing their app.

While we should 100% warn by default when this happens, some of those logs will be spurious. This behavior might be expected, and users should be able to turn them off.

What solution would you like?

Allow users to disable this logging by disabling it at the system config level, which should work on both systems and system sets.

It would be nice to be able to change the log level to any of the standard levels: none, debug, info, warn, error, panic, but warn, and ignore are the most important.

What alternative(s) have you considered?

We could also expose a way to turn off these warnings globally, but this is likely to be a footgun: your systems silently not running is very bad!

We could also add resource / component-level log disabling, but that's more complex. My preferred solution there now is to add opt-in support for reads/writes system sets (based on https://github.com/bevyengine/bevy/pull/5388), and then use the mechanism above to configure them.

Additional context

Previously discussed in https://github.com/bevyengine/bevy/issues/15265.

[MiniaczQ]

Hmm, I'm not sure whether we need all the log levels, system skipping sounds like it should emit a single type Is there a use case for more?

[alice-i-cecile]

Hmm, that's fine. It can be useful for folks configuring their logging filtering extensively, but warn/panic/silent is fine.

[MiniaczQ]

This needs more thought, observers and run_system also need to validate params

I think we need to do the HashSet solution first to prevent spam and discuss before doing more

[MiniaczQ]

What about something like

#[derive(Default)]
enum WarnStatus {
  Never,
  #[default]
  Once,
  Always,
}

impl WarnStatus {
  pub fn warn(&mut self) -> bool {
    let (next, should_warn) = match self {
      Never => (Never, false),
      Once => (Never, true),
      Always => (Always, true),
    }
    *self = next;
    should_warn
  }
}

we can store this in SystemMeta which we can always access mutably within the system functions. Then configuration is a matter of exposing few additional configuration methods.

This means we don't store a hashset

[MiniaczQ]

Something we might consider is track exactly which system parameter failed validation behind a feature flag.

[MiniaczQ]

Okay, so #15500 adds the internals for warnings, but I couldn't figure out how to expose the configuration functions as following:

// Realistically we want shorthand methods: `.no_warns()`, etc.
app.add_systems(Update, my_system.with_validation_warning(ValidationWarning::Never));

closest I've got is

app.add_systems(Update, IntoSystem::into_system(my_system).with_validation_warning(ValidationWarning::Never));

Note that only function systems make sense here. Combined systems store multiple SystemMetas (just configure one of them) and exclusive systems always have valid params (world).

[MiniaczQ]

Also, about tracing exactly which param is failing, we could do right now if we move warnings into validate_param. Which I'm quite in favor of, I'll look into it.

We'll lose system/condition/observer information, but the name will be there so this is redundant, I'd rather know the exact param.