Operation of the DataStore

Help and Feedback Code Review
1

hi guys, i made a DataStore and i want to get feedback from more experienced people. is it reliably done or are there unsafe places in my code, if there is an opportunity to give feedback, I will appreciate it.

i may not have done much optimization, but in the future a lot of values will be stored, so i’m contacting you so that i don’t lose the data later and so that it doesn’t get damaged as a result of some bug. >_o

this is my first DataStore, so please don’t judge me too harshly.

-- server module

local navigations = {
	acceptPLOT = require("@game/ServerScriptService/server/plot/acceptPLOT"),
	rejectPLOT = require("@game/ServerScriptService/server/plot/rejectPLOT"),
	data = game:GetService("DataStoreService"),
	
	--..
	studio = game:GetService("RunService"):IsStudio(),
	isStudio = false -- saved studio
}

local dataPLAYER = {} -- not touch
local data = {}

local default = {
	cash = 0,
	test = true,
	
	--..
	lastJOIN = nil,
	lastREJECT = nil
}

local configuration = {
	data = "players"
}

function reconcile(...) -- reconcile data
	local a = {...} --a[1] - player, a[2] - default data
	for key, value in pairs(a[2]) do
		if (not a[1][key]) then
			a[1][key] = value
		end
	end
	return a[1]
end

function dataPLAYER.load(...) -- load data
	local a = {...} -- a[1] - player
	local ds = navigations.data:GetDataStore(configuration.data) -- get data

	local success, err = pcall(function()
		local val = ds:GetAsync(a[1].UserId) or default
		if val == nil then
			val = table.clone(default)
		end
		
		reconcile(val, default)
		
		data[a[1].UserId] = val -- load data
		data[a[1].UserId].lastJOIN = os.time() -- save time join
		print(val)
	end)
	
	if not success then
		warn(err)
		a[1]:Kick("your data failed to load, please rejoin in game.")
	else
		print(success)
		navigations.acceptPLOT(a[1]) -- load plot player
	end
	
	return data[a[1].UserId]
end

function dataPLAYER.save(...) -- save data
	local a = {...} -- a[1] - player
	local ds = navigations.data:GetDataStore(configuration.data) -- get data
	
	if navigations.isStudio then
		return print("studio disabled saved.")
	end
	
	local success, err = pcall(function()
		local val = data[a[1].UserId]
		if val == nil then
			val = table.clone(default)
		end
		
		ds:UpdateAsync(a[1].UserId, function()
			data[a[1].UserId].lastREJECT = os.time() -- save time reject
			print(data[a[1].UserId])
			return val
		end)
	end)
	
	if not success then
		warn(err)
	else
		print(success)
	end
	
	navigations.rejectPLOT(a[1]) -- reject plot player
	
	return data[a[1].UserId]
end

return dataPLAYER


local navigations = {
	dataPLAYER = require("@game/ServerScriptService/server/data/dataPLAYER"),
	players = game:GetService("Players")
}

navigations.players.PlayerAdded:Connect(function(player) -- load player data
	player.CharacterAdded:Connect(function(character)
		navigations.dataPLAYER.load(player)
	end)
end)

navigations.players.PlayerRemoving:Connect(function(player) -- save player data
	navigations.dataPLAYER.save(player)
end)

game:BindToClose(function() -- save all data
	for i, v in navigations.players:GetPlayers() do
		navigations.dataPLAYER.save(v)
	end
end)
2

Is this code AI generated?

In terms of organization, you don’t really need to make a table to define your variables, defining them outside of the table is just fine.

Naming schemes seem a bit strange, would look into camel case or other naming conventions to make your code more readable.

For loading data, I would typically try to load it 2-3 times before kicking the player. You can do that with a loop very easily.

You also don’t need to call :GetDataStore every time you call the save function. You can define it as a variable once at the top of the code alongside the rest of your variables and be done with it. Please also use GetAsync to reference any data.

I don’t really code my own datastore modules since ProfileStore by loleris does the job fine for me. Would advise you look into it.

1 Like
3

Your structure is fine, the risky parts are all in the details. I went through it properly, here are the ones that will actually cost you data, worst first.

or default hands out the shared table, not a copy

local val = ds:GetAsync(a[1].UserId) or default
if val == nil then
    val = table.clone(default)
end

For a brand new player GetAsync returns nil, so val becomes default itself, the same table the whole server shares. Then data[a[1].UserId] = val and data[a[1].UserId].lastJOIN = os.time() write straight into it. From that point default is not your defaults any more, it is that player’s live data, and the next new player who joins inherits their cash.

The table.clone line underneath is meant to stop exactly this, but it can never run, because or default already guaranteed val is not nil. Swap the order:

local val = ds:GetAsync(a[1].UserId)
if val == nil then
    val = table.clone(default)
end

Also worth knowing table.clone is shallow. As soon as you nest something, an inventory table for example, every new player shares that inner table again and you get the same bug one level down.

Data reloads on every respawn

navigations.players.PlayerAdded:Connect(function(player)
    player.CharacterAdded:Connect(function(character)
        navigations.dataPLAYER.load(player)
    end)
end)

CharacterAdded fires every time they spawn, so this re-runs on every death. Each run does a fresh GetAsync and overwrites data[UserId] with whatever is on disk, throwing away everything the player earned since their last save. Die once and the cash you handed them this session is gone.

Load on PlayerAdded instead. If you need the character present for acceptPLOT, connect CharacterAdded separately for just that bit.

reconcile tests truthiness instead of nil

if (not a[1][key]) then

In Luau only nil and false are falsy, so your numbers are safe, but a stored false is not. You already have test = true in the defaults. The day a player’s test is saved as false, it gets flipped back to true on their next load, and the same will happen to every boolean you add later, the hasCompletedTutorial kind of field. Use:

if a[1][key] == nil then

UpdateAsync throws away the value it is handed

ds:UpdateAsync(a[1].UserId, function()
    return val
end)

The transform function receives the currently stored value as its first argument, and that is the whole reason to use UpdateAsync over SetAsync. Roblox may also run your function again with fresher data if the key changed underneath you. Ignoring the argument makes this a blind overwrite that costs more than SetAsync would have. Either take the old value and merge with it, or use SetAsync and be honest about what it does.

Nothing saves on shutdown

PlayerRemoving covers someone leaving normally. It does not reliably cover the server itself closing, which is exactly when you lose whole sessions. Add:

game:BindToClose(function()
    for _, player in ipairs(navigations.players:GetPlayers()) do
        navigations.dataPLAYER.save(player)
    end
end)

Your Studio guard never fires

studio = game:GetService("RunService"):IsStudio(),
isStudio = false -- saved studio

save checks navigations.isStudio, which is hardcoded false, so the early return never happens and Studio testing does write to live player data. The real value is sitting in navigations.studio right above it.

Small one to finish: data[a[1].UserId] is never set back to nil after saving, so that table keeps every player who has ever joined for the lifetime of the server.

On the retry point owenkb raised, that is worth doing, but only wrap the GetAsync read in the retry loop with a task.wait between attempts before you kick. Reading is safe to repeat.