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:
		
							parent
							
								
									90f77beca2
								
							
						
					
					
						commit
						731a6fbb92
					
				@ -1113,7 +1113,7 @@ impl ScheduleGraph {
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
        let sys_count = self.systems.len();
 | 
					        let sys_count = self.systems.len();
 | 
				
			||||||
        let set_with_conditions_count = hg_set_ids.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
 | 
					        // get the number of dependencies and the immediate dependents of each system
 | 
				
			||||||
        // (needed by multi-threaded executor to run systems in the correct order)
 | 
					        // (needed by multi-threaded executor to run systems in the correct order)
 | 
				
			||||||
@ -1145,7 +1145,7 @@ impl ScheduleGraph {
 | 
				
			|||||||
            let bitset = &mut systems_in_sets_with_conditions[i];
 | 
					            let bitset = &mut systems_in_sets_with_conditions[i];
 | 
				
			||||||
            for &(col, sys_id) in &hg_systems {
 | 
					            for &(col, sys_id) in &hg_systems {
 | 
				
			||||||
                let idx = dg_system_idx_map[&sys_id];
 | 
					                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);
 | 
					                bitset.set(idx, is_descendant);
 | 
				
			||||||
            }
 | 
					            }
 | 
				
			||||||
        }
 | 
					        }
 | 
				
			||||||
@ -1160,7 +1160,7 @@ impl ScheduleGraph {
 | 
				
			|||||||
                .enumerate()
 | 
					                .enumerate()
 | 
				
			||||||
                .take_while(|&(_idx, &row)| row < col)
 | 
					                .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);
 | 
					                bitset.set(idx, is_ancestor);
 | 
				
			||||||
            }
 | 
					            }
 | 
				
			||||||
        }
 | 
					        }
 | 
				
			||||||
@ -1570,3 +1570,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);
 | 
				
			||||||
 | 
					    }
 | 
				
			||||||
 | 
					}
 | 
				
			||||||
 | 
				
			|||||||
		Loading…
	
		Reference in New Issue
	
	Block a user