DataStore Inconsistancy

[Bearturnedhuman (BearPlusPlus)]

I’m not sure if this is a bug. I previously made a post claiming the BindToClose() method was unnecessary and that only .PlayerRemoving was required. This held true when saving data of any size but I’ve noticed an inconsistency after revising the codebase.

This is the hierarchy (the rest is a normal baseplate)

This is the code in the Server script

-- Services
local Players = game:GetService("Players")
local ServerStorage = game:GetService("ServerStorage")

-- Containers
local ServerModules = ServerStorage["Server Modules"]

-- Modules
local DATASTORE = require(ServerModules.DataStore)

-- Functions
local function PlayerAdded(player)
	print(tostring(player) .. " joined the game!")
	DATASTORE.LoadPlayerData(player)
end

local function PlayerRemoving(player)
	print(tostring(player) .. " is leaving!")
	task.wait(2) -- To yield and stop SavePlayerData() from firing, simulating a failed save
	DATASTORE.SavePlayerData(player)
end

local function Exit()
	for Index, player in pairs(Players:GetChildren()) do
		if not DATASTORE.Saved[player] then
			print("Save")
			DATASTORE.SavePlayerData(player)
		end
	end
	
	print("Server closed")
end

-- Connections
Players.PlayerAdded:Connect(PlayerAdded)
Players.PlayerRemoving:Connect(PlayerRemoving)

-- BindToClose
game:BindToClose(Exit)

This is the code in the DataStore module

-- Services
local DataStoreService = game:GetService("DataStoreService")

-- DataStore
local DataStore = DataStoreService:GetDataStore("DataStore Index")

-- Module
local MODULE = {}
MODULE.Saved = {} -- To store if their data has already been saved

function MODULE.SavePlayerData(player)
	local function SetData()
		DataStore:IncrementAsync(player.UserId, 1) -- Increase their data by 1
	end
	
	local Success = pcall(SetData)
	
	if Success then
		print(tostring(player) .. "'s data saved!")
		MODULE.Saved[player] = true
	end
end

function MODULE.LoadPlayerData(player)
	local Data
	local function GetData()
		Data = DataStore:GetAsync(player.UserId) -- player.UserId is only called once so I didn't create a variable for it
	end
	
	local Success = pcall(GetData)
	
	if Success then
		if not Data then -- Initialize the data if it doesn't exist
			Data = 0
		end
		print(tostring(player) .. "'s data: " ..  tostring(Data))
	end
end

return MODULE

Sometimes (most of the time) I get this output

Rarely I get this output

Nothing consistently changes the outcome.

I’ve replaced the Exit() function with the code below and that seemed to slightly increase the rate at which it successfully saves

local function Exit()
	for Index, player in pairs(Players:GetChildren()) do
		print(tostring(player)) -- Added this
		if not DATASTORE.Saved[player] then
			print("Save")
			DATASTORE.SavePlayerData(player)
		end
	end
	
	print("Server closed")
end

The server closes before the data can save and I want to avoid yielding with methods like task.wait() it feels so unreliable and hacky

I also tried replacing the Saved index with player.UserId instead of the player object, that didn’t work

I’ve considered making a handler but if the code isn’t reliably firing I’m not sure how I can expect to integrate one (adding prints for some reason increases the success rate)

I’m thinking that as long as you don’t yield the .PlayerRemoving event, it should be quick enough (not matter the size) to save the data. My issue is also wanting to perform other actions, like refunding something a player spawned in the server (checking for it, removing it, then modifying the data)

Note: I cannot test in an actual game because the output isn’t replicated when servers shutdown (most of the time, that also seems to be inconsistent)

[Judgy_Oreo (DisplayName)]

@Bearturnedhuman

I want to avoid yielding with methods like task.wait() it feels so unreliable and hacky

It’s not unreliable, in fact I believe it’s what most people use because it’s pretty simple to just stop the server from shutting down for a few seconds so all the data can save, just by putting a task.wait(10) in the :BindToClose function.

“Hackyness” is subjective in this case.

@Bearturnedhuman

it should be quick enough (not matter the size)

Are you sure about this? It sounds kind of weird that the server uploads the data at the same speed no matter the size.

[Bearturnedhuman (BearPlusPlus)]

@Judgy_Oreo

Are you sure about this? It sounds kind of weird that the server uploads the data at the same speed no matter the size.

It’s not too weird, since it’s to Roblox’s own endpoint, the time from the SetAsync() method and the callback is fractions of a second. It feels insecure to just use task.wait(n) which is why I’m hesitant to just slap that in there. I like having everything connected to events with no loose ends or yielding longer than it has to.

[McThor2]

Looking at the BindToClose docs they give an example of using coroutines to make sure the saving gets completed.

[Bearturnedhuman (BearPlusPlus)]

I saw that and I feel really dumb because I looked at it and went “ew the indention” and ignored it. It is exactly what I was looking for ;-;

[McThor2]

Yeah the indentation is a bit whack. Less icky indentation below:

local DataStoreService = game:GetService("DataStoreService")
local RunService = game:GetService("RunService")

local playerDataStore = DataStoreService:GetDataStore("PlayerData")

local allPlayerSessionDataCache = {}

local function savePlayerDataAsync(userId, data)
	return playerDataStore:UpdateAsync(userId, function(oldData)
		return data
	end)
end

local function onServerShutdown()
	if RunService:IsStudio() then
		-- Avoid writing studio data to production and stalling test session closing
		return
	end

	-- Reference for yielding and resuming later
	local mainThread = coroutine.running()

	-- Counts up for each new thread, down when the thread finishes. When 0 is reached,
	-- the individual thread knows it's the last thread to finish and should resume the main thread
	local numThreadsRunning = 0

	-- Calling this function later starts on a new thread because of coroutine.wrap
	local startSaveThread = coroutine.wrap(function(userId, sessionData)
		-- Perform the save operation
		local success, result = pcall(savePlayerDataAsync, userId, sessionData)
		if not success then
			-- Could implement a retry
			warn(string.format("Failed to save %d's data: %s", userId, result))
		end

		-- Thread finished, decrement counter
		numThreadsRunning -= 1

		if numThreadsRunning == 0 then
			-- This was the last thread to finish, resume main thread
			coroutine.resume(mainThread)
		end
	end)

	-- This assumes playerData gets cleared from the data table during a final save on PlayerRemoving,
	-- so this is iterating over all the data of players still in the game that hasn't been saved
	for userId, sessionData in pairs(allPlayerSessionDataCache) do
		numThreadsRunning += 1

		-- This loop finishes running and counting numThreadsRunning before any of
		-- the save threads start because coroutine.wrap has built-in deferral on start
		startSaveThread(userId, sessionData)
	end

	if numThreadsRunning > 0 then
		-- Stall shutdown until save threads finish. Resumed by the last save thread when it finishes
		coroutine.yield()
	end
end

game:BindToClose(onServerShutdown)

[Bearturnedhuman (BearPlusPlus)]

I appreciate it. I feel incredibly dumb for this one but that’s part of programming I guess