System ambiguity detection does not take query filters into account

[jannik4]

Bevy version

0.12.1

What you did

Use a custom schedule with ambiguity_detection set to LogLevel::Error. Insert two systems that are disjoint by using Without<T>.

use bevy::{
    ecs::schedule::{ScheduleBuildSettings, ScheduleLabel},
    prelude::*,
};

fn main() {
    let mut app = App::new();

    app.add_plugins(DefaultPlugins);
    app.edit_schedule(MySchedule, |schedule| {
        schedule.set_build_settings(ScheduleBuildSettings {
            ambiguity_detection: bevy::ecs::schedule::LogLevel::Error,
            ..schedule.get_build_settings()
        });
    });

    app.add_systems(MySchedule, (a, b));
    app.add_systems(Update, |world: &mut World| world.run_schedule(MySchedule));

    app.run();
}

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, ScheduleLabel)]
struct MySchedule;

#[derive(Component)]
struct Shared;

#[derive(Component)]
struct Disjoint;

fn a(_query: Query<&mut Shared, &Disjoint>) {}

fn b(_query: Query<&mut Shared, Without<Disjoint>>) {}

What went wrong

bevy panicked, although the queries are disjoint and therefore not ambiguous:

thread 'main' panicked at C:\Users\Jannik\.cargo\registry\src\index.crates.io-6f17d22bba15001f\bevy_ecs-0.12.1\src\schedule\schedule.rs:290:33:
Error when initializing schedule MySchedule: Systems with conflicting access have indeterminate run order.
1 pairs of systems with conflicting data access have indeterminate execution order. Consider adding `before`, `after`, or `ambiguous_with` relationships between these:
 -- b and a
    conflict on: ["debug_tick_schedule::Shared"]

Additional information

Looking a little bit into the code, I noticed bevy has both Access<T> and FilteredAccess<T> to check for conflicts. But it seems it only uses only the Access<T> to check for conflicts here:

bevy/crates/bevy_ecs/src/schedule/schedule.rs

Lines 1321 to 1339 in f4dab8a

	let system_a = self.systems[a.index()].get().unwrap();
	let system_b = self.systems[b.index()].get().unwrap();
	if system_a.is_exclusive() || system_b.is_exclusive() {
	conflicting_systems.push((a, b, Vec::new()));
	} else {
	let access_a = system_a.component_access();
	let access_b = system_b.component_access();
	if !access_a.is_compatible(access_b) {
	let conflicts: Vec<_> = access_a
	.get_conflicts(access_b)
	.into_iter()
	.filter(|id| !ignored_ambiguities.contains(id))
	.collect();
	if !conflicts.is_empty() {
	conflicting_systems.push((a, b, conflicts));
	}
	}
	}

I don't know if this is just an oversight or if there's a good reason for it....

[alice-i-cecile]

This is an oversight / bug: we should be detecting these via FilteredAccess.

[rparrett]

Is this a duplicate of #4381?

[jannik4]

Looks like it yes. Now that #4381 is almost two years old, I wonder if something has changed so that this problem is easier to solve now?

[alice-i-cecile]

The ambiguity detection internals are meaningfully cleaner now: I suspect this will be easier to tackle now.