Fix ambiguous_with breaking run conditions (#9253)

# Objective

- Fixes #9114

## Solution

Inside `ScheduleGraph::build_schedule()` the variable `node_count =
self.systems.len() + self.system_sets.len()` is used to calculate the
indices for the `reachable` bitset derived from `self.hierarchy.graph`.
However, the number of nodes inside `self.hierarchy.graph` does not
always correspond to `self.systems.len() + self.system_sets.len()` when
`ambiguous_with` is used, because an ambiguous set is added to
`system_sets` (because we need an `NodeId` for the ambiguity graph)
without adding a node to `self.hierarchy`.

In this PR, we rename `node_count` to the more descriptive name
`hg_node_count` and set it to `self.hierarchy.graph.node_count()`.

---------

Co-authored-by: James Liu <contact@jamessliu.com>
This commit is contained in:
Edgar Geier 2023-08-03 09:53:20 +02:00 committed by Carter Anderson
parent 5b379d98e6
commit fefa8ea53b

View file

@ -1120,7 +1120,7 @@ impl ScheduleGraph {
let sys_count = self.systems.len();
let set_with_conditions_count = hg_set_ids.len();
let node_count = self.systems.len() + self.system_sets.len();
let hg_node_count = self.hierarchy.graph.node_count();
// get the number of dependencies and the immediate dependents of each system
// (needed by multi-threaded executor to run systems in the correct order)
@ -1152,7 +1152,7 @@ impl ScheduleGraph {
let bitset = &mut systems_in_sets_with_conditions[i];
for &(col, sys_id) in &hg_systems {
let idx = dg_system_idx_map[&sys_id];
let is_descendant = hier_results.reachable[index(row, col, node_count)];
let is_descendant = hier_results.reachable[index(row, col, hg_node_count)];
bitset.set(idx, is_descendant);
}
}
@ -1167,7 +1167,7 @@ impl ScheduleGraph {
.enumerate()
.take_while(|&(_idx, &row)| row < col)
{
let is_ancestor = hier_results.reachable[index(row, col, node_count)];
let is_ancestor = hier_results.reachable[index(row, col, hg_node_count)];
bitset.set(idx, is_ancestor);
}
}
@ -1577,3 +1577,30 @@ impl ScheduleBuildSettings {
}
}
}
#[cfg(test)]
mod tests {
use crate::{
self as bevy_ecs,
schedule::{IntoSystemConfigs, IntoSystemSetConfig, Schedule, SystemSet},
world::World,
};
// regression test for https://github.com/bevyengine/bevy/issues/9114
#[test]
fn ambiguous_with_not_breaking_run_conditions() {
#[derive(SystemSet, Debug, Clone, PartialEq, Eq, Hash)]
struct Set;
let mut world = World::new();
let mut schedule = Schedule::new();
schedule.configure_set(Set.run_if(|| false));
schedule.add_systems(
(|| panic!("This system must not run"))
.ambiguous_with(|| ())
.in_set(Set),
);
schedule.run(&mut world);
}
}