Datastore code review

Help and Feedback Code Review
#1

Ok so in my 7 months of scripting I find myself constantly having to make a new datastore for every project I undertook, so I tried making a more easily customisable one.

However as i’m continually striving to improve I would like to know any bad practises / potentially damaging flaws this new datastore I made has.

All feedback appreciated

Model: https://www.roblox.com/library/4218353378/Customizable-Datastore

code:

-- Configurations --
AddNewValues = true --If you add a new value to the datalist it will auto load on exisitng data stores
SaveAttempts = 3 -- if there is a datastore error how many times the script will try again and save
SaveVersionId = 1 -- if you have AddNewValues as true, change this whenever you change the standard data list to update players onto new save

-- Vars --
local DSS = game:GetService("DataStoreService")
local PlayerDataStore = DSS:GetDataStore("PlayerData2")

local DataList = {
	SurvivalStats = {
		Thirst = {Current = 100, Max = 100};
		Health = {Current = 100, Max = 100};
		Stamina = {Current = 100, Max = 100};
		Hunger = {Current = 100, Max = 100};
	};
	Core = {
		Gold = 0;
		Position = {X = 0, Y = 0, Z = 0};
		Level = {Level = 1, XpNeeded = 10, Xp = 5};
	};
	
}


local function CapitalFirst(str)
    return str:sub(1,1):upper()..str:sub(2).. "Value"
end

function SetUpPlayerFunctions(Player, Data)
	-- Here is where you put your custom functions which you want to occur 
end

function TableToFolder(Table, Parent)
	for i,v in pairs(Table) do
		if typeof(v) ~= "table" then
			local T = CapitalFirst(typeof(v))
			if T == "BooleanValue" then
				T = "BoolValue"
			end
			local Value =  Instance.new(T)
			Value.Name = i
			Value.Parent = Parent
			Value.Value = v
		else
			local F = Instance.new("Folder")
			F.Parent = Parent
			F.Name = i
			TableToFolder(v, F)
		end
	end
	return Parent
end

function AddNewValuesFunc(Table, Parent)
	for i,v in pairs(Table) do
		local P = Parent:FindFirstChild(i)
		if typeof(v) ~= "table" then
			if not P then
			local T = CapitalFirst(typeof(v))
			if T == "BooleanValue" then
				T = "BoolValue"
			end
			local Value =  Instance.new(T)
			Value.Name = i
			Value.Parent = Parent
			Value.Value = v
			end
		else
			if not P then
			P = Instance.new("Folder")
			P.Parent = Parent
			P.Name = i
			end
			AddNewValuesFunc(v, P)
		end
	end
	return Parent
end

function IterateThroughFolder(Folder)
	local Table = {}
	for i,v in pairs(Folder:GetChildren()) do
		if #v:GetChildren() == 0 or not v:IsA("Folder") then
			Table[v.Name] = v.Value
		else
			Table[v.Name] = IterateThroughFolder(v)
		end
	end	
	return Table
end






function SaveData(Player)
	if Player.Loaded.Value then
	
	local DataTable = IterateThroughFolder(Player.Data)
	DataTable["VersionNumber"] = SaveVersionId

	local S, E = pcall(function()
	PlayerDataStore:SetAsync(Player.UserId, DataTable)
	
	
	end)
	
	local C = 0
	if not S then
	repeat
		C = C + 1
		S, E = pcall(function()
	    PlayerDataStore:SetAsync(Player.UserId, DataTable)
		end)
		wait(3)
	until S or C > SaveAttempts
	end
	end
end

function LoadData(Player)
	local Loaded = Instance.new("BoolValue")
	Loaded.Name = "Loaded"
	Loaded.Parent = Player
	Loaded.Value = false
	
	local S, E = pcall(function()
	Data = PlayerDataStore:GetAsync(Player.UserId)
	end)
	
	local C = 0
	if not S then
	repeat
		C = C + 1
		S, E = pcall(function()
	    Data = PlayerDataStore:GetAsync(Player.UserId)
		end)
		wait(3)
	until S or C > SaveAttempts
	end
	
	if not S then
		Player:Kick("Data failed to load, if issue persists contact a dev")
	end
	
	if not Data then
		print(Player.Name.. " is a new player")
		Data = DataList
	end
	
	local CoreFolder = Instance.new("Folder")
    CoreFolder.Parent = Player
    CoreFolder.Name = "Data"
	
	TableToFolder(Data, CoreFolder)
											
	if not Player.Data:FindFirstChild("VersionNumber") then
	local Value =  Instance.new("IntValue")
	Value.Name = "VersionNumber"
	Value.Parent = Player.Data
	Value.Value = SaveVersionId
	end
	
	if AddNewValues and Player.Data.VersionNumber.Value ~= SaveVersionId then
		AddNewValuesFunc(DataList, Player.Data)
	end
	
	if not Player.Character then
		Player.CharacterAdded:wait()
	end
	
	SetUpPlayerFunctions(Player, Player.Data)
	
	Loaded.Value = true
end

game.Players.PlayerAdded:Connect(LoadData)
game.Players.PlayerRemoving:Connect(SaveData)
5 Likes
#2

I read through it a couple times and didn’t notice anything really bad. One thing you might want to look into is DataModel:BindToClose, since sometimes not every PlayerRemoving event fires on a shutdown.

I noticed some parts of the code could be bad from a readability perspective, there’s a lot of over-engineering and weird solutions to problems, like for example the CapitalFirst function. The way that’s used makes me wary, especially when there’s only a small handful of types you’re saving, and the fact that you have a special cases dedicated to correcting the outut of CapitalFirst. It almost adds more code than a simple if/else block. If you really need to use value instances then I would recommend changing your DataList table so that it contains NumberValues, BooleanValues, etc. from the start instead of literals.

3 Likes
#3

For only seven months into learning this is very well done. You are handling everything properly and even kicking the player on data load failure which weirdly seems to be uncommon among newer devs. Really the only relatively bad thing I’ve noticed is that the formatting is a little messy but other than that it’s well done! :smile:

2 Likes
#4

For seven months, you’ve really improved quick! I didn’t notice anything wrong after reading it a few times, so looks great!

2 Likes
#5

Mhm I can agree with that, especially on the loading and saving functions, I just kept adding stuff to it without structuring it properly

2 Likes
#6

Mhm, I am aware of using BindToClose, I currently use it to save building data to a vip server however I will definitely modify my code to iterate trough players and fire the save function!

And ye, as I added more to code kinda got a bit messy

2 Likes
#7

For the record I agree with the others that for 7 months of scripting you’re great. Most people seem to take a couple years at least to reach your level, so if you didn’t know Lua or any programming ahead of time then you’ve got a gift and the problems people find will likely be nitpicks. :slight_smile:

3 Likes