Sleitnick/RbxUtil

`Loader` race conditions

Closed this issue · 2 comments

lopi-py commented

I'm working on a game structure like the following:

folder/
  foo/
    init.luau
    replicator.luau
  bar/
    init.luau
    replicator.luau

Loader.SpawnAll(Loader.LoadDescendants(script), "OnStart")

sometimes foo's replicator is called but other times bar's is called. I think it has to do with

RbxUtil/modules/loader/init.lua

Line 82 in 19fc2de

modules[descendant.Name] = m

RbxUtil/modules/loader/init.lua

Line 82 in 19fc2de

modules[descendant.Name] = m

use module:GetFullName() instead of module.Name maybe?

colbert2677 commented

If you have a case like this I think it's better to rely on the provided MatchesName predicate, another predicate of your choosing or create your own loading logic. Ultimately one of the pitfalls of not using a predicate is that the descendants of the given object will all need to be uniquely named. GetFullName doesn't sound fun because it breaks this sample use case:

Knit.Modules = Loader.LoadDescendants(...)

Loader itself is essentially glorified syntax sugar. Essentially the only behaviour that blocks this use case is that the loader functions return a table and if you get rid of that it's just one line of code that's effectively iterating all descendants of a given target and requiring the modules in them.

Sleitnick commented

Loader itself is essentially glorified syntax sugar. Essentially the only behaviour that blocks this use case is that the loader functions return a table and if you get rid of that it's just one line of code that's effectively iterating all descendants of a given target and requiring the modules in them.

This is the big issue here. Knit uses this behavior internally and uses the names in the external API. It is also used in order to set the memory category based on the module name.

So, I think the solution here is:

  1. Change the loader to assert that the name doesn't exist yet (i.e. throw an error early rather than load overtop).
  2. Add better documentation for this behavior, and include a custom example for people who need to just load modules.

FWIW, in your case, I'd just custom-role a "loader" such as this:

local function LoadModules(parent)
   local all = {}
   for _, v in parent:GetDescendants() do
      if v:IsA("ModuleScript") then
         table.insert(all, require(v))
      end
   end

   for _, v in all do
      if typeof(v) == "table" and typeof(v.OnStart) == "function" then
         task.spawn(function() v:OnStart() end)
      end
   end
end
  • 👍2