From d6f7469597c55298c606b61cdf10e412a8529ef2 Mon Sep 17 00:00:00 2001 From: waynemwashuma <94756970+waynemwashuma@users.noreply.github.com> Date: Tue, 26 May 2026 02:24:18 +0300 Subject: [PATCH 1/3] Involve empty system groups during ordering --- src/schedule/core/systembuilder.js | 114 +++++++++++++++++++++++------ 1 file changed, 91 insertions(+), 23 deletions(-) diff --git a/src/schedule/core/systembuilder.js b/src/schedule/core/systembuilder.js index e64ea397..aac0c009 100644 --- a/src/schedule/core/systembuilder.js +++ b/src/schedule/core/systembuilder.js @@ -219,7 +219,7 @@ export class SchedulerBuilder { * @returns {SystemRegistration[]} */ sortScheduleSystems(context) { - const graph = this.expandScheduleGraph(context) + const { graph, systemsByGraphId } = this.expandScheduleGraph(context) const sorted = kahnTopologySort(graph) if (!sorted) { @@ -230,9 +230,10 @@ export class SchedulerBuilder { const ordered = [] for (let i = 0; i < sorted.length; i++) { - const system = graph.getNodeWeight(sorted[i]) + const system = systemsByGraphId.get(sorted[i]) + + if (!system) continue - assert(system, `Internal error: Could not resolve system node ${sorted[i]} on schedule "${context.label}".`) ordered.push(system) } @@ -242,25 +243,41 @@ export class SchedulerBuilder { /** * @private * @param {ScheduleContext} context - * @returns {Graph} + * @returns {{ graph: Graph, systemsByGraphId: Map }} */ expandScheduleGraph(context) { - const graph = /** @type {Graph} */ (new Graph(true)) + const graph = /** @type {Graph} */ (new Graph(true)) /** @type {Map} */ const graphIdsBySystemId = new Map() + /** @type {Map} */ + const graphIdsByGroupId = new Map() + + context.graphIdsByGroupId = graphIdsByGroupId + + /** @type {Map} */ + const systemsByGraphId = new Map() + /** @type {Map} */ const groupSystemsCache = new Map() /** @type {Set} */ const edges = new Set() + for (let i = 0; i < context.groups.length; i++) { + const group = context.groups[i] + const graphId = graph.addNode(group) + + graphIdsByGroupId.set(group.id, graphId) + } + for (let i = 0; i < context.systems.length; i++) { const system = context.systems[i] const graphId = graph.addNode(system) graphIdsBySystemId.set(system.id, graphId) + systemsByGraphId.set(graphId, system) } for (let i = 0; i < context.systems.length; i++) { @@ -281,13 +298,16 @@ export class SchedulerBuilder { }, group.config.before, group.config.after, groupSystemsCache) } - return graph + return { + graph, + systemsByGraphId + } } /** * @private * @param {ScheduleContext} context - * @param {Graph} graph + * @param {Graph} graph * @param {Map} graphIdsBySystemId * @param {Set} edges * @param {ScheduleNodeRef} source @@ -336,7 +356,7 @@ export class SchedulerBuilder { /** * @private * @param {ScheduleContext} context - * @param {Graph} graph + * @param {Graph} graph * @param {Map} graphIdsBySystemId * @param {Set} edges * @param {ScheduleNodeRef} from @@ -345,32 +365,78 @@ export class SchedulerBuilder { * @param {Map} groupSystemsCache */ addExpandedEdge(context, graph, graphIdsBySystemId, edges, from, to, targetLabel, groupSystemsCache) { - const fromSystems = this.expandNodeToSystems(context, from, groupSystemsCache) - const toSystems = this.expandNodeToSystems(context, to, groupSystemsCache) + const fromNodes = this.expandNodeToOrderingNodes(context, from, graphIdsBySystemId, groupSystemsCache) + const toNodes = this.expandNodeToOrderingNodes(context, to, graphIdsBySystemId, groupSystemsCache) - for (let i = 0; i < fromSystems.length; i++) { - for (let j = 0; j < toSystems.length; j++) { - const fromSystemId = fromSystems[i] - const toSystemId = toSystems[j] + for (let i = 0; i < fromNodes.length; i++) { + for (let j = 0; j < toNodes.length; j++) { + const fromNodeId = fromNodes[i] + const toNodeId = toNodes[j] - if (fromSystemId === toSystemId) { + if (fromNodeId === toNodeId) { throws(`The reference "${describeReference(targetLabel)}" creates a self-referential system ordering on schedule "${context.label}".`) } - const graphFrom = graphIdsBySystemId.get(fromSystemId) - const graphTo = graphIdsBySystemId.get(toSystemId) - - assert(graphFrom !== undefined, `Internal error: Could not resolve graph node for system ${fromSystemId} on schedule "${context.label}".`) - assert(graphTo !== undefined, `Internal error: Could not resolve graph node for system ${toSystemId} on schedule "${context.label}".`) - - const key = `${graphFrom}:${graphTo}` + const key = `${fromNodeId}:${toNodeId}` if (edges.has(key)) continue edges.add(key) - graph.addEdge(graphFrom, graphTo, undefined) + graph.addEdge(fromNodeId, toNodeId, undefined) + } + } + } + + /** + * @private + * @param {ScheduleContext} context + * @param {ScheduleNodeRef} node + * @param {Map} graphIdsBySystemId + * @param {Map} groupSystemsCache + * @returns {number[]} + */ + expandNodeToOrderingNodes(context, node, graphIdsBySystemId, groupSystemsCache) { + if (node.kind === ScheduleNodeKind.System) { + const graphId = graphIdsBySystemId.get(node.id) + + assert(graphId !== undefined, `Internal error: Could not resolve graph node for system ${node.id} on schedule "${context.label}".`) + + return [graphId] + } + + const systems = this.expandGroupToSystems(context, node.id, groupSystemsCache, new Set()) + + if (systems.length > 0) { + /** @type {number[]} */ + const graphIds = [] + + for (let i = 0; i < systems.length; i++) { + const graphId = graphIdsBySystemId.get(systems[i]) + + assert(graphId !== undefined, `Internal error: Could not resolve graph node for system ${systems[i]} on schedule "${context.label}".`) + graphIds.push(graphId) } + + return graphIds } + + const graphId = this.expandGroupToGraphNode(context, node.id) + + return [graphId] + } + + /** + * @private + * @param {ScheduleContext} context + * @param {number} groupId + * @returns {number} + */ + expandGroupToGraphNode(context, groupId) { + const graphId = context.graphIdsByGroupId?.get(groupId) + + assert(graphId !== undefined, `Internal error: Could not resolve graph node for system group ${groupId} on schedule "${context.label}".`) + + return graphId } /** @@ -445,6 +511,7 @@ function getOrCreateScheduleContext(schedules, label) { groups: [], nodesByLabel: new Map(), groupIdsByTypeId: new Map(), + graphIdsByGroupId: undefined, defaultSystemGroup: undefined }) @@ -475,6 +542,7 @@ const ScheduleNodeKind = Object.freeze({ * @property {SystemGroupRegistration[]} groups * @property {Map} nodesByLabel * @property {Map} groupIdsByTypeId + * @property {Map | undefined} graphIdsByGroupId * @property {Constructor | undefined} defaultSystemGroup */ From dc48f91bbef01a1d2b590431d33649a5208921fa Mon Sep 17 00:00:00 2001 From: waynemwashuma <94756970+waynemwashuma@users.noreply.github.com> Date: Tue, 26 May 2026 02:24:28 +0300 Subject: [PATCH 2/3] Add new test case --- src/schedule/tests/scheduleBuilder.test.js | 47 ++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/src/schedule/tests/scheduleBuilder.test.js b/src/schedule/tests/scheduleBuilder.test.js index a94836af..b0680d49 100644 --- a/src/schedule/tests/scheduleBuilder.test.js +++ b/src/schedule/tests/scheduleBuilder.test.js @@ -334,6 +334,53 @@ describe('Testing `SchedulerBuilder`', () => { deepStrictEqual(order, ['earlyOne', 'earlyTwo', 'lateOne', 'lateTwo']) }) + test('treats empty groups as ordering barriers', () => { + const builder = new SchedulerBuilder() + const scheduler = new Scheduler() + const world = new World() + /** @type {string[]} */ + const order = [] + + class StartPhase { } + class MiddlePhase { } + class EndPhase { } + + function startSystem() { order.push('start') } + function endSystem() { order.push('end') } + + scheduler.set(new Executable({ label: 'update' })) + + builder.addGroup({ + label: StartPhase, + schedule: 'update', + before: [MiddlePhase] + }) + builder.addGroup({ + label: MiddlePhase, + schedule: 'update', + before: [EndPhase] + }) + builder.addGroup({ + label: EndPhase, + schedule: 'update' + }) + builder.add({ + schedule: 'update', + systemGroup: EndPhase, + system: endSystem + }) + builder.add({ + schedule: 'update', + systemGroup: StartPhase, + system: startSystem + }) + + builder.pushToScheduler(scheduler) + scheduler.get('update')?.run(world) + + deepStrictEqual(order, ['start', 'end']) + }) + test('inherits parent ordering constraints across descendants', () => { const builder = new SchedulerBuilder() const scheduler = new Scheduler() From fe9fd20166ac3f18525726c03d9e6c7a306f11f3 Mon Sep 17 00:00:00 2001 From: waynemwashuma <94756970+waynemwashuma@users.noreply.github.com> Date: Tue, 26 May 2026 02:33:26 +0300 Subject: [PATCH 3/3] lint files --- src/schedule/core/systembuilder.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/schedule/core/systembuilder.js b/src/schedule/core/systembuilder.js index aac0c009..4ad9de85 100644 --- a/src/schedule/core/systembuilder.js +++ b/src/schedule/core/systembuilder.js @@ -407,6 +407,7 @@ export class SchedulerBuilder { const systems = this.expandGroupToSystems(context, node.id, groupSystemsCache, new Set()) if (systems.length > 0) { + /** @type {number[]} */ const graphIds = []