Skip to content

Commit

Permalink
Fix scheduler not respecting priority (#51)
Browse files Browse the repository at this point in the history
Co-authored-by: Marcus <[email protected]>
  • Loading branch information
metrowaii and Ukendio authored Apr 15, 2024
1 parent f7d40c4 commit ff62f44
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 43 deletions.
82 changes: 39 additions & 43 deletions lib/Loop.lua
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,6 @@ local function systemName(system: System)
return debug.info(fn, "s") .. "->" .. debug.info(fn, "n")
end

local function systemPriority(system: System)
if type(system) == "table" then
return system.priority or 0
end

return 0
end

--[=[
@class Loop
Expand Down Expand Up @@ -222,9 +214,46 @@ function Loop:replaceSystem(old: System, new: System)
end

local function orderSystemsByDependencies(unscheduledSystems: { System })
local sortedUnscheduledSystems = table.clone(unscheduledSystems)
local systemPriorityMap = {}
local visiting = "v"

local function systemPriority(system: System)
local priority = systemPriorityMap[system]

if not priority then
priority = 0

systemPriorityMap[system] = visiting

if type(system) == "table" then
if system.after then
for _, dependency in system.after do
if systemPriorityMap[dependency] ~= visiting then
priority = math.max(priority, systemPriority(dependency) + 1)
else
local errorStatement = {
`Cyclic dependency detected: System '{systemName(system)}' is set to execute after System '{systemName(
dependency
)}', and vice versa. This creates a loop that prevents the systems from being able to execute in a valid order.`,
"To resolve this issue, reconsider the dependencies between these systems. One possible solution is to update the 'after' field from one of the systems.",
}
error(table.concat(errorStatement, "\n"))
end
end
elseif system.priority then
priority = system.priority
end
end

systemPriorityMap[system] = priority
end

table.sort(sortedUnscheduledSystems, function(a, b)
return priority
end

local scheduledSystems = table.clone(unscheduledSystems)

table.sort(scheduledSystems, function(a, b)
local priorityA = systemPriority(a)
local priorityB = systemPriority(b)

Expand All @@ -242,39 +271,6 @@ local function orderSystemsByDependencies(unscheduledSystems: { System })
return priorityA < priorityB
end)

local scheduledSystemsSet = {}
local scheduledSystems = {}

local visited, explored = 1, 2

local function scheduleSystem(system)
scheduledSystemsSet[system] = visited

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

scheduledSystemsSet[system] = explored

table.insert(scheduledSystems, system)
end

for _, system in sortedUnscheduledSystems do
if scheduledSystemsSet[system] == nil then
scheduleSystem(system)
end
end

return scheduledSystems
end

Expand Down
42 changes: 42 additions & 0 deletions lib/Loop.spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,48 @@ return function()
connection.default:Disconnect()
end)

it("should call systems in order with dependent system after priority system", function()
local loop = Loop.new()

local order = {}
local systemB = {
system = function()
table.insert(order, "b")
end,
}
local systemC = {
system = function()
table.insert(order, "c")
end,
priority = 1000,
}
local systemA = {
system = function()
table.insert(order, "a")
end,
after = { systemC },
}

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 not schedule systems more than once", function()
local loop = Loop.new()

Expand Down

0 comments on commit ff62f44

Please sign in to comment.