Lua Signal Class Comparison & Optimal `GoodSignal` Class

[ShardItem (Nathan)]

I’m using this library from traditionally using the BindableEvent system, works like a charm, but there’s a weird thing I’d love to know what’s going on…

In this example, the code awaits for a Promise to complete, it does this by waiting for the PlayerDataLoaded GoodSignal event to fire with some business logic present. This code snippet works completely fine.
(the connection variable is stated and initialized in two lines)

return Promise.new(function(resolve)
		local connection
		connection = PlayerDataService.Events.PlayerDataLoaded:Connect(function(loadedPlayer)
			if player == loadedPlayer then
				connection:Disconnect()
				return resolve(true)
			end
		end)
	end):await()

However, when changing the code to this:
(the connection variable is stated and initialized on one line)

return Promise.new(function(resolve)
		local connection = PlayerDataService.Events.PlayerDataLoaded:Connect(function(loadedPlayer)
			if player == loadedPlayer then
				connection:Disconnect()
				return resolve(true)
			end
		end)
	end):await()

I get an attempt to index nil with ‘Disconnect’ error when calling connection:Disconnect(). What’s the issue with assigning the GoodScript Connection object on just one line?

[MagmaBurnsV]

The same reason why you can’t do this:

local Table = {
	Value = Table
}

The variable is still being initialized, so it’s not possible to use it inside itself unless it has already been defined above.

[dukzae]

I’ve not changed anything in my code when suddenly this begun arbitrarily throwing errors: C stack errors and “Error occurred, no output from Lua” errors. Any help would be greatly appreciated as it very randomly can break entire servers: not being able to load data, previous default Roblox chat not loading, etc. It has happened once every day for the past three days, whereas before it would not happen at all.

The following image shows my Health script also erroring after using GoodSignal, despite me not changing it for months.

For now I’ve wrapped Line 139 in a pcall (admittedly I have no clue whether it’ll fix it or not), but any explanations would be great.

[dukzae]

Hey @stravant, this doesn’t happen anymore but I still do get C stack overflow errors for unknown reasons. Guidance would be helpful, I’ve tried everything, I’ve used debug.traceback to try to trace where the stack begins but as I don’t know the complex mechanics of the module, I don’t know what exactly is causing this; nested connections? Nested :Fire()? Thanks.

[stravant]

Yes, you have event re-entrancy somewhere.

If you have no idea where to look and don’t have event re-entrancy somewhere else you could add
self._firing = true, self._firing = false around the body of the :Fire call and error if it’s already being fired to find whose fault the re-entrancy is.

[dukzae]

Hey, thanks for your response. It appears that the errors are completely anonymous. Have no clue what to do at this point; the error doesn’t even appear after key game events or triggers so I’m completely lost. Why is Roblox not supplying context with the error message?

Note: I can only reproduce this in-game. Haven’t seen it once in studio; perhaps more common when large amount of players in-game?

function Signal.new()
	return setmetatable({
		_handlerListHead = false,
		_firing = false,
	}, Signal)
end

function Signal:Fire(...)
	local item = self._handlerListHead
	
	if self._firing then
		error("re-entrancy")
	end
	
	self._firing = true
	
	while item do
		if item._connected then
			--local trace = debug.traceback()

			--local split = trace:split("\n")
			--local new = {}

			--for _, v in split do
			--	local newSplit = v:split(".")

			--	table.insert(new, newSplit[#newSplit])
			--end

			--print(table.concat(new, ", "))

			if not freeRunnerThread then
				freeRunnerThread = coroutine.create(runEventHandlerInFreeThread)
				-- Get the freeRunnerThread to the first yield
				coroutine.resume(freeRunnerThread)
			end
			task.spawn(freeRunnerThread, item._fn, ...)
		end
		item = item._next
	end
	
	self._firing = false
end

[stravant]

Do you have deferred events enabled?

[RuizuKun_Dev (Kitsune)]

Hey, I use Good Signal.

I’m curious about the relevance of enabling deferred events when Good Signal is written in pure Lua. Typically, the “deferred events” setting only affects Instances.

[dukzae]

Not at this moment, I do not.

@RuizuKun_Dev

I am wondering why having deferred events enabled is relevant when Good Signal is pure Lua and I expect that property to only affect Events from Instances

I am going off the same assumption as the above

Edit: I’ve found the source of the bug, thank you for your help.

[JoyfulFlowerMary (Mary)]

Lately, I’ve thought about how modules that people release on these forums have their own events that feel just like the built-in ones, and have discovered why they use “Signal” and how useful it could be for my own scripts. (Custom events could be so helpful!)

One question, though. I’ve noticed that multiple modules that I’ve added to my game come with their own copies of signal within them. Does having multiple redundant copies of this signal module accessed by different scripts affect performance or is suboptimal in some way?

I’m considering placing the signal module in a universal location then pointing all of the modules to use it instead of their embedded versions, but I don’t know if it’s worthwhile or necessary.

[MagmaBurnsV]

Having multiple modules for the same class would only hurt a miniscule amount of performance. I would say plugging in a universal implementation would be useless.

---

My main problem with any signal implementation, though, is the lack of being able to type-annotate them. I semi-succeeded in creating one that got the datatypes, but there is no way to type the parameter names, making it a bit useless.

For self-serving modules, I usually just use BindableEvents and override their RBXScriptSignal with a pseudo-type. For example:

local Bindable = Instance.new("BindableEvent")

type Callback = (Name: string, Value: number) -> ()
type Signal = {
	Connect: (self: Signal, Callback: Callback) -> RBXScriptConnection,
	Wait: (self: Signal) -> (string, number),
	Once: (self: Signal, Callback: Callback) -> RBXScriptConnection,
	ConnectParallel: (self: Signal, Callback: Callback) -> RBXScriptConnection
}

local Event = (Bindable.Event :: any) :: Signal
Event:Connect(print) -- Callback: (Name: string, Value: number) -> ()

Obviously, this is far from being compact and trivial, but the alternative is no autocomplete.

[stravant]

@JoyfulFlowerMary

Does having multiple redundant copies of this signal module accessed by different scripts affect performance or is suboptimal in some way?

Not worth the trouble.

Roblox has a parser cache, so if two modules have exactly the same source code in them, the game engine will only parse the module once, and both modules will share most of the same function definitions etc. The only thing that won’t be shared is the closure / table memory which is not very much.

I measured it and extra copy of GoodSignal uses just under 3kb of memory (and no additional network bandwidth because script sources are deduplicated for network replication). 1 full size texture uses 4000kb of memory, so pick your battles. There’s probably better things for you to be spending your time on than deduplicating GoodSignal.

[2jammers (james)]

Do you ever plan on supporting ConnectParallel? Or is that not possible.

[stravant]

I think it’s too early, with details of parallel Luau still being likely to change a bit. The next major update to parallel Luau is probably the first time I’ll consider it.

[Eternity_Devs (EternityDev)]

ConnectParallel can be implemented by adding task.synchronize and task.desynchronize, after that the user need to use Actor Instance for the usage.

[exprav]

Does GoodSignal behave the same as deferred lua event handling as described here?

[stravant]

No, however, if you just replace the line:
task.spawn(freeRunnerThread, item._fn, ...)

With:
task.defer(freeRunnerThread, item._fn, ...)

It will have that behavior. I don’t think it makes sense for me to offer this variant until after the transition to deferred has happened.

[0Shank]

Hello, I’m still a little confused on the GC behavior of goodsignal, it might be general lua GC behavior, but for some reason cyclic references are causing the GC to not collect things.

Here is an example to show this:

local Weak = require(script.Parent.ModuleScript)
local Table = {
Event = require(game.ServerScriptService.Signal).new()
}

Table.Event:Connect(function()
print(Table)
end)

Weak[2] = Table

Here’s my test for checking if the table has been GCed

local Weak = setmetatable({}, {__mode = ‘v’})

task.spawn(function()
while true do
table.create(10^6)
print(Weak)
task.wait()
end
end)

return Weak

And in this first example, as you see the table never get’s gced.

I know this is specfically an issue with cyclic references, because if I reference an instance it still gets collected

local Weak = require(script.Parent.ModuleScript)
local Table = {
Event = require(game.ServerScriptService.Signal).new()
}

Table.Event:Connect(function()
print(workspace.Baseplate)
end)

Weak[2] = Table

This does get collected.

And the bizzare thing is, this only happens in the script corourtine, if I spawn a new coroutine it does getcollected

local Weak = require(script.Parent.ModuleScript)

task.spawn(function()
local Table = {
Event = require(game.ServerScriptService.Signal).new()
}

Table.Event:Connect(function()
print(Table)
end)

Weak[2] = Table
end)

Which doesn’t make sense to me, because in both cases “Table” it isn’t accessible anymore, once the script is done running or the coroutine finishes. But even more this specfically seems to be an issue with cyclic references

[WreckDough]

This is a PSA, if you coroutine.yield() inside of a connection callback and then later resume it, DONT RESUME IT MORE THAN ONCE. Seems like common sense but you won’t get any direct errors making it unclear where the problem is and the signal event handler will get messed up. Spent a good hour figuring out why GoodSignal was erroring.

you could probably change the GoodSignal code to check if a RunnerThread was resumed from within GoodSignal and ignore or error otherwise

[CremaCoffeeOverseer (jenny)]

Is this how I would implement a RemoteEvent with a signal?

	local Event = Signal.new(RemoteEvent.OnClientEvent)
	Event:Connect(function()
		print("remote fired to client")
	end)