How to ensure that an arbitrary callback runs for players exactly once when they leave or the server shuts down

Help and Feedback Scripting Support
, , , ,
#1

How you can help

  • If you are a Roblox employee, please let me know if there is a guarantee on PlayerRemoving running before BindToClose upon server shutdown and when the last player leaves.
  • Please let me know if there is a flaw in my logic, a better way to achieve my goal, or any suggestions for optimization.

Solution requirements

  • I want to run a single callback whenever a player leaves exactly once. The player leaving can either be by choice or caused by a shutdown, whether they are the last to leave or not. My use case is saving data.
  • I do not want to place restrictions on this callback, such as whether it yields, whether it uses the task scheduler, how many times it yields, etc.
  • I also do not want the author of the callback to worry about how the game executes it, they should just be able to write an arbitrary function like they would anywhere else.

Resources and previous discussion

There are quite a few previous threads talking about this, including:

What are the problems I’m trying to work around?

  • There is no canonical information about whether PlayerRemoving and BindToClose are guaranteed to fire and in what order.
  • Existing solutions in the Roblox documentation and DevForum do not meet the requirements I outlined above.
    • The example code in the Roblox BindToClose documentation:
      • Does not take into account PlayerRemoving
      • Doesn’t allow multiple yields
      • Doesn’t account for usages of task.wait() or other task scheduler logic. This is because if you task.wait, it yields the thread, and if you have a coroutine.resume() call, you can resume a coroutine before it waits for the appropriate time interval.
      • Is somewhat complex as the child coroutines need to resume the parent.
    • The solution from @sjr04 does not allow for multiple yields or task.wait() calls for similar reasons as the ones above.

Proposed solution

This solution:

  • Assumes that BindToClose always fires upon server shutdown (may not be true during crashes, so we should periodically autosave, but that’s another issue.)
  • Does not assume whether or not PlayerRemoving fires, or if it does fire, assume it runs before BindToClose.
    • In Studio, I have observed PlayerRemoving consistently fire before BindToClose, but I don’t want to assume that will always be the case unless there is a guarantee.
  • Does not place any restrictions on the content of the callback, such as whether it yields, whether it uses task.wait, or anything else.
  • Maintains these properties at the cost of increased latency - it will check at an arbitrary interval whether all of the threads are done or not. The engineer in me kind of cringes as this seems like a hack, but I’m not sure it’s possible to do otherwise. If there is a better way to do this, let me know!
-- This is how often we poll to see whether the callbacks are done
local COROUTINE_CHECK_INTERVAL_SEC = 0.1
-- This map will store the callback thread for each player.
local callbackThreads: { [Player]: thread } = {}

-- This function has multiple yields, print statements, and `task.wait`s so that
-- we can observe it fulfills all of the desired properties.
local playerLeavingCallback = function(_: Player)
	print("Player leaving waiting for one second")
	task.wait(1)
	print("Waiting again...")
	task.wait(1)
	print("Player leaving callback finished!")
end

local spawnPlayerLeavingCallbackIfNotExists = function(player: Player)
	-- If a thread already exists for a player, we do not want to create one.
	if not callbackThreads[player] then
		-- Immediately spawn a thread which will run the callback
		callbackThreads[player] = task.spawn(function()
			playerLeavingCallback(player)
		end)
	end
end

-- This is the normal case when the server isn't shutting down.
game.Players.PlayerRemoving:Connect(spawnPlayerLeavingCallbackIfNotExists)

game:BindToClose(function()
	-- For each player remaining on the server, we spawn a callback if one doesn't exist.
	for _, player in game.Players:GetPlayers() do
		spawnPlayerLeavingCallbackIfNotExists(player)
	end
	
	local threadsStillRunning: boolean
	repeat
		-- We want to determine if any callback hasn't finished running.
		local threadsStillRunning = false
		for _, thread in callbackThreads do
			if coroutine.status(thread) ~= "dead" then
				threadsStillRunning = true
				break
			end
		end

		-- If threads are still running, yield the thread for some time 
		-- to wait for the callbacks to finish.
		if threadsStillRunning then
			task.wait(COROUTINE_CHECK_INTERVAL_SEC)
		end
	until not threadsStillRunning
end)
1 Like