diff --git a/lib/Loop.lua b/lib/Loop.lua index b76a3b7b..84252ce1 100644 --- a/lib/Loop.lua +++ b/lib/Loop.lua @@ -132,7 +132,9 @@ type System = (...any) -> () | { system: (...any) -> (), event: string?, priorit ]=] function Loop:scheduleSystems(systems: { System }) for _, system in ipairs(systems) do - self._systems[system] = system + if table.find(self._systems, system) == nil then + table.insert(self._systems, system) + end self._systemState[system] = {} if RunService:IsStudio() then @@ -143,7 +145,6 @@ function Loop:scheduleSystems(systems: { System }) self:_sortSystems() end - --[=[ Schedules a single system. This is an expensive function to call multiple times. Instead, try batch scheduling systems with [Loop:scheduleSystems] if possible. @@ -162,11 +163,14 @@ end @param system System ]=] function Loop:evictSystem(system: System) - if self._systems[system] == nil then + local systemIndex = table.find(self._systems, system) + + if systemIndex == nil then error("Can't evict system because it doesn't exist") end - self._systems[system] = nil + table.remove(self._systems, systemIndex) + self._systemErrors[system] = nil topoRuntime.start({ @@ -187,12 +191,15 @@ end @param new System ]=] function Loop:replaceSystem(old: System, new: System) - if not self._systems[old] then + local systemIndex = table.find(self._systems, old) + + if not systemIndex then error("Before system does not exist!") end - self._systems[new] = new - self._systems[old] = nil + table.remove(self._systems, systemIndex) + table.insert(self._systems, new) + self._systemState[new] = self._systemState[old] or {} self._systemState[old] = nil @@ -201,7 +208,7 @@ function Loop:replaceSystem(old: System, new: System) self._skipSystems[new] = true end - for system in self._systems do + for _, system in self._systems do if type(system) == "table" and system.after then local index = table.find(system.after, old) @@ -215,12 +222,21 @@ function Loop:replaceSystem(old: System, new: System) end local function orderSystemsByDependencies(unscheduledSystems: { System }) - table.sort(unscheduledSystems, function(a, b) + local sortedUnscheduledSystems = table.clone(unscheduledSystems) + + table.sort(sortedUnscheduledSystems, function(a, b) local priorityA = systemPriority(a) local priorityB = systemPriority(b) if priorityA == priorityB then - return systemName(a) < systemName(b) + local nameA = systemName(a) + local nameB = systemName(b) + + if nameA == nameB then + return table.find(unscheduledSystems, a) < table.find(unscheduledSystems, b) + end + + return nameA < nameB end return priorityA < priorityB @@ -229,43 +245,33 @@ local function orderSystemsByDependencies(unscheduledSystems: { System }) local scheduledSystemsSet = {} local scheduledSystems = {} - local explore = 1 - local visited = 2 - - while #scheduledSystems < #unscheduledSystems do - local index = 1 + local visited, explored = 1, 2 - while index <= #unscheduledSystems do - local system = unscheduledSystems[index] + local function scheduleSystem(system) + scheduledSystemsSet[system] = visited - if scheduledSystemsSet[system] == visited then - index += 1 - continue - end - - scheduledSystemsSet[system] = explore - - local allScheduled = true - - if type(system) == "table" and system.after then - for _, dependency in ipairs(system.after) do - if scheduledSystemsSet[dependency] == explore then - error("Unable to schedule systems due to cycle") - elseif scheduledSystemsSet[dependency] ~= visited then - allScheduled = false - break - end + if type(system) == "table" and system.after then + for _, dependency in system.after do + if scheduledSystemsSet[dependency] == nil then + scheduleSystem(dependency) + elseif scheduledSystemsSet[dependency] == visited then + error( + `Unable to schedule systems due to cyclic dependency between: \n{systemName(system)} \nAND \n{systemName( + dependency + )}` + ) end end + end - if allScheduled then - scheduledSystemsSet[system] = visited - table.insert(scheduledSystems, system) - --Once this system is scheduled we want to start from the beginning to schedule systems that have a dependency on this system - break - end + scheduledSystemsSet[system] = explored + + table.insert(scheduledSystems, system) + end - index += 1 + for _, system in sortedUnscheduledSystems do + if scheduledSystemsSet[system] == nil then + scheduleSystem(system) end end @@ -275,7 +281,7 @@ end function Loop:_sortSystems() local systemsByEvent = {} - for system in pairs(self._systems) do + for _, system in pairs(self._systems) do local eventName = "default" if type(system) == "table" then @@ -294,7 +300,7 @@ function Loop:_sortSystems() end for _, dependency in system.after do - if not self._systems[dependency] then + if not table.find(self._systems, dependency) then error( `Unable to schedule "{systemName(system)}" because the system "{systemName(dependency)}" is not scheduled.\n\nEither schedule "{systemName( dependency diff --git a/lib/Loop.spec.lua b/lib/Loop.spec.lua index 49ab9d78..ce871ceb 100644 --- a/lib/Loop.spec.lua +++ b/lib/Loop.spec.lua @@ -164,6 +164,137 @@ return function() connection.default:Disconnect() end) + it("should not schedule systems more than once", function() + local loop = Loop.new() + + local order = {} + local systemA = { + system = function() + table.insert(order, "a") + end, + t = 1, + } + + local systemB = { + system = function() + table.insert(order, "b") + end, + t = 2, + } + + loop:scheduleSystems({ + systemA, + systemA, + }) + + loop:scheduleSystem(systemB) + loop:scheduleSystem(systemB) + + local connection = loop:begin({ default = bindable.Event }) + + expect(#order).to.equal(0) + + bindable:Fire() + + expect(#order).to.equal(2) + expect(order[1]).to.equal("a") + expect(order[2]).to.equal("b") + + connection.default:Disconnect() + end) + + it("should schedule systems in order if dependencies are defined", function() + local loop = Loop.new() + + local order = {} + local systemA = { + system = function() + table.insert(order, "a") + end, + } + + local function namedSystemB() + table.insert(order, "b") + end + + local systemB = { + system = namedSystemB, + after = { systemA }, + } + + --By naming the system functions this ensures that the initial table.sort call in the + --system scheduler returns the following order systemC -> systemB -> systemA + --and we want to schedule it in this order to ensure that the scheduler is able to complete + --without erroring + local function namedSystemA() + table.insert(order, "c") + end + + local systemC = { + system = namedSystemA, + after = { systemA, systemB }, + } + + loop:scheduleSystems({ + systemC, + systemB, + systemA, + }) + + local connection = loop:begin({ default = bindable.Event }) + + expect(#order).to.equal(0) + + bindable:Fire() + + expect(#order).to.equal(3) + expect(order[1]).to.equal("a") + expect(order[2]).to.equal("b") + expect(order[3]).to.equal("c") + + connection.default:Disconnect() + end) + + it("should schedule systems based on order passed into scheduleSystems", function() + local loop = Loop.new() + + local order = {} + local systemA = { + system = function() + table.insert(order, "a") + end, + } + local systemB = { + system = function() + table.insert(order, "b") + end, + } + local systemC = { + system = function() + table.insert(order, "c") + end, + } + + loop:scheduleSystems({ + systemB, + systemC, + systemA, + }) + + local connection = loop:begin({ default = bindable.Event }) + + expect(#order).to.equal(0) + + bindable:Fire() + + expect(#order).to.equal(3) + expect(order[1]).to.equal("b") + expect(order[2]).to.equal("c") + expect(order[3]).to.equal("a") + + connection.default:Disconnect() + end) + it("should throw error for systems with unscheduled depedencies", function() local loop = Loop.new()