What are the main things that can be changed for this DataStore script?

Help and Feedback Code Review
#1

What can be changed, upgraded, added, and what are some things that can make this script more efficient?

CODE 

local Players = game:GetService("Players")
local DataStoreService = game:GetService("DataStoreService")
local PlayerDataStore = DataStoreService:GetDataStore("PlayerData")

local defaultData = {
	Level = 1;
	XP = 0;
	MaxXP = 100;
}

Players.PlayerAdded:Connect(function(player)
	
	local data = nil
	local success, err = pcall(function()
		data = PlayerDataStore:GetAsync(player.UserId.."-data")
	end)
	
	if success then
		
		local level = player:WaitForChild("Data"):WaitForChild("Level")
		local xp = level:WaitForChild("XP")
		local maxXP = level:WaitForChild("MaxXP")
		
		if data and data ~= defaultData then
			
			print(player.Name.. " has data! Loading data...")
			
			level.Value = data.Level
			maxXP.Value = data.MaxXP
			xp.Value = data.XP
			
		else
			print(player.Name.. " is a new player! Creating new data...")
			level.Value = defaultData.Level
			maxXP.Value = defaultData.MaxXP
			xp.Value = defaultData.XP
		end
	else
		warn("Unable to grab ".. player.Name.."'s data!")
		warn("Error:", err)
	end
	
end)

Players.PlayerRemoving:Connect(function(player)
	
	local level = player:WaitForChild("Data"):WaitForChild("Level")
	local xp = level:WaitForChild("XP")
	local maxXP = level:WaitForChild("MaxXP")
	
	local data = {
		Level = level.Value;
		XP = xp.Value;
		MaxXP = maxXP.Value;
	}
	
	local success, err = pcall(function()
		PlayerDataStore:UpdateAsync(player.UserId.."-data", function()
			return data
		end)
	end)
	
	if success then
		print(player.Name.. "'s data was saved successfully!")
	else
		warn("Unable to save ".. player.Name.."'s data!")
		warn("Error:", err)
	end
	
end)

My main issue with this script is how I use UpdateAsync() how can I make the saving code better, because right now it just behaves the same as :SetAsync().

#2

UpdateAsync() and SetAysnc() don’t behave the same. One sets a value, whilst the other updates an already existing value.

#3

What I meant was that the code used basically functions the same way as :SetAsync(), but I don’t want to only return a value. I want the script to be more secure than that.

My main issue is that this script isn’t secure enough. I think so anyway…

I really don’t know what to do in the :UpdateAsync() call, all I did was return a value, and I know that you need to do more than just return a value.

What can I do to make this script better…?

#4 
local data = nil

There is no reason to have the data set to nil.

	local success, err = pcall(function()
		data = PlayerDataStore:GetAsync(player.UserId.."-data")
	end)

If there was data but the pcall failed, it won’t try to get the data gain. It’s advisable to use repeat calls. Also, there is no reason to concatenate the player’s UserId with a string, just use the player’s UserId.

local level = player:WaitForChild("Data"):WaitForChild("Level")
local xp = level:WaitForChild("XP")
local maxXP = level:WaitForChild("MaxXP")

These are some bad uses of WaitForChild(), you use WaitForChild on an instance that is inside a folder. ( I believe, correct me If I’m wrong ) Just use WaitForChild() on that folder only. Same goes with xp and MaxXP, once you have used WaitForChild() on level, no need to wait for the instances inside it.

Therefore:

local level = player:WaitForChild("Data"):WaitForChild("Level")
local xp = level.XP
local maxXP = level.MaxXP

if data and data ~= defaultData then

else
	print(player.Name.. " is a new player! Creating new data...")
	level.Value = defaultData.Level
	maxXP.Value = defaultData.MaxXP
	xp.Value = defaultData.XP
end

What if the pcall wasn’t successful but the player had data? This would make the script think it’s a new player and therefore reset it’s values.

else
	warn("Unable to grab ".. player.Name.."'s data!")
	warn("Error:", err)

If data was defaultData, this would still run. Because what if a new player joined, didn’t do anything and just left the game? You need to rethink about how you handle player’s data.

There may be other things to review as well…

#5

I just added in the repeat save / load features.

UPDATED CODE 
local Players = game:GetService("Players")
local DataStoreService = game:GetService("DataStoreService")
local PlayerDataStore = DataStoreService:GetDataStore("PlayerData")

local defaultData = {
	Level = 1;
	XP = 0;
	MaxXP = 100;
}

Players.PlayerAdded:Connect(function(player)
	
	local data = nil
	
	local success, err = nil
	local tries = 0
	
	repeat
		success, err = pcall(function()
			data = PlayerDataStore:GetAsync(player.UserId.."-data")
		end)
		tries += 1
	until success or tries > 3
	
	if success then
		
		local level = player:WaitForChild("Data"):WaitForChild("Level")
		local xp = level:WaitForChild("XP")
		local maxXP = level:WaitForChild("MaxXP")
		
		if data and data ~= defaultData then
			
			print(player.Name.. " has data! Loading data...")
			
			level.Value = data.Level
			maxXP.Value = data.MaxXP
			xp.Value = data.XP
			
		else
			print(player.Name.. " is a new player! Creating new data...")
			level.Value = defaultData.Level
			maxXP.Value = defaultData.MaxXP
			xp.Value = defaultData.XP
		end
	else
		warn("Unable to grab ".. player.Name.."'s data!")
		warn("Error:", err)
	end
	
end)

Players.PlayerRemoving:Connect(function(player)
	
	local level = player:WaitForChild("Data"):WaitForChild("Level")
	local xp = level.XP
	local maxXP = level.MaxXP
	
	local data = {
		Level = level.Value;
		XP = xp.Value;
		MaxXP = maxXP.Value;
	}
	
	local success, err = nil
	local tries = 0
	
	repeat
		success, err = pcall(function()
			PlayerDataStore:UpdateAsync(player.UserId.."-data", function()
				return data
			end)
		end)
		tries += 1
	until success or tries > 3
	
	if success then
		print(player.Name.. "'s data was saved successfully!")
	else
		warn("Unable to save ".. player.Name.."'s data!")
		warn("Error:", err)
	end
	
end)

game:BindToClose(function()
	for _, player in pairs(Players:GetPlayers()) do
		
		coroutine.wrap(function()
			
			local level = player:WaitForChild("Data"):WaitForChild("Level")
			local xp = level.XP
			local maxXP = level.MaxXP
			
			local data = {
				Level = level.Value;
				XP = xp.Value;
				MaxXP = maxXP.Value;
			}
			
			local success, err = nil
			local tries = 0
			
			repeat
				success, err = pcall(function()
					PlayerDataStore:UpdateAsync(player.UserId.."-data", function()
						return data
					end)
				end)
				tries += 1
			until success or tries > 3
			
			if success then
				print(player.Name.. "'s data was saved successfully!")
			else
				warn("Unable to save ".. player.Name.."'s data!")
				warn("Error:", err)
			end
			
		end)()
		
	end
end)