Is my datastore script secure from dataloss?

Help and Feedback Code Review
#1

Hello, and I have made a datastore script for my friend.
I would like to know, how can I improve my script, maybe remove something, is there a chance, to experience dataloss?

--// services
local players = game:GetService("Players")
local dss = game:GetService("DataStoreService")

--// variables
local dataBase = dss:GetDataStore("RandomNameHere") --// changing the name will make all data go poo, but putting it back it will revert the data

--// functions
local function getData(plr)
	local data = nil
	local leaderstats = nil
	
	for x = 1,10 do --// max 10 attempts
		local success, error = pcall(function()
			data = dataBase:GetAsync("player_#"..plr.UserId)
		end)
		---
		if success then
			leaderstats = plr:WaitForChild("leaderstats")
			
			---
			if data ~= nil then
				leaderstats:WaitForChild("Tokens").Value = data.tokens
				---
				plr:WaitForChild("loaded").Value = plr:WaitForChild("loaded").Value + 1
				---
				leaderstats:WaitForChild("Time Played").Value = data.time
				---
				plr:WaitForChild("loaded").Value = plr:WaitForChild("loaded").Value + 1
			else
				print("New player: "..plr.Name)
				plr:WaitForChild("loaded").Value = 2
			end
			---
			
			print("Successfull data load for #"..plr.UserId.." ("..plr.Name.."). Attempt: "..x)
			break --// ends loop
		end
		---
		if error then
			plr:WaitForChild("loaded").Value = 0
			print("Unsuccessfull data load for #"..plr.UserId.." ("..plr.Name.."). Attempt: "..x.." | Error: "..error)
		end
		--
		wait(2) --// coldown between each attempt
	end
end

local function saveData(plr)
	for x = 1,10 do --// max 10 attempts
		local success, error = pcall(function()
			dataBase:SetAsync("player_#"..plr.UserId, {time = plr.leaderstats["Time Played"].Value, tokens = plr.leaderstats.Tokens.Value})
		end)
		---
		if success then
			print("Successfull data save for #"..plr.UserId.." ("..plr.Name.."). Attempt: "..x)
			break --// ends loop
		end
		---
		if error then
			print("Unsuccessfull data save for #"..plr.UserId.." ("..plr.Name.."). Attempt: "..x.." | Error: "..error)
		end
		--
		wait(2) --// coldown between each attempt
	end
end

players.PlayerAdded:Connect(function(plr)
	local leaderstats = Instance.new("Folder", plr)
	leaderstats.Name = "leaderstats"
	
	local timePlayed = Instance.new("NumberValue", leaderstats)
	timePlayed.Name = "Time Played"
	timePlayed.Value = 0
	
	local tokens = Instance.new("NumberValue", leaderstats)
	tokens.Name = "Tokens"
	tokens.Value = 0 --// starting tokens count
	
	local loaded = Instance.new("IntValue", plr)
	loaded.Name = "loaded" --// do not touch
	loaded.Value = 0 --// do not touch
	
	---
	getData(plr)
end)

players.PlayerRemoving:Connect(function(plr)
	if plr.loaded.Value == 2 then
		saveData(plr)
	else
		print(plr.Name.." is leaving too fast, wont save data.")
	end
end)

game:BindToClose(function()
	for _, x in pairs(players:GetChildren()) do
		saveData(x)
		
		print("Unexpected server shutdown, saving data for "..x.Name)
	end
end)

Thanks for the help!
Yes, please don’t copy the script

#2

You’re making memory leaks because you’re parenting instances one by one instead you parent all of the children’s of the folder before you parent the folder in the final.

What you’re doing:
Create Folder and Parent It —> Create Other Children and Parent Them to The Folder One By One

What you SHOULD be doing:
Create folder and DON’T PARENT IT YET. —> Create all other children of the folder and DON’T PARENT IT YET —> After creating the children, you may parent all of the children to the folder. —> after you got all the children in the folder, you may parent the folder to the player as the final decision.

Instead of just creating extra variables, you could use the return function to get value of whether the game can get the data for player and the data of the player.

Correct way:

local success, result = pcall(function()
    return DataStore:GetAsync(plr)
end)
if success then
    — use result variable to load data.
else
    error("DataStore failed to get data for "..plr.UserId..". Error code: "..result")
end

By using return, the success variable will contain whether the data store can get the data for the player, while the result variable contains the player’s data, in a dictionary obviously. If the data store failed to get the player’s data, the result variable will be the error code for the failure.

This is literally pointless, just replace it with my code.

This as well, in your saveData function, won’t work.

Also, using SetAsync() to save data is hazardous if you’re using it to save important data. Use UpdateAsync() instead.

1 Like
#3

Thanks for explaining, I will update my code, also I made this post, cause I wanted to use it in my future game. Also will the old data load on rejoin, if data failed to load 10 times? Cause value loaded wont be 2. That’s why I made loaded value. So on leave data specially wont save, to not replace data with 0

1 Like