Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
chess123mate opened this issue Feb 11, 2022 · 1 comment

Comments

@chess123mate
Copy link

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.

@lucasmz-dev
Copy link

lucasmz-dev commented Nov 16, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants