Issues with my event system

Help and Feedback Scripting Support
#1

Basically, I made a module that emulates the behavior of BindableEvents using coroutines. I tried to copy the original functionality of BindableEvents as best as I could, and I also added two extra functions: GetId and IsAlive. The problem I’m having is with event:Wait() and event:DisconnectAll().

The way the :Wait() works is by yielding the thread it was called from (the running one) and saving it in a table (self.Waiting) in order to be resumed later. I somewhat managed to make it work, but it only works inside a running thread. This is why I had to include this line:

assert(running(), 'Wait can only be used inside a thread')

If I remove this (and use it outside of a running thread), I get this error: “attempt to yield across a metamethod/c-call boundary”. This snippet better shows what’s happening:

local function test()
    return coroutine.yield()
end
test() -- errors with the same error i mentioned

This is how I’m trying to stop the current thread, but it doesn’t work if it isn’t inside a coroutine. I need to figure out a way to make :Wait() work without having it have to be in a coroutine.

With :DisconnectAll(), I don’t know how to properly implement something so I can kill the threads. What I have in my code already for :DisconnectAll() is just a placeholder and it doesn’t actually do what it is supposed to do.

Here’s the full module:

--// custom event system because bindables have a weird icon

local module = {}
module.__index = module

local events = {}

--// stuff
local coroutine = coroutine
local running   = coroutine.running
local resume    = coroutine.resume
local create    = coroutine.create
local yield     = coroutine.yield

--// functions
-- (i have this because table.find isn't a thing in vanilla lua)
local function find(tab, value)
    for i, v in pairs(tab) do
        if v == value then
            return i
        end
    end
end

-- creates a weak table to ensure it gets garbage collected
-- i added this to avoid memory leaks
local function weak(tab)
    return setmetatable(tab, {__mode = 'k'})
end

--// magic begins here
function module.new(id)
    if id and events[id] then return events[id] end
    local self = setmetatable({
        Id = #events + 1,
        Connections = weak({}),
        Waiting = weak({});
    }, module)

    events[self.Id] = self
    return self
end

function module:GetId()
    return self.Id
end

function module:Connect(func, num)
    assert(type(func) == 'function')
    local thread

    if num then
        local threads = weak({})
        for x = 1, num do
            table.insert(threads, create(func))
        end
        table.insert(self.Connections, threads)
    else
        thread = create(func)
        table.insert(self.Connections, thread)
    end

    local self = {
        Disconnect = function()
            local found = find(self.Connections, thread)

            if found then
                self.Connections[found] = nil
                yield()
            end
        end,

        IsAlive = function()
            return running() == thread
        end;
    }

    self.Kill = self.Disconnect
    self.IsRunning = self.IsAlive

    return self
end

function module:DisconnectAll()
    self.Connections = weak({})
end

function module:Wait()
    assert(running(), 'Wait can only be used inside a thread')
    table.insert(self.Waiting, running())
    return yield()
end

function module:Fire(...)
    for i, thread in ipairs(self.Waiting) do
        resume(coro, ...)
        self.Waiting[i] = nil
    end

    for i, thread in ipairs(self.Connections) do
        if type(thread) == 'thread' then
            resume(thread, ...)
        elseif type(thread) == 'table' then
            for _, thread in ipairs(thread) do
                resume(thread, ...)
            end

            thread = nil
        end
    end
end

-- i don't know why i added this, but why not
function module:Destroy()
    self:DisconnectAll()
    events[self.Id] = nil
    self = nil
end

--// Aliases
module.KillAll = module.DisconnectAll
module.Bind = module.Connect
module.Init = module.Fire

setmetatable(module, {
    __call = function(self, ...)
        return self.new(...)
    end,

    __newindex = function()
        error("Make changes by editing the module itself.")
    end,
})

return module
--// thanks for stopping by

Any help/feedback is appreciated.

Is it worth it implementing my own BindableEvent module?
#2

It doesn’t work because coro doesn’t exist. I think you were ment to use thread.

If you use :Wait() twice, the second time it gets used, it won’t return the right arguments passed to :Fire(), but only if :Fire() is fired before :Wait() is fired.

Your :Wait() function, stores waiting coroutines in the self.Waiting table.

Your :Fire() function is clearing out the self.Waiting table.

And one of the calls could be overwriting one of the things.

 
changing the :Fire() function to this, should fix the issue

function module:Fire(...)
	local waitingCoroutines = {}
	for i, thread in ipairs(self.Waiting) do
		table.insert(waitingCoroutines, thread)
		self.Waiting[i] = nil
	end

	for i, thread in ipairs(self.Connections) do
		if type(thread) == 'thread' then
			coroutine.resume(thread, ...)
		elseif type(thread) == 'table' then
			for _, thread in ipairs(thread) do
				coroutine.resume(thread, ...)
			end
		end
	end

	for i, thread in ipairs(waitingCoroutines) do
		coroutine.resume(thread, ...)
		waitingCoroutines[i] = nil
	end
end

Personally I think it’s chaos. Unless you actually wanted to somehow keep track of all the threads. But otherwise, you don’t even need to do all of this.

Connect is a function that connects. And Fire runs Connections. You can create a Connection for Wait as well, and capture the arguments from any Fire.