Use a per-place, Opt-In boolean property for SignalBehavior rather than an Enum with a "Default" option

This announcement was very nonchalantly made, and has been met with a large amount of concern from many developers including myself:

Not only does this lead to a very large amount of undefined/unpredictable behavior in existing code, there’s even a number of bugs with existing core script code (see: Camera panning is one-frame-off from input when Workspace.SignalBehavior is set to Deferred)

This update also, in effect, makes it so I have to replace 100% of instances in my code when I use BindableEvent objects, since the behavior of events in general has fundamentally changed. This requires me to update a large number of public libraries and old codebases in order to conform with this change. For a lot of these instances, I have already switched from using plain old BindableEvent instances, to using a custom Signal class:

Workarounds I've made so far to future-proof my code:

Signal (ModuleScript):

--!strict

local FastSpawnListener = Instance.new('BindableFunction')

FastSpawnListener.OnInvoke = function(cb: (...any) -> (), ...: any)
	cb(...)
end :: any

export type Connection = {
	Connected: boolean,
	Disconnect: () -> (),
	Destroy: () -> (),
}
export type FunctionArgs<T> = any --[[typeof((function(...)
	local x: T
	x(...)
	return ...
end)())]]
export type Signal<T> = {
	Connect: (Signal<T>, T) -> Connection,
	Wait: (Signal<T>) -> ...FunctionArgs<T>,
	Fire: (Signal<T>, ...FunctionArgs<T>) -> (),
	Destroy: () -> (),
}

local Connection = {}
Connection.__index = {}

function Connection.__index.Disconnect(self: any)
	if self.Connected then
		self.Connected = false
		self._signal._cbSet[self._cb] = nil
		self._signal = nil
		self._cb = nil
	end
end
Connection.__index.Destroy = Connection.__index.Disconnect

function Connection.new(_signal: any, _cb: any): Connection
	local self = {}
	setmetatable(self, Connection)
	self.Connected = true
	self._signal = _signal
	self._cb = _cb
	
	return self :: any
end

local Signal = {}
Signal.__index = {}

function Signal.__index.Connect(self: any, cb: any): Connection
	local conn = Connection.new(self, cb)
	self._cbSet[cb] = true
	return conn
end

function Signal.__index.Destroy(self: any)
	self._cbSet = {}
	self._threads = {}
end

function Signal.__index.Wait(self: any): ...any
	local thread = coroutine.running()
	table.insert(self._threads, thread)
	return coroutine.yield()
end

function Signal.__index.Fire(self: any, ...: any)
	local cbs = {}
	for cb in pairs(self._cbSet) do
		table.insert(cbs, cb)
	end
	for i = 1, #cbs do
		local cb = cbs[i]
		coroutine.resume(
			coroutine.create(
				function(...)
					FastSpawnListener:Invoke(cb, ...)
				end
			),
			...
		)
	end
	
	local threads = self._threads
	if #threads > 0 then
		self._threads = {}
		for i = 1, #threads do
			coroutine.resume(threads[i], ...)
		end
	end
end

function Signal.new(): Signal<any>
	local self = {}
	setmetatable(self, Signal)
	
	self._cbSet = {}
	self._threads = {}
	
	return self :: any
end

return Signal

FastSpawn (ModuleScript):

--!strict

local FAST_SPAWN_BINDABLE = Instance.new('BindableFunction')
local FAST_SPAWN_CALLER = function(cb: () -> ())
	cb()
end :: any
local LOCK_IS_INVOKING = false
FAST_SPAWN_BINDABLE.OnInvoke = FAST_SPAWN_CALLER

local function FastSpawn(func: () -> ())
	if LOCK_IS_INVOKING then
		local nestedCallFastSpawnBindable = Instance.new('BindableFunction')
		nestedCallFastSpawnBindable.OnInvoke = FAST_SPAWN_CALLER
		coroutine.resume(
			coroutine.create(function()
				nestedCallFastSpawnBindable:Invoke(func)
			end)
		)
	else
		LOCK_IS_INVOKING = true
		coroutine.resume(
			coroutine.create(function()
				FAST_SPAWN_BINDABLE:Invoke(func)
			end)
		)
		LOCK_IS_INVOKING = false
	end
end

return FastSpawn

Because Workspace.SignalBehavior is not accessible by scripts, the time is still ripe to make an API change that would save a lot of headache from the vast majority of roblox developers with a large, existing codebase that relies on the immediate behavior of signals firing.

My proposal is that there should be a boolean property on Workspace, akin to StreamingEnabled (which also makes very impactful, breaking changes to an existing codebase), called SignalDeferenceEnabled. In the future, when parallel Luau is released, having this property set to true could be a requirement to enabling parallel luau.

What this means is that existing games don’t have to be updated at all, since when migrating an old file, SignalDeferenceEnabled can be set to false. However, for newly-created places, this can be set to true by default, since these places can start fresh with new, deferred-signal-compatible code.

37 Likes

Thought I’d make a poll here, since I think it might provide some insight into what percentage of users would have their games break because of Deferred SignalBehavior; @loleris mentioned that his games are working fine, whereas my current project completely breaks, and I still don’t know exactly what the issue is yet.

When you set SignalBehavior to Deferred in your game, what happens?
  • The game breaks completely (i.e. is unplayable)
  • The game is playable, but major systems fail
  • There are occasional bugs sometimes, or minor systems fail
  • Only very minor issues like off-by-one-frame bugs

0 voters

I think this second question might be a little more insightful as well:

When a system fails or bug occurs as a result of setting SignalBehavior to Deferred, how easy is it to debug and fix the issue?
  • Intractably impossible
  • Difficult, but possible to isolate and fix the issue after a lot of effort.
  • Easy, but requires looking deeper into the system than just a one-line fix
  • A one-line fix

0 voters

2 Likes

I heavily rely on disconnecting after an event gets fired once, so this can lead to a lot of things such as multiplicative damage taken if multiple touched events fire at the same frame for instance. I strongly dislike the fact that their response to that is just “remake it”

5 Likes