Skip to content

Move more functions off of ScheduleGraph #21817

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+1,348 −670
Open

Move more functions off of ScheduleGraph #21817

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
16ec698
Move more functions off of `ScheduleGraph`
ItsDoot Oct 21, 2025
b2dbec0
markdownlint
ItsDoot Nov 12, 2025
84c0ca6
add more tests
ItsDoot Nov 16, 2025
e7903ec
fix var names
ItsDoot Nov 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions crates/bevy_ecs/src/schedule/auto_insert_apply_deferred.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use alloc::{boxed::Box, collections::BTreeSet, vec::Vec};

use bevy_platform::collections::HashMap;
use bevy_platform::collections::{HashMap, HashSet};

use crate::{
schedule::{SystemKey, SystemSetKey},
schedule::{graph::Dag, SystemKey, SystemSetKey},
system::{IntoSystem, System},
world::World,
};
Expand Down Expand Up @@ -72,11 +72,11 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass {
&mut self,
_world: &mut World,
graph: &mut ScheduleGraph,
dependency_flattened: &mut DiGraph<SystemKey>,
dependency_flattened: &mut Dag<SystemKey>,
) -> Result<(), ScheduleBuildError> {
let mut sync_point_graph = dependency_flattened.clone();
let topo = dependency_flattened
.toposort()
let mut sync_point_graph = dependency_flattened.graph().clone();
let (topo, flat_dependency) = dependency_flattened
.toposort_and_graph()
.map_err(ScheduleBuildError::FlatDependencySort)?;

fn set_has_conditions(graph: &ScheduleGraph, set: SystemSetKey) -> bool {
Expand Down Expand Up @@ -124,7 +124,7 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass {
let mut distance_to_explicit_sync_node: HashMap<u32, SystemKey> = HashMap::default();

// Determine the distance for every node and collect the explicit sync points.
for &key in &topo {
for &key in topo {
let (node_distance, mut node_needs_sync) = distances_and_pending_sync
.get(&key)
.copied()
Expand All @@ -146,7 +146,7 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass {
node_needs_sync = graph.systems[key].has_deferred();
}

for target in dependency_flattened.neighbors_directed(key, Direction::Outgoing) {
for target in flat_dependency.neighbors_directed(key, Direction::Outgoing) {
let (target_distance, target_pending_sync) =
distances_and_pending_sync.entry(target).or_default();

Expand Down Expand Up @@ -179,13 +179,13 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass {

// Find any edges which have a different number of sync points between them and make sure
// there is a sync point between them.
for &key in &topo {
for &key in topo {
let (node_distance, _) = distances_and_pending_sync
.get(&key)
.copied()
.unwrap_or_default();

for target in dependency_flattened.neighbors_directed(key, Direction::Outgoing) {
for target in flat_dependency.neighbors_directed(key, Direction::Outgoing) {
let (target_distance, _) = distances_and_pending_sync
.get(&target)
.copied()
Expand Down Expand Up @@ -215,14 +215,14 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass {
}
}

*dependency_flattened = sync_point_graph;
**dependency_flattened = sync_point_graph;
Ok(())
}

fn collapse_set(
&mut self,
set: SystemSetKey,
systems: &[SystemKey],
systems: &HashSet<SystemKey>,
dependency_flattening: &DiGraph<NodeId>,
) -> impl Iterator<Item = (NodeId, NodeId)> {
if systems.is_empty() {
Expand Down
53 changes: 31 additions & 22 deletions crates/bevy_ecs/src/schedule/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
use alloc::{format, string::String, vec::Vec};
use bevy_platform::collections::HashSet;
use core::fmt::Write as _;

use thiserror::Error;

use crate::{
component::{ComponentId, Components},
component::Components,
schedule::{
graph::{DiGraphToposortError, GraphNodeId},
NodeId, ScheduleGraph, SystemKey, SystemSetKey,
graph::{
DagCrossDependencyError, DagOverlappingGroupError, DagRedundancyError,
DiGraphToposortError, GraphNodeId,
},
AmbiguousSystemConflictsWarning, ConflictingSystems, NodeId, ScheduleGraph, SystemKey,
SystemSetKey, SystemTypeSetAmbiguityError,
},
world::World,
};
Expand All @@ -26,14 +31,14 @@ pub enum ScheduleBuildError {
#[error("Failed to topologically sort the flattened dependency graph: {0}")]
FlatDependencySort(DiGraphToposortError<SystemKey>),
/// Tried to order a system (set) relative to a system set it belongs to.
#[error("`{0:?}` and `{1:?}` have both `in_set` and `before`-`after` relationships (these might be transitive). This combination is unsolvable as a system cannot run before or after a set it belongs to.")]
CrossDependency(NodeId, NodeId),
#[error("`{:?}` and `{:?}` have both `in_set` and `before`-`after` relationships (these might be transitive). This combination is unsolvable as a system cannot run before or after a set it belongs to.", .0.0, .0.1)]
CrossDependency(#[from] DagCrossDependencyError<NodeId>),
/// Tried to order system sets that share systems.
#[error("`{0:?}` and `{1:?}` have a `before`-`after` relationship (which may be transitive) but share systems.")]
SetsHaveOrderButIntersect(SystemSetKey, SystemSetKey),
#[error("`{:?}` and `{:?}` have a `before`-`after` relationship (which may be transitive) but share systems.", .0.0, .0.1)]
SetsHaveOrderButIntersect(#[from] DagOverlappingGroupError<SystemSetKey>),
/// Tried to order a system (set) relative to all instances of some system function.
#[error("Tried to order against `{0:?}` in a schedule that has more than one `{0:?}` instance. `{0:?}` is a `SystemTypeSet` and cannot be used for ordering if ambiguous. Use a different set without this restriction.")]
SystemTypeSetAmbiguity(SystemSetKey),
#[error(transparent)]
SystemTypeSetAmbiguity(#[from] SystemTypeSetAmbiguityError),
/// Tried to run a schedule before all of its systems have been initialized.
#[error("Tried to run a schedule before all of its systems have been initialized.")]
Uninitialized,
Expand All @@ -56,7 +61,7 @@ pub enum ScheduleBuildWarning {
/// [`LogLevel::Ignore`]: crate::schedule::LogLevel::Ignore
/// [`LogLevel::Error`]: crate::schedule::LogLevel::Error
#[error("The hierarchy of system sets contains redundant edges: {0:?}")]
HierarchyRedundancy(Vec<(NodeId, NodeId)>),
HierarchyRedundancy(#[from] DagRedundancyError<NodeId>),
/// Systems with conflicting access have indeterminate run order.
///
/// This warning is **disabled** by default, but can be enabled by setting
Expand All @@ -66,8 +71,8 @@ pub enum ScheduleBuildWarning {
/// [`ScheduleBuildSettings::ambiguity_detection`]: crate::schedule::ScheduleBuildSettings::ambiguity_detection
/// [`LogLevel::Warn`]: crate::schedule::LogLevel::Warn
/// [`LogLevel::Error`]: crate::schedule::LogLevel::Error
#[error("Systems with conflicting access have indeterminate run order: {0:?}")]
Ambiguity(Vec<(SystemKey, SystemKey, Vec<ComponentId>)>),
#[error(transparent)]
Ambiguity(#[from] AmbiguousSystemConflictsWarning),
}

impl ScheduleBuildError {
Expand Down Expand Up @@ -101,13 +106,13 @@ impl ScheduleBuildError {
ScheduleBuildError::FlatDependencySort(DiGraphToposortError::Cycle(cycles)) => {
Self::dependency_cycle_to_string(cycles, graph)
}
ScheduleBuildError::CrossDependency(a, b) => {
Self::cross_dependency_to_string(a, b, graph)
ScheduleBuildError::CrossDependency(error) => {
Self::cross_dependency_to_string(error, graph)
}
ScheduleBuildError::SetsHaveOrderButIntersect(a, b) => {
ScheduleBuildError::SetsHaveOrderButIntersect(DagOverlappingGroupError(a, b)) => {
Self::sets_have_order_but_intersect_to_string(a, b, graph)
}
ScheduleBuildError::SystemTypeSetAmbiguity(set) => {
ScheduleBuildError::SystemTypeSetAmbiguity(SystemTypeSetAmbiguityError(set)) => {
Self::system_type_set_ambiguity_to_string(set, graph)
}
ScheduleBuildError::Uninitialized => Self::uninitialized_to_string(),
Expand Down Expand Up @@ -144,7 +149,7 @@ impl ScheduleBuildError {
}

fn hierarchy_redundancy_to_string(
transitive_edges: &[(NodeId, NodeId)],
transitive_edges: &HashSet<(NodeId, NodeId)>,
graph: &ScheduleGraph,
) -> String {
let mut message = String::from("hierarchy contains redundant edge(s)");
Expand Down Expand Up @@ -195,7 +200,11 @@ impl ScheduleBuildError {
message
}

fn cross_dependency_to_string(a: &NodeId, b: &NodeId, graph: &ScheduleGraph) -> String {
fn cross_dependency_to_string(
error: &DagCrossDependencyError<NodeId>,
graph: &ScheduleGraph,
) -> String {
let DagCrossDependencyError(a, b) = error;
format!(
"{} `{}` and {} `{}` have both `in_set` and `before`-`after` relationships (these might be transitive). \
This combination is unsolvable as a system cannot run before or after a set it belongs to.",
Expand Down Expand Up @@ -227,7 +236,7 @@ impl ScheduleBuildError {
}

pub(crate) fn ambiguity_to_string(
ambiguities: &[(SystemKey, SystemKey, Vec<ComponentId>)],
ambiguities: &ConflictingSystems,
graph: &ScheduleGraph,
components: &Components,
) -> String {
Expand All @@ -236,7 +245,7 @@ impl ScheduleBuildError {
"{n_ambiguities} pairs of systems with conflicting data access have indeterminate execution order. \
Consider adding `before`, `after`, or `ambiguous_with` relationships between these:\n",
);
let ambiguities = graph.conflicts_to_string(ambiguities, components);
let ambiguities = ambiguities.to_string(graph, components);
for (name_a, name_b, conflicts) in ambiguities {
writeln!(message, " -- {name_a} and {name_b}").unwrap();

Expand All @@ -261,10 +270,10 @@ impl ScheduleBuildWarning {
/// replaced with their names.
pub fn to_string(&self, graph: &ScheduleGraph, world: &World) -> String {
match self {
ScheduleBuildWarning::HierarchyRedundancy(transitive_edges) => {
ScheduleBuildWarning::HierarchyRedundancy(DagRedundancyError(transitive_edges)) => {
ScheduleBuildError::hierarchy_redundancy_to_string(transitive_edges, graph)
}
ScheduleBuildWarning::Ambiguity(ambiguities) => {
ScheduleBuildWarning::Ambiguity(AmbiguousSystemConflictsWarning(ambiguities)) => {
ScheduleBuildError::ambiguity_to_string(ambiguities, graph, world.components())
}
}
Expand Down
Loading