goodsignal icon indicating copy to clipboard operation
goodsignal copied to clipboard

Difference in behaviour with BindableEvent:Destroy() when called during a connection handler

Open chess123mate opened this issue 3 years ago • 1 comments

local x = Instance.new("BindableEvent")
for i = 1, 10 do
	local con; con = x.Event:Connect(function()
		x:Destroy()
		print("B", i)
	end)
end
x:Fire()


local x = require(script.GoodSignal).new()
for i = 1, 10 do
	local con; con = x:Connect(function()
		x:DisconnectAll()
		print("G", i)
	end)
end
x:Fire()

Only B 10 is printed for BindableEvent, but GoodSignal prints out G 10 through G 1. I point this out due to the claim

A Roblox Lua Signal implementation that has full API and behavioral parity with Roblox' RBXScriptSignal type.

This can be corrected by adding if not self._handlerListHead then break end after task.spawn(freeRunnerThread, item._fn, ...) in the Fire function.

chess123mate avatar Feb 11 '22 13:02 chess123mate

The better way to fix this is to have a proper public Connected field and set it to false on every connection when disconnecting. The fact that Connected isn't a thing in GoodSignal also shows that no, GoodSignal does not have full API & behavioral parity. (And no Deferred event support in a form of a separate module or one that automatically chooses depending on what mode the game is currently on, which isn't hard to come up with; mixing up signal behaviours (from native roblox events and dev-made ones) is stressful and causes hidden problems)

GoodSignal attempts at saving time by just setting the start node to nil, which overshadows the issue that he did notice with not checking Connected when firing when you disconnect connections in a certain way.

lucasmz-dev avatar Nov 16 '22 01:11 lucasmz-dev