Help with Data Save code

Help and Feedback Code Review
, ,
1

Even though I love my own way of coding and how I understand it; I really need another set of eyes on this. It is fairly slow and probably not the most efficient way of data saving for inventory so I just need as much feedback as possible to make sure the data saving is reliable and wont cause data loss

local DataStoreService = game:GetService("DataStoreService")
local Players = game:GetService("Players")
local GunFolder = ReplicatedStorage:WaitForChild("Guns")

local playerData = DataStoreService:GetDataStore("PlayerData")

local function loadData(player, what)
	local data
	local item = player:WaitForChild(what)
	local success, errormessage = pcall(function()
		data = playerData:GetAsync(player.UserId .. "-" .. what)
	end)
	
	if success then
		if data ~= nil then
			item.Value = data
		end
	else
		warn(errormessage)
	end
end

function saveToString(array)
	local invString = ""
	for _, v in array:GetChildren() do
		invString = invString .. v.Name .. "|"
	end
	print(invString)
	return invString
end

function readData(dataToRead, player)
	for _, data in string.split(dataToRead, "|") do
		if data ~= "Instance" then
			local gunType = nil
			if GunFolder:FindFirstChild(data) then
				if GunFolder:WaitForChild(data):GetAttribute("gunType") then
					gunType = GunFolder:WaitForChild(data):GetAttribute("gunType")
					local item = nil
					item = Instance.new("StringValue")
					item.Name = data
					item.Value = gunType
					item.Parent = player.Inventory
					print(data)
				end
			end
		end
	end
end

function addGunToPlayerInventory(player, gun, gunType)
	local inventory = player.Inventory
	local newGun = Instance.new("StringValue")
	newGun.Parent = inventory
	newGun.Name = gun
	newGun.Value = gunType
end

-- Player Join
Players.PlayerAdded:Connect(function(player)
	player:SetAttribute("Loaded", false)
	
	local level = Instance.new("NumberValue")
	level.Name = "level"
	level.Parent = player
	
	local points = Instance.new("NumberValue")
	points.Name = "points"
	points.Parent = player
	
	local tickets = Instance.new("NumberValue")
	tickets.Name = "tickets"
	tickets.Parent = player
	
	local timeSurvived = Instance.new("NumberValue")
	timeSurvived.Name = "timeSurvived"
	timeSurvived.Parent = player
	
	local kills = Instance.new("NumberValue")
	kills.Name = "kills"
	kills.Parent = player
	
	local timePlayed = Instance.new("NumberValue")
	timePlayed.Name = "time"
	timePlayed.Parent = player
	
	local timeSurvivedInGame = Instance.new("NumberValue")
	timeSurvivedInGame.Name = "survivalTime"
	timeSurvivedInGame.Parent = player
	
	local roundsSurvived = Instance.new("NumberValue")
	roundsSurvived.Name = "roundsSurvived"
	roundsSurvived.Parent = player
	
	local roundKills = Instance.new("NumberValue")
	roundKills.Name = "roundKills"
	roundKills.Parent = player
	
	local playerGuns = Instance.new("Folder")
	playerGuns.Parent = player
	playerGuns.Name = "Inventory"
	
	local equippedGun = Instance.new("StringValue")
	equippedGun.Parent = player
	equippedGun.Name = "currentGun"
	equippedGun.Value = "Assault Rifle"
	
	local playerSpeed = Instance.new("NumberValue")
	playerSpeed.Parent = player
	playerSpeed.Name = "Speed"
	playerSpeed.Value = game.StarterPlayer.CharacterWalkSpeed
	
	local equippedConsumable = Instance.new("StringValue")
	equippedConsumable.Parent = player
	equippedConsumable.Name = "consumable"
	equippedConsumable.Value = "Nothing Soda"
	
	local colas = Instance.new("NumberValue")
	colas.Parent = player
	colas.Name = "colas"
	colas.Value = 0
	local bandages = Instance.new("NumberValue")
	bandages.Parent = player
	bandages.Name = "bandages"
	bandages.Value = 0
	
	task.wait(0.5)
	
	loadData(player, "level")
	loadData(player, "points")
	loadData(player, "tickets")
	loadData(player, "kills")
	loadData(player, "timeSurvived")
	loadData(player, "time")
	loadData(player, "roundsSurvived")
	loadData(player, "currentGun")
	loadData(player, "colas")
	loadData(player, "bandages")
	
	-- Inventory
	if playerData:GetAsync(player.UserId .. "-" .. "inventory") == nil or tostring(playerData:GetAsync(player.UserId .. "-" .. "inventory")) == " Instance" then
		local item = nil
		item = Instance.new("StringValue", playerGuns)
		item.Name = "Assault Rifle"
		item.Value = "AR"
		item = Instance.new("StringValue", playerGuns)
		item.Name = "Sniper"
		item.Value = "Sniper"
		item = Instance.new("StringValue", playerGuns)
		item.Name = "Sub Machine Gun"
		item.Value = "SMG"
		item = Instance.new("StringValue", playerGuns)
		item.Name = "Shotgun"
		item.Value = "Shotgun"
	else
		print(playerData:GetAsync(player.UserId .. "-" .. "inventory"))
		readData(playerData:GetAsync(player.UserId .. "-" .. "inventory"), player)
		
		if #player.Inventory:GetChildren() == 0 then
			local item = nil
			item = Instance.new("StringValue", playerGuns)
			item.Name = "Assault Rifle"
			item.Value = "AR"
			item = Instance.new("StringValue", playerGuns)
			item.Name = "Sniper"
			item.Value = "Sniper"
			item = Instance.new("StringValue", playerGuns)
			item.Name = "Sub Machine Gun"
			item.Value = "SMG"
			item = Instance.new("StringValue", playerGuns)
			item.Name = "Shotgun"
			item.Value = "Shotgun"
		end
	end
	if player.currentGun.Value == "nil" or player.currentGun.Value == nil then
		player.currentGun.Value = "Assault Rifle"
	end
	if bandages == nil or colas == nil then
		bandages.Value = 0
		colas.Value = 0
	end
end)

function saveData(player)
	if player:GetAttribute("InLoaded") == false then return end
	local saveFolder = Instance.new("Folder")
	saveFolder.Parent = script
	saveFolder.Name = player.UserId
	player.Inventory.Parent = saveFolder
	for i,v in player:GetChildren() do
		if v:IsA("NumberValue") or v:IsA("StringValue") then
			v.Parent = saveFolder
		end
	end
	local success, errormessage = pcall(function()
		playerData:SetAsync(saveFolder.Name.."-level", saveFolder.level.Value)
		playerData:SetAsync(saveFolder.Name.."-points", saveFolder.points.Value)
		playerData:SetAsync(saveFolder.Name.."-tickets", saveFolder.tickets.Value)
		playerData:SetAsync(saveFolder.Name.."-kills", saveFolder.kills.Value)
		playerData:SetAsync(saveFolder.Name.."-time", saveFolder.time.Value)
		playerData:SetAsync(saveFolder.Name.."-timeSurvived", saveFolder.timeSurvived.Value)
		playerData:SetAsync(saveFolder.Name.."-roundsSurvived", saveFolder.roundsSurvived.Value)
		playerData:SetAsync(saveFolder.Name.."-currentGun", saveFolder.currentGun.Value)
		-- Inventory
		playerData:SetAsync(saveFolder.Name.."-inventory", saveToString(saveFolder.Inventory))
		-- Items
		playerData:SetAsync(saveFolder.Name.."-colas", saveFolder.colas.Value)
		playerData:SetAsync(saveFolder.Name.."-bandages", saveFolder.bandages.Value)
	end)

	if success then
		print("Data Saved")
	else
		print("Error saving player data")
		warn(errormessage)
	end
end

Players.PlayerRemoving:Connect(function(player)
	saveData(player)
end)
2

This is bad practice. I would recommend completely restructuring your datastore scripts. Instead of separating each value into its own datastore object, store everything in an array. Additionally, instead of parenting value objects to the player object, you can create a module with get and set methods. Furthermore, your code does not account for data loss and other concerns, which could be an issue down the road.

Instead of coding a datastore module from scratch, I suggest using ProfileStore and Replica which does all the heavy lifting for you.

1 Like
3

Your code is too repetitive. If you wanna avoid that and save ton of time, you can use ProfileService, a datastore module.

1 Like
4

Like they have said, use ProfileStore. It will handle all the necissary stuff for you and includes stuff like session locking to prevent dupe bugs.

Here is a quick basic example code on how to use ProfileStore

--!strict
local Players = game:GetService('Players')

--> References
local ProfileStore = require(script.ProfileStore)

--> Profile Store
local Template = {
	Coins = 0,
	Gems = 0,
	Inventory = {
		['Sword'] = {
			Quantity = 1,
			Equipped = true,
		}
	}
}
local Store = ProfileStore.New('User_Data', Template)

--> Profiles
local Profiles = {} :: {[Player] : typeof(Store:StartSessionAsync(...))}

local function PlayerAdded(player : Player)
	local profile = Store:StartSessionAsync(tostring(player.UserId), {
		OnCancel = function()
			return player.Parent ~= Players
		end
	})

	if profile then
		profile:AddUserId(player.UserId)
		profile:Reconcile() --> update their data with any new missing data from the template.

		Profiles[player] = profile
		
		--> create leaderstats (optional)
		local leaderstats = Instance.new('Folder')
		leaderstats.Name = 'leaderstats'
		
		local Coins = Instance.new('NumberValue')
		Coins.Name = 'Coins'
		
		Coins.Parent = leaderstats
		leaderstats.Parent = player
		
		Coins.Value = profile.Data.Coins
	end
end

local function PlayerRemoving(player : Player)
	local profile = Profiles[player]
	if profile then
		profile:EndSession()
	end
end

--> data functions
local function AddCoins(player : Player, amount : number)
	local profile = Profiles[player]
	if profile == nil then return end --> no profile loaded
	profile.Data.Coins += (tonumber(amount) or 0)
	
	--> update leaderstats
	local leaderstats = player:FindFirstChild('leaderstats')
	local coins = leaderstats:FindFirstChild('Coins') :: NumberValue
	
	coins.Value = profile.Data.Coins
end

--> race condition (if any player joined before this script loaded for some reason)
for _, v in ipairs(Players:GetPlayers()) do
	PlayerAdded(v)
end

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

game:BindToClose(function() --> when game shuts down
	for _, v in ipairs(Players:GetPlayers()) do
		PlayerRemoving(v)
	end
end)
2 Likes
5

Just wanna chime in, I believe you should use ProfileStore as said as above, although it is good practice to learn how to code your own Data Store Module from scratch, I would 100% look into that in the future if you want to learn more about compression, session locking, and overall how data store works.

2 Likes
6

I have made a better and more reliable way of saving data from scratch, but thanks for your feedback!