Is this a secure data system in you guys' opinions?

I don’t use updateasync due to the speed, but I do a small sanity check to ensure data has been checked to exist or not before allowing the game to save data.

Getting Data

	while not DataChecked and RetryCount < 6 do
		wait()
		RetryCount = RetryCount + 1
		local DataLoadSuccess, DataLoadIssue = pcall(function()
			GetData = CurrentDataStore:GetAsync(Key)
		end)
		if DataLoadSuccess then
			DataChecked = true
			if GetData ~= nil then
				print("Data found for "..player.Name)
				if GetData.Cash ~= nil then
					Cash.Value = GetData.Cash
				else
					print("Data found for "..player.Name.." but cash is a new stat for the player and will apply on next save")
				end
				if GetData.Rubies ~= nil then
					Rubies.Value = GetData.Rubies
				else
					print("Data found for "..player.Name.." but rubies is a new stat for the player and will apply on next save")
				end
				if GetData.Diamonds ~= nil then
					Diamonds.Value = GetData.Diamonds
				else
					print("Data found for "..player.Name.." but diamonds is a new stat for the player and will apply on next save")
				end
				if GetData.XP ~= nil then
					XP.Value = GetData.XP
				else
					print("Data found for "..player.Name.." but xp is a new stat for the player and will apply on next save")
				end
				if GetData.Level ~= nil then
					Level.Value = GetData.Level
				else
					print("Data found for "..player.Name.." but level is a new stat for the player and will apply on next save")
				end
				if GetData.Wins ~= nil then
					Wins.Value = GetData.Wins
				else
					print("Data found for "..player.Name.." but wins is a new stat for the player and will apply on next save")
				end
				if GetData.RadioID ~= nil then
					RadioID.Value = GetData.RadioID
				else
					print("Data found for "..player.Name.." but radio id is a new stat for the player and will apply on next save")
				end
				if GetData.RadioPitch ~= nil then
					RadioPitch.Value = GetData.RadioPitch
				else
					print("Data found for "..player.Name.." but radio pitch is a new stat for the player and will apply on next save")
				end
				if GetData.RadioSkin ~= nil then
					RadioSkin.Value = GetData.RadioSkin
				else
					print("Data found for "..player.Name.." but radio skin is a new stat for the player and will apply on next save")
				end
				if GetData.KnifeSkin ~= nil then
					KnifeSkin.Value = GetData.KnifeSkin
				else
					print("Data found for "..player.Name.." but knife skin is a new stat for the player and will apply on next save")
				end
				if GetData.GunSkin ~= nil then
					GunSkin.Value = GetData.GunSkin
				else
					print("Data found for "..player.Name.." but gun skin is a new stat for the player and will apply on next save")
				end
				if GetData.KnifeEffect ~= nil then
					KnifeEffect.Value = GetData.KnifeEffect
				else
					print("Data found for "..player.Name.." but knife effect is a new stat for the player and will apply on next save")
				end
				if GetData.GunEffect ~= nil then
					GunEffect.Value = GetData.GunEffect
				else
					print("Data found for "..player.Name.." but gun effect is a new stat for the player and will apply on next save")
				end
				if GetData.KnifePower ~= nil then
					KnifePower.Value = GetData.KnifePower
				else
					print("Data found for "..player.Name.." but knife power is a new stat for the player and will apply on next save")
				end
				if GetData.Gear ~= nil then
					Gear.Value = GetData.Gear
				else
					print("Data found for "..player.Name.." but gear is a new stat for the player and will apply on next save")
				end
				if GetData.Pet ~= nil then
					Pet.Value = GetData.Pet
				else
					print("Data found for "..player.Name.." but gear is a new stat for the player and will apply on next save")
				end
				if GetData.Perk ~= nil then
					Perk.Value = GetData.Perk
				else
					print("Data found for "..player.Name.." but gear is a new stat for the player and will apply on next save")
				end
				if GetData.KnifeSkins ~= nil then
					local KS = GetData.KnifeSkins
					for k = 1, #KS do
						if KS[k] ~= nil then
							local AddItem = Instance.new("ObjectValue")
							AddItem.Name = KS[k]
							AddItem.Parent = KnifeSkins
						end
					end
				else
					print("KnifeSkins new to "..player.Name.."'s Data Table")
				end
				if GetData.GunSkins ~= nil then
					local GS = GetData.GunSkins
					for l = 1, #GS do
						if GS[l] ~= nil then
							local AddItem = Instance.new("ObjectValue")
							AddItem.Name = GS[l]
							AddItem.Parent = GunSkins
						end
					end
				else
					print("GunSkins new to "..player.Name.."'s Data Table")
				end
				if GetData.KnifeEffects ~= nil then
					local KE = GetData.KnifeEffects
					for m = 1, #KE do
						if KE[m] ~= nil then
							local AddItem = Instance.new("ObjectValue")
							AddItem.Name = KE[m]
							AddItem.Parent = KnifeEffects
						end
					end
				else
					print("KnifeEffects new to "..player.Name.."'s Data Table")
				end
				if GetData.GunEffects ~= nil then
					local GE = GetData.GunEffects
					for n = 1, #GE do
						if GE[n] ~= nil then
							local AddItem = Instance.new("ObjectValue")
							AddItem.Name = GE[n]
							AddItem.Parent = GunEffects
						end
					end
				else
					print("GunEffects new to "..player.Name.."'s Data Table")
				end
				if GetData.KnifePowers ~= nil then
					local KP = GetData.KnifePowers
					for o = 1, #KP do
						if KP[o] ~= nil then
							local AddItem = Instance.new("ObjectValue")
							AddItem.Name = KP[o]
							AddItem.Parent = KnifePowers
						end
					end
				else
					print("KnifePowers new to "..player.Name.."'s Data Table")
				end
				if GetData.Gears ~= nil then
					local GRS = GetData.Gears
					for p = 1, #GRS do
						if GRS[p] ~= nil then
							local AddItem = Instance.new("ObjectValue")
							AddItem.Name = GRS[p]
							AddItem.Parent = Gears
						end
					end
				else
					print("Gears new to "..player.Name.."'s Data Table")
				end
				if GetData.Pets ~= nil then
					local PTS = GetData.Pets
					for q = 1, #PTS do
						if PTS[q] ~= nil then
							local AddItem = Instance.new("ObjectValue")
							AddItem.Name = PTS[q]
							AddItem.Parent = Pets
						end
					end
				else
					print("Pets new to "..player.Name.."'s Data Table")
				end
				if GetData.Perks ~= nil then
					local PKS = GetData.Perks
					for r = 1, #PKS do 
						if PKS[r] ~= nil then
							local AddItem = Instance.new("ObjectValue")
							AddItem.Name = PKS[r]
							AddItem.Parent = Perks
						end
					end
				else
					print("Perks new to "..player.Name.."'s Data Table")
				end
			else
				print("New data must be created for "..player.Name)
				Level.Value = 1
				XP.Value = 1000
				Wins.Value = 0
				Cash.Value = 0
				Rubies.Value = 0
				Diamonds.Value = 0
			end
			local DataFolder = Instance.new("Folder")
			DataFolder.Name = Key
			DataFolder.Parent = game.ServerStorage.DataBench	
			break
		elseif DataLoadIssue then
			if RetryCount < 6 then
				warn("Player Data request for"..player.Name.." has failed, retrying ("..RetryCount..")")
			else
				warn("Player Data request for "..player.Name.." has failed after "..RetryCount.." attempts, kicking player to avoid data corruption")
				player:Kick("To avoid data loss! You may rejoin")
			end
		end
	end

Updating Data (General Data and storing levels for leaderboard)

local function SaveData(player)
	local Key = GetKey(player)
	local DataTable = {
		Cash = player.Data.Cash.Value,
		Rubies = player.Data.Rubies.Value,
		Diamonds = player.Data.Diamonds.Value,
		XP = player.Data.XP.Value,
		Level = player.leaderstats.Level.Value,
		Wins = player.Data.Wins.Value,
		RadioID = player.Data.RadioID.Value,
		RadioPitch = player.Data.RadioPitch.Value,
		RadioSkin = player.Data.RadioSkin.Value,
		KnifeSkin = player.Data.KnifeSkin.Value,
		GunSkin = player.Data.GunSkin.Value,
		KnifeEffect = player.Data.KnifeEffect.Value,
		GunEffect = player.Data.GunEffect.Value,
		KnifePower = player.Data.KnifePower.Value,
		Gear = player.Data.Gear.Value,
		Pet = player.Data.Pet.Value,
		Perk = player.Data.Perk.Value,
		KnifeSkins = {},
		GunSkins = {},
		KnifeEffects = {},
		GunEffects = {},
		KnifePowers = {},
		Gears = {},
		Pets = {},
		Perks = {},
	}
	local KnifeSkins = player.Data.KnifeSkins:GetChildren()
	local GunSkins = player.Data.GunSkins:GetChildren()
	local KnifeEffects = player.Data.KnifeEffects:GetChildren()
	local GunEffects = player.Data.GunEffects:GetChildren()
	local KnifePowers = player.Data.KnifePowers:GetChildren()
	local Gears = player.Data.Gears:GetChildren()
	local Pets = player.Data.Pets:GetChildren()
	local Perks = player.Data.Perks:GetChildren()
	for s = 1, #KnifeSkins do
		if KnifeSkins[s] ~= nil then
			table.insert(DataTable.KnifeSkins, KnifeSkins[s].Name)
		end
	end
	for t = 1, #GunSkins do
		if GunSkins[t] ~= nil then
			table.insert(DataTable.GunSkins, GunSkins[t].Name)
		end
	end
	for u = 1, #KnifeEffects do
		if KnifeEffects[u] ~= nil then
			table.insert(DataTable.KnifeEffects, KnifeEffects[u].Name)
		end
	end
	for v = 1, #GunEffects do
		if GunEffects[v] ~= nil then
			table.insert(DataTable.GunEffects, GunEffects[v].Name)
		end
	end
	for w = 1, #KnifePowers do
		if KnifePowers[w] ~= nil then
			table.insert(DataTable.KnifePowers, KnifePowers[w].Name)
		end
	end
	for x = 1, #Gears do
		if Gears[x] ~= nil then
			table.insert(DataTable.Gears, Gears[x].Name)
		end
	end
	for y = 1, #Pets do
		if Pets[y] ~= nil then
			table.insert(DataTable.Pets, Pets[y].Name)
		end
	end
	for z = 1, #Perks do 
		if Perks[z] ~= nil then
			table.insert(DataTable.Perks, Perks[z].Name)
		end
	end
	local GetDataFolder = game.ServerStorage.DataBench:FindFirstChild(Key)
	if GetDataFolder then
		local CurrentDataSaved, Issue = pcall(function()
			CurrentDataStore:SetAsync(Key, DataTable)
		end)
		if CurrentDataSaved then
			print("Data saved for "..player.Name)
		else
			warn("Issue saving data for "..player.Name)
		end
		local ValidUser = false
		local LevelStored, Issue = pcall(function()
			if tonumber(string.sub(Key, string.len("User-#"), string.len(Key))) >= 1 then
				ValidUser = true
				GlobalLeaderboard:SetAsync(Key, DataTable.Level)
			end
		end)
		if LevelStored and ValidUser then
			print("Stored level of "..player.Name.." for comparing")
		else
			warn("Issue storing the level of "..player.Name.." for comparing")
		end
		GetDataFolder:Destroy()
	end
end

Updating Leaderboard

local function UpdateLeaderboard()
	local Leaderboard = workspace.Lobby.Top10Players.LB.SurfaceGui
	local ColorByRank = Module.ColorByRank
	local LevelDataToCompare = "null"
	local GotData, IssueComparing = pcall(function()
		LevelDataToCompare = GlobalLeaderboard:GetSortedAsync(false, 10, 1)
	end)
	if GotData then
		print("Comparing Level Data, and Updating Leaderboard")
		for z, FoundData in ipairs(LevelDataToCompare:GetCurrentPage()) do
			if FoundData ~= nil then
				local PlayerId = (string.sub(FoundData.key, string.len("User-#"), string.len(FoundData.key)))
				local PlayerName = game.Players:GetNameFromUserIdAsync(PlayerId)
				local PlayerLevel = FoundData.value
				Leaderboard[tostring(z)].Player.Text = PlayerName
				Leaderboard[tostring(z)].Level.Text = "Lvl "..PlayerLevel
				Leaderboard[tostring(z)].Level.TextColor3 = Module.GetRankColor(PlayerLevel)
				Leaderboard[tostring(z)].Thumbnail.Image = game.Players:GetUserThumbnailAsync(PlayerId, Enum.ThumbnailType.HeadShot, Enum.ThumbnailSize.Size420x420)
				-- Leaves info for applicable users
			else
				Leaderboard[tostring(z)].Player.Text = "(To Be Decided)"
				Leaderboard[tostring(z)].Level.Text = "Lvl - -"
				Leaderboard[tostring(z)].Level.TextColor3 = ColorByRank.Bronze
				Leaderboard[tostring(z)].Thumbnail.Image = ""
				-- leaves these sections to bet set if the corresponding data doesn't exist
			end
		end
	elseif IssueComparing then
		warn("There was an issue comparing data for the leaderboard, will retry next request!")
	end
end

I run the update function in a loop between players on the bindtoclosed event , and also on the playerremoving event, Updating Data runs once every 2 minutes after initially running on the first players join.

Note: GetKey function just returns “User-”…(The players user id")

Next time put the code improvement topics in the code review section.

Its a good system, however there is an issue. You don’t wrap any of the Async functions in a pcall (protected call)

Async functions (functions ending with Async), usually are recommended to be wrapped around with pcalls because they can frequently error. The Pcall will ensure it cant break the whole script and will allow you do react when it does error.

Edit: Ok I will give you the benefit of the doubt in that you did add it to some.

Consider saving data to Attributes in a folder or configuration instance in the player instead. It makes organization and saving/getting data easier. Having a hundred BaseValues for storing data is unorganized.

And asking if it’s secure doesn’t really make sense to me, datasaving is all on the server.

seems like a pretty secure one!

When I say secure I don’t mean in clients having access, I mean like with data not being lost, since I use setasync which runs way quicker than updateasync, I create a datafolder if all the data necessary loads, if it doesn’t exist the setasync won’t run, theoretically preventing data from being loss if data never loads.

Yeah, I didn’t add a pcall to the saving data, (yet) because if it errors it just won’t save data, it’s in an event so the code won’t break, for now I guess I can just use it to make the error a warning, however for getasync I do use a pcall and it retries 6 times before kicking you out of the game. The call for setasync will only be executed if there is first a folder in the serverstoage data bench folder named after the key. The folder only gets created if the pcall doesn’t fail for getasync. It will also only get created after the last bits of data loads, or if it doesn’t fail or the key is new with no old data!

For the setasync requests I added them to pcalls, will probably make them handle an error in a sort of useful way later on but here for now just so they don’t error on dev console.

local GetDataFolder = game.ServerStorage.DataBench:FindFirstChild(Key)
	if GetDataFolder then
		local CurrentDataSaved, Issue = pcall(function()
			CurrentDataStore:SetAsync(Key, DataTable)
		end)
		if CurrentDataSaved then
			print("Data saved for "..player.Name)
		else
			warn("Issue saving data for "..player.Name)
		end
		local LevelStored, Issue = pcall(function()
			GlobalLeaderboard:SetAsync(Key, DataTable.Level)
		end)
		if LevelStored then
			print("Stored level of "..player.Name.." for comparing")
		else
			warn("Issue storing the level of "..player.Name.." for comparing")
		end
		GetDataFolder:Destroy()
	end