diff --git a/package-lock.json b/package-lock.json index ccdbb1ec..10c8be5e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,7 +9,7 @@ "version": "0.3.0", "license": "SEE LICENSE in LICENSE.txt", "dependencies": { - "vifaa": "^0.1.0" + "vifaa": "^0.2.0" }, "devDependencies": { "@rollup/plugin-terser": "^1.0.0", @@ -4603,9 +4603,9 @@ } }, "node_modules/vifaa": { - "version": "0.1.0", - "resolved": "https://registry.npmjs.org/vifaa/-/vifaa-0.1.0.tgz", - "integrity": "sha512-Ojqrj/kk88E8jSDBFL/bueN2bPPB2Dnl+WMriSjc/X87/y3tCyVaxRU4SgjBXhAXiXfkrL8PgFhlXVBKS1ssjQ==", + "version": "0.2.0", + "resolved": "https://registry.npmjs.org/vifaa/-/vifaa-0.2.0.tgz", + "integrity": "sha512-1GsQfsNZiSuqo7xOkRHjTDFjLZ90BEDrjPPEH1dw/J03u43HaUAxdb1tfjMYSkzg2acyuzl+pxWAMrsBlNP28g==", "license": "MIT" }, "node_modules/vite": { diff --git a/package.json b/package.json index e7a17237..bf8fffb6 100644 --- a/package.json +++ b/package.json @@ -72,6 +72,6 @@ "dist/*" ], "dependencies": { - "vifaa": "^0.1.0" + "vifaa": "^0.2.0" } } diff --git a/src/schedule/core/systembuilder.js b/src/schedule/core/systembuilder.js index 695952b5..34d0b57e 100644 --- a/src/schedule/core/systembuilder.js +++ b/src/schedule/core/systembuilder.js @@ -72,6 +72,7 @@ export class SchedulerBuilder { const group = { id: context.groups.length, config, + parentId: undefined, systems: [] } @@ -101,9 +102,6 @@ export class SchedulerBuilder { } context.systems.push(system) - context.systemIdsByFunc.set(systemLabel, system.id) - - // context.systemIdsByFunc.set(systemLabel, system.id) // Removed as it's no longer used if (systemLabel !== '') { const existing = context.nodesByLabel.get(systemLabel) @@ -117,6 +115,8 @@ export class SchedulerBuilder { } for (const [scheduleLabel, context] of schedules) { + this.resolveGroupParents(context, scheduleLabel) + for (let i = 0; i < context.systems.length; i++) { const system = context.systems[i] const groupLabel = system.config.systemGroup @@ -136,6 +136,72 @@ export class SchedulerBuilder { return schedules } + /** + * @private + * @param {ScheduleContext} context + * @param {string} scheduleLabel + */ + resolveGroupParents(context, scheduleLabel) { + for (let i = 0; i < context.groups.length; i++) { + const group = context.groups[i] + const parentLabel = group.config.parent + + if (!parentLabel) continue + + const parentId = context.groupIdsByTypeId.get(typeid(parentLabel)) + + if (parentId === undefined) { + throws(`The parent system group "${parentLabel.name}" must be registered explicitly before it can be used on schedule "${scheduleLabel}".`) + } + + group.parentId = parentId + } + + this.assertNoGroupCycles(context) + } + + /** + * @private + * @param {ScheduleContext} context + */ + assertNoGroupCycles(context) { + + /** @type {GroupVisitState[]} */ + const state = new Array(context.groups.length).fill(GroupVisitState.Unvisited) + + for (let i = 0; i < context.groups.length; i++) { + this.visitGroupHierarchy(context, i, state) + } + } + + /** + * @private + * @param {ScheduleContext} context + * @param {number} groupId + * @param {GroupVisitState[]} state + */ + visitGroupHierarchy(context, groupId, state) { + const visitState = state[groupId] + + if (visitState === GroupVisitState.Visiting) { + const group = context.groups[groupId] + + throws(`Schedule "${context.label}" contains cyclic system group nesting involving "${group.config.label.name}".`) + } + + if (visitState === GroupVisitState.Visited) return + + state[groupId] = GroupVisitState.Visiting + + const { parentId } = context.groups[groupId] + + if (parentId !== undefined) { + this.visitGroupHierarchy(context, parentId, state) + } + + state[groupId] = GroupVisitState.Visited + } + /** * @private * @param {ScheduleContext} context @@ -173,6 +239,9 @@ export class SchedulerBuilder { /** @type {Map} */ const graphIdsBySystemId = new Map() + /** @type {Map} */ + const groupSystemsCache = new Map() + /** @type {Set} */ const edges = new Set() @@ -189,7 +258,7 @@ export class SchedulerBuilder { this.addNodeOrdering(context, graph, graphIdsBySystemId, edges, { kind: ScheduleNodeKind.System, id: system.id - }, system.config.before, system.config.after) + }, system.config.before, system.config.after, groupSystemsCache) } for (let i = 0; i < context.groups.length; i++) { @@ -198,7 +267,7 @@ export class SchedulerBuilder { this.addNodeOrdering(context, graph, graphIdsBySystemId, edges, { kind: ScheduleNodeKind.Group, id: group.id - }, group.config.before, group.config.after) + }, group.config.before, group.config.after, groupSystemsCache) } return graph @@ -213,15 +282,16 @@ export class SchedulerBuilder { * @param {ScheduleNodeRef} source * @param {(SystemFunc | Constructor | string)[] | undefined} before * @param {(SystemFunc | Constructor | string)[] | undefined} after + * @param {Map} groupSystemsCache */ - addNodeOrdering(context, graph, graphIdsBySystemId, edges, source, before, after) { + addNodeOrdering(context, graph, graphIdsBySystemId, edges, source, before, after, groupSystemsCache) { if (before) { for (let i = 0; i < before.length; i++) { const label = describeReference(before[i]) - this.addExpandedEdge(context, graph, graphIdsBySystemId, edges, source, this.resolveNode(context, label), before[i]) - this.addExpandedEdge(context, graph, graphIdsBySystemId, edges, source, this.resolveNode(context, label), label) + this.addExpandedEdge(context, graph, graphIdsBySystemId, edges, source, this.resolveNode(context, label), before[i], groupSystemsCache) + this.addExpandedEdge(context, graph, graphIdsBySystemId, edges, source, this.resolveNode(context, label), label, groupSystemsCache) } } @@ -229,8 +299,8 @@ export class SchedulerBuilder { for (let i = 0; i < after.length; i++) { const label = describeReference(after[i]) - this.addExpandedEdge(context, graph, graphIdsBySystemId, edges, this.resolveNode(context, label), source, after[i]) - this.addExpandedEdge(context, graph, graphIdsBySystemId, edges, this.resolveNode(context, label), source, label) + this.addExpandedEdge(context, graph, graphIdsBySystemId, edges, this.resolveNode(context, label), source, after[i], groupSystemsCache) + this.addExpandedEdge(context, graph, graphIdsBySystemId, edges, this.resolveNode(context, label), source, label, groupSystemsCache) } } } @@ -261,10 +331,11 @@ export class SchedulerBuilder { * @param {ScheduleNodeRef} from * @param {ScheduleNodeRef} to * @param {SystemFunc | Constructor | string} targetLabel + * @param {Map} groupSystemsCache */ - addExpandedEdge(context, graph, graphIdsBySystemId, edges, from, to, targetLabel) { - const fromSystems = this.expandNodeToSystems(context, from) - const toSystems = this.expandNodeToSystems(context, to) + addExpandedEdge(context, graph, graphIdsBySystemId, edges, from, to, targetLabel, groupSystemsCache) { + const fromSystems = this.expandNodeToSystems(context, from, groupSystemsCache) + const toSystems = this.expandNodeToSystems(context, to, groupSystemsCache) for (let i = 0; i < fromSystems.length; i++) { for (let j = 0; j < toSystems.length; j++) { @@ -295,12 +366,55 @@ export class SchedulerBuilder { * @private * @param {ScheduleContext} context * @param {ScheduleNodeRef} node + * @param {Map} groupSystemsCache * @returns {number[]} */ - expandNodeToSystems(context, node) { + expandNodeToSystems(context, node, groupSystemsCache) { if (node.kind === ScheduleNodeKind.System) return [node.id] - return context.groups[node.id].systems + return this.expandGroupToSystems(context, node.id, groupSystemsCache, new Set()) + } + + /** + * @private + * @param {ScheduleContext} context + * @param {number} groupId + * @param {Map} cache + * @param {Set} visiting + * @returns {number[]} + */ + expandGroupToSystems(context, groupId, cache, visiting) { + const cached = cache.get(groupId) + + if (cached) return cached + + if (visiting.has(groupId)) { + const group = context.groups[groupId] + + throws(`Schedule "${context.label}" contains cyclic system group nesting involving "${group.config.label.name}".`) + } + + visiting.add(groupId) + + /** @type {number[]} */ + const systems = [...context.groups[groupId].systems] + + for (let i = 0; i < context.groups.length; i++) { + const child = context.groups[i] + + if (child.parentId !== groupId) continue + + const childSystems = this.expandGroupToSystems(context, child.id, cache, visiting) + + for (let j = 0; j < childSystems.length; j++) { + systems.push(childSystems[j]) + } + } + + visiting.delete(groupId) + cache.set(groupId, systems) + + return systems } } @@ -319,10 +433,7 @@ function getOrCreateScheduleContext(schedules, label) { systems: [], groups: [], nodesByLabel: new Map(), - groupIdsByTypeId: new Map(), - systemIdsByFunc: new Map() - - // systemIdsByFunc: new Map() // Removed as it's no longer used + groupIdsByTypeId: new Map() }) schedules.set(label, created) @@ -352,7 +463,6 @@ const ScheduleNodeKind = Object.freeze({ * @property {SystemGroupRegistration[]} groups * @property {Map} nodesByLabel * @property {Map} groupIdsByTypeId - * @property {Map} systemIdsByFunc */ /** @@ -365,6 +475,7 @@ const ScheduleNodeKind = Object.freeze({ * @typedef SystemGroupRegistration * @property {number} id * @property {SystemGroupConfig} config + * @property {number | undefined} parentId * @property {number[]} systems */ @@ -373,3 +484,12 @@ const ScheduleNodeKind = Object.freeze({ * @property {ScheduleNodeKind} kind * @property {number} id */ + +/** + * @enum {number} + */ +const GroupVisitState = Object.freeze({ + Unvisited: 0, + Visiting: 1, + Visited: 2 +}) diff --git a/src/schedule/core/systemconfig.js b/src/schedule/core/systemconfig.js index c5d0bd37..39a80f3d 100644 --- a/src/schedule/core/systemconfig.js +++ b/src/schedule/core/systemconfig.js @@ -19,6 +19,7 @@ * @typedef SystemGroupConfig * @property {Constructor} label * @property {string} schedule + * @property {Constructor | undefined} [parent] * @property {SystemOrderReference[] | undefined} [before] * @property {SystemOrderReference[] | undefined} [after] */ diff --git a/src/schedule/tests/scheduleBuilder.test.js b/src/schedule/tests/scheduleBuilder.test.js index 0703881f..7bf15b25 100644 --- a/src/schedule/tests/scheduleBuilder.test.js +++ b/src/schedule/tests/scheduleBuilder.test.js @@ -4,6 +4,8 @@ import { World } from '../../ecs/index.js' import { Executable, Scheduler, SchedulerBuilder } from '../index.js' class Phase { } +class ParentPhase { } +class ChildPhase { } class MissingPhase { } describe('Testing `SchedulerBuilder`', () => { @@ -232,6 +234,119 @@ describe('Testing `SchedulerBuilder`', () => { deepStrictEqual(order, ['earlyOne', 'earlyTwo', 'lateOne', 'lateTwo']) }) + test('inherits parent ordering constraints across descendants', () => { + const builder = new SchedulerBuilder() + const scheduler = new Scheduler() + const world = new World() + /** @type {string[]} */ + const order = [] + + class RootPhase { } + class NestedPhase { } + + function head() { order.push('head') } + function middle() { order.push('middle') } + function tail() { order.push('tail') } + function nested() { order.push('nested') } + + scheduler.set(new Executable({ label: 'update' })) + + builder.addGroup({ + label: RootPhase, + schedule: 'update', + after: [head], + before: [middle, tail] + }) + builder.addGroup({ + label: NestedPhase, + parent: RootPhase, + schedule: 'update' + }) + builder.add({ + schedule: 'update', + system: head + }) + builder.add({ + schedule: 'update', + before: [tail], + system: middle + }) + builder.add({ + schedule: 'update', + systemGroup: NestedPhase, + system: nested + }) + builder.add({ + schedule: 'update', + system: tail + }) + + builder.pushToScheduler(scheduler) + scheduler.get('update')?.run(world) + + deepStrictEqual(order, ['head', 'nested', 'middle', 'tail']) + }) + + test('combines inherited constraints from multiple ancestors', () => { + const builder = new SchedulerBuilder() + const scheduler = new Scheduler() + const world = new World() + /** @type {string[]} */ + const order = [] + + class GrandPhase { } + class ParentPhase { } + class ChildPhase { } + + function head() { order.push('head') } + function middle() { order.push('middle') } + function tail() { order.push('tail') } + function nested() { order.push('nested') } + + scheduler.set(new Executable({ label: 'update' })) + + builder.addGroup({ + label: GrandPhase, + schedule: 'update', + before: [tail] + }) + builder.addGroup({ + label: ParentPhase, + parent: GrandPhase, + schedule: 'update', + after: [head], + before: [middle] + }) + builder.addGroup({ + label: ChildPhase, + parent: ParentPhase, + schedule: 'update' + }) + builder.add({ + schedule: 'update', + system: head + }) + builder.add({ + schedule: 'update', + before: [tail], + system: middle + }) + builder.add({ + schedule: 'update', + systemGroup: ChildPhase, + system: nested + }) + builder.add({ + schedule: 'update', + system: tail + }) + + builder.pushToScheduler(scheduler) + scheduler.get('update')?.run(world) + + deepStrictEqual(order, ['head', 'nested', 'middle', 'tail']) + }) + test('keeps schedules isolated from each other', () => { const builder = new SchedulerBuilder() const scheduler = new Scheduler() @@ -328,6 +443,39 @@ describe('Testing `SchedulerBuilder`', () => { throws(() => builder.pushToScheduler(scheduler), /must be registered explicitly/) }) + test('requires parent groups to be registered explicitly', () => { + const builder = new SchedulerBuilder() + const scheduler = new Scheduler() + + scheduler.set(new Executable({ label: 'update' })) + builder.addGroup({ + label: ParentPhase, + parent: MissingPhase, + schedule: 'update' + }) + + throws(() => builder.pushToScheduler(scheduler), /parent system group/) + }) + + test('rejects cyclic group nesting', () => { + const builder = new SchedulerBuilder() + const scheduler = new Scheduler() + + scheduler.set(new Executable({ label: 'update' })) + builder.addGroup({ + label: ParentPhase, + parent: ChildPhase, + schedule: 'update' + }) + builder.addGroup({ + label: ChildPhase, + parent: ParentPhase, + schedule: 'update' + }) + + throws(() => builder.pushToScheduler(scheduler), /cyclic system group nesting/) + }) + test('detects cyclic ordering constraints', () => { const builder = new SchedulerBuilder() const scheduler = new Scheduler()