Signal bugfix

Sleitnick · Oct 23, 2023 · 288b9ef · 288b9ef
1 parent c9b268a
commit 288b9ef
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 48 deletions.
diff --git a/README.md b/README.md
@@ -5,26 +5,26 @@
 
 | Module | Dependency | Description |
 | -- | -- | -- |
-| [Tree](https://sleitnick.github.io/RbxUtil/api/Tree) | `Tree = "sleitnick/[email protected]"` | Utility functions for accessing instances in the game hierarchy |
-| [Silo](https://sleitnick.github.io/RbxUtil/api/Silo) | `Silo = "sleitnick/[email protected]"` | State container class |
-| [TableUtil](https://sleitnick.github.io/RbxUtil/api/TableUtil) | `TableUtil = "sleitnick/[email protected]"` | Table utility functions |
-| [Net](https://sleitnick.github.io/RbxUtil/api/Net) | `Net = "sleitnick/[email protected]"` | Static networking module |
-| [Streamable](https://sleitnick.github.io/RbxUtil/api/Streamable) | `Streamable = "sleitnick/[email protected]"` | Streamable class and StreamableUtil |
-| [TaskQueue](https://sleitnick.github.io/RbxUtil/api/TaskQueue) | `TaskQueue = "sleitnick/[email protected]"` | Batches tasks that occur on the same execution step |
-| [Shake](https://sleitnick.github.io/RbxUtil/api/Shake) | `Shake = "sleitnick/[email protected]"` | Shake class for making things shake |
-| [Input](https://sleitnick.github.io/RbxUtil/api/Input) | `Input = "sleitnick/[email protected]"` | Basic input classes |
-| [Timer](https://sleitnick.github.io/RbxUtil/api/Timer) | `Timer = "sleitnick/[email protected]"` | Timer class |
-| [Trove](https://sleitnick.github.io/RbxUtil/api/Trove) | `Trove = "sleitnick/[email protected]"` | Trove class for tracking and cleaning up objects |
 | [Comm](https://sleitnick.github.io/RbxUtil/api/Comm) | `Comm = "sleitnick/[email protected]"` | Comm library for remote communication |
 | [Component](https://sleitnick.github.io/RbxUtil/api/Component) | `Component = "sleitnick/[email protected]"` | Component class |
-| [Symbol](https://sleitnick.github.io/RbxUtil/api/Symbol) | `Symbol = "sleitnick/[email protected]"` | Symbol |
-| [Option](https://sleitnick.github.io/RbxUtil/api/Option) | `Option = "sleitnick/[email protected]"` | Represent optional values in Lua |
+| [Concur](https://sleitnick.github.io/RbxUtil/api/Concur) | `Concur = "sleitnick/[email protected]"` | Concurrent task handler |
 | [EnumList](https://sleitnick.github.io/RbxUtil/api/EnumList) | `EnumList = "sleitnick/[email protected]"` | Enum List class |
-| [PID](https://sleitnick.github.io/RbxUtil/api/PID) | `PID = "sleitnick/[email protected].0"` | PID Controller class |
+| [Input](https://sleitnick.github.io/RbxUtil/api/Input) | `Input = "sleitnick/[email protected].0"` | Basic input classes |
 | [Loader](https://sleitnick.github.io/RbxUtil/api/Loader) | `Loader = "sleitnick/[email protected]"` | Requires all modules within a given instance |
-| [Concur](https://sleitnick.github.io/RbxUtil/api/Concur) | `Concur = "sleitnick/[email protected]"` | Concurrent task handler |
 | [Log](https://sleitnick.github.io/RbxUtil/api/Log) | `Log = "sleitnick/[email protected]"` | Log class for logging to PlayFab |
+| [Net](https://sleitnick.github.io/RbxUtil/api/Net) | `Net = "sleitnick/[email protected]"` | Static networking module |
+| [Option](https://sleitnick.github.io/RbxUtil/api/Option) | `Option = "sleitnick/[email protected]"` | Represent optional values in Lua |
+| [PID](https://sleitnick.github.io/RbxUtil/api/PID) | `PID = "sleitnick/[email protected]"` | PID Controller class |
 | [Quaternion](https://sleitnick.github.io/RbxUtil/api/Quaternion) | `Quaternion = "sleitnick/[email protected]"` | Quaternion class |
 | [Ser](https://sleitnick.github.io/RbxUtil/api/Ser) | `Ser = "sleitnick/[email protected]"` | Ser class for serialization and deserialization |
+| [Shake](https://sleitnick.github.io/RbxUtil/api/Shake) | `Shake = "sleitnick/[email protected]"` | Shake class for making things shake |
+| [Signal](https://sleitnick.github.io/RbxUtil/api/Signal) | `Signal = "sleitnick/[email protected]"` | Signal class |
+| [Silo](https://sleitnick.github.io/RbxUtil/api/Silo) | `Silo = "sleitnick/[email protected]"` | State container class |
+| [Streamable](https://sleitnick.github.io/RbxUtil/api/Streamable) | `Streamable = "sleitnick/[email protected]"` | Streamable class and StreamableUtil |
+| [Symbol](https://sleitnick.github.io/RbxUtil/api/Symbol) | `Symbol = "sleitnick/[email protected]"` | Symbol |
+| [TableUtil](https://sleitnick.github.io/RbxUtil/api/TableUtil) | `TableUtil = "sleitnick/[email protected]"` | Table utility functions |
+| [TaskQueue](https://sleitnick.github.io/RbxUtil/api/TaskQueue) | `TaskQueue = "sleitnick/[email protected]"` | Batches tasks that occur on the same execution step |
+| [Timer](https://sleitnick.github.io/RbxUtil/api/Timer) | `Timer = "sleitnick/[email protected]"` | Timer class |
+| [Tree](https://sleitnick.github.io/RbxUtil/api/Tree) | `Tree = "sleitnick/[email protected]"` | Utility functions for accessing instances in the game hierarchy |
+| [Trove](https://sleitnick.github.io/RbxUtil/api/Trove) | `Trove = "sleitnick/[email protected]"` | Trove class for tracking and cleaning up objects |
 | [WaitFor](https://sleitnick.github.io/RbxUtil/api/WaitFor) | `WaitFor = "sleitnick/[email protected]"` | WaitFor class for awaiting instances |
-| [Signal](https://sleitnick.github.io/RbxUtil/api/Signal) | `Signal = "sleitnick/[email protected]"` | Signal class |
diff --git a/modules/signal/init.lua b/modules/signal/init.lua
@@ -7,15 +7,6 @@
 -- the signal handlers comes at minimal extra cost over a naive signal        --
 -- implementation that either always or never spawns a thread.                --
 --                                                                            --
--- API:                                                                       --
---   local Signal = require(THIS MODULE)                                      --
---   local sig = Signal.new()                                                 --
---   local connection = sig:Connect(function(arg1, arg2, ...) ... end)        --
---   sig:Fire(arg1, arg2, ...)                                                --
---   connection:Disconnect()                                                  --
---   sig:DisconnectAll()                                                      --
---   local arg1, arg2, ... = sig:Wait()                                       --
---                                                                            --
 -- License:                                                                   --
 --   Licensed under the MIT license.                                          --
 --                                                                            --
@@ -87,15 +78,6 @@ end
 local Connection = {}
 Connection.__index = Connection
 
-function Connection.new(signal, fn)
-	return setmetatable({
-		Connected = true,
-		_signal = signal,
-		_fn = fn,
-		_next = false,
-	}, Connection)
-end
-
 function Connection:Disconnect()
 	if not self.Connected then
 		return
@@ -166,7 +148,9 @@ function Signal.new<T...>(): Signal<T...>
 	local self = setmetatable({
 		_handlerListHead = false,
 		_proxyHandler = nil,
+		_yieldedThreads = nil,
 	}, Signal)
+
 	return self
 end
 
@@ -188,10 +172,12 @@ function Signal.Wrap<T...>(rbxScriptSignal: RBXScriptSignal): Signal<T...>
 		typeof(rbxScriptSignal) == "RBXScriptSignal",
 		"Argument #1 to Signal.Wrap must be a RBXScriptSignal; got " .. typeof(rbxScriptSignal)
 	)
+
 	local signal = Signal.new()
 	signal._proxyHandler = rbxScriptSignal:Connect(function(...)
 		signal:Fire(...)
 	end)
+
 	return signal
 end
 
@@ -219,13 +205,20 @@ end
 	```
 ]=]
 function Signal:Connect(fn)
-	local connection = Connection.new(self, fn)
+	local connection = setmetatable({
+		Connected = true,
+		_signal = self,
+		_fn = fn,
+		_next = false,
+	}, Connection)
+
 	if self._handlerListHead then
 		connection._next = self._handlerListHead
 		self._handlerListHead = connection
 	else
 		self._handlerListHead = connection
 	end
+
 	return connection
 end
 
@@ -256,24 +249,29 @@ end
 function Signal:Once(fn)
 	local connection
 	local done = false
+
 	connection = self:Connect(function(...)
 		if done then
 			return
 		end
+
 		done = true
 		connection:Disconnect()
 		fn(...)
 	end)
+
 	return connection
 end
 
 function Signal:GetConnections()
 	local items = {}
+
 	local item = self._handlerListHead
 	while item do
 		table.insert(items, item)
 		item = item._next
 	end
+
 	return items
 end
 
@@ -292,6 +290,15 @@ function Signal:DisconnectAll()
 		item = item._next
 	end
 	self._handlerListHead = false
+
+	if self._yieldedThreads then
+		for thread in self._yieldedThreads do
+			if coroutine.status(thread) == "suspended" then
+				task.cancel(thread)
+			end
+		end
+		table.clear(self._yieldedThreads)
+	end
 end
 
 -- Signal:Fire(...) implemented by running the handler functions on the
@@ -333,7 +340,9 @@ end
 function Signal:FireDeferred(...)
 	local item = self._handlerListHead
 	while item do
-		task.defer(item._fn, ...)
+		if item.Connected then
+			task.defer(item._fn, ...)
+		end
 		item = item._next
 	end
 end
@@ -354,17 +363,20 @@ end
 	```
 ]=]
 function Signal:Wait()
-	local waitingCoroutine = coroutine.running()
-	local connection
-	local done = false
-	connection = self:Connect(function(...)
-		if done then
-			return
-		end
-		done = true
-		connection:Disconnect()
-		task.spawn(waitingCoroutine, ...)
+	local yieldedThreads = rawget(self, "_yieldedThreads")
+	if not yieldedThreads then
+		yieldedThreads = {}
+		rawset(self, "_yieldedThreads", yieldedThreads)
+	end
+
+	local thread = coroutine.running()
+	yieldedThreads[thread] = true
+
+	self:Once(function(...)
+		yieldedThreads[thread] = nil
+		task.spawn(thread, ...)
 	end)
+
 	return coroutine.yield()
 end
 
@@ -382,6 +394,7 @@ end
 ]=]
 function Signal:Destroy()
 	self:DisconnectAll()
+
 	local proxyHandler = rawget(self, "_proxyHandler")
 	if proxyHandler then
 		proxyHandler:Disconnect()
@@ -398,8 +411,8 @@ setmetatable(Signal, {
 	end,
 })
 
-return {
+return table.freeze({
 	new = Signal.new,
 	Wrap = Signal.Wrap,
 	Is = Signal.Is,
-}
+})
diff --git a/modules/signal/wally.toml b/modules/signal/wally.toml
@@ -1,7 +1,7 @@
 [package]
 name = "sleitnick/signal"
 description = "Signal class"
-version = "1.5.0"
+version = "2.0.0"
 license = "MIT"
 authors = ["Stephen Leitnick"]
 registry = "https://github.com/UpliftGames/wally-index"