Issue with XP not saving

Help and Feedback Scripting Support
#1

I’m having an issue where peoples XP won’t save after you rejoin. Since this was added in a newer update, it saves peoples XP from the last time they played since the update. I added tools to the recent update (Lift, Lift2, Lift3) and ever since that update it hasn’t been saving. If anyone could possible help me fix this issue with it saving. If you need more information about the issue I’m here to provide it for you.

leaderstats script:

local Players = game:GetService("Players")
local serverStorage = game:GetService("ServerStorage")
local DataStoreService = game:GetService("DataStoreService")
local SaveDataStore = DataStoreService:GetDataStore("SaveData")
local replicatedStorage = game:GetService("ReplicatedStorage")
local remoteData = game:GetService("ServerStorage"):WaitForChild("RemoteData")
local cooldown = 1

local function SavePlayerData(player)
	
	local success, errormsg = pcall(function()
		local SaveData = {}
		for i, stats in pairs(player.leaderstats:GetChildren()) do
			SaveData[stats.Name] = stats.Value
		end	
		SaveDataStore:UpdateAsync(player.UserId, SaveData)
	end)
	
	if not success then 
		return errormsg
	end	
end

game.Players.PlayerAdded:Connect(function(player)
	
	local leaderstats = Instance.new("Folder", player)
    leaderstats.Name = "leaderstats"
	leaderstats.Parent = player
	
	local level = Instance.new("IntValue")
	level.Name = "Level"
	level.Value = 0
	level.Parent = leaderstats
	
	local XP = Instance.new("IntValue")
	XP.Name = "XP"
	XP.Value = 0
	XP.Parent = leaderstats
	
    local Deaths = Instance.new("IntValue", leaderstats)
    Deaths.Name = "Deaths"
    Deaths.Value = 0
    Deaths.Parent = leaderstats

	local dataFolder = Instance.new("Folder")
	dataFolder.Name = player.Name
	dataFolder.Parent = serverStorage.RemoteData
	
	local debounce = Instance.new("BoolValue")
	debounce.Name = "Debounce"
	debounce.Parent = dataFolder
	
	
local Data = SaveDataStore:GetAsync(player.UserId)
	
	if Data then
		for i, stats in pairs(leaderstats:GetChildren()) do
			stats.Value = Data[stats.Name]
		end		
	else		
		print(player.Name .. " has no data.")			
	end
	
	local expToLevelUp
	local expForPreviousLevel = 0
	
	player.CharacterAdded:Connect(function(char)
      char.Humanoid.Died:Connect(function()
	     Deaths.Value = Deaths.Value + 1
	
      end) 
	end)
		
replicatedStorage.Remotes.Lift.OnServerEvent:Connect(function(player)
	
	if not remoteData:FindFirstChild(player.Name) then return "NoFolder" end
	
	local debounce = remoteData[player.Name].Debounce

	if not debounce.Value then
		
		debounce.Value = true

		player.leaderstats.XP.Value = player.leaderstats.XP.Value + 170

		wait(cooldown)
		
		debounce.Value = false
				
	end
					
end)
	
replicatedStorage.Remotes.Lift.OnServerEvent:Connect(function(player)
	
	if not remoteData:FindFirstChild(player.Name) then return "NoFolder" end
	
	local debounce = remoteData[player.Name].Debounce

	if not debounce.Value then
		
		debounce.Value = true

		player.leaderstats.XP.Value = player.leaderstats.XP.Value + 170

		wait(cooldown)
		
		debounce.Value = false
		
	end
					
end)

local cooldown = 1

replicatedStorage.Remotes.Lift2.OnServerEvent:Connect(function(player)
	
	if not remoteData:FindFirstChild(player.Name) then return "NoFolder" end
	
	local debounce = remoteData[player.Name].Debounce

	if not debounce.Value then
		
		debounce.Value = true

		player.leaderstats.XP.Value = player.leaderstats.XP.Value + 80

		wait(cooldown)
		
		debounce.Value = false
		
	end
					
end)

local cooldown = 1

replicatedStorage.Remotes.Lift3.OnServerEvent:Connect(function(player)
	
	if not remoteData:FindFirstChild(player.Name) then return "NoFolder" end
	
	local debounce = remoteData[player.Name].Debounce

	if not debounce.Value then
		
		debounce.Value = true

		player.leaderstats.XP.Value = player.leaderstats.XP.Value + 320

		wait(cooldown)
		
		debounce.Value = false
		
	end
					
end)

local cooldown = 1

replicatedStorage.Remotes.Lift4.OnServerEvent:Connect(function(player)
	
	if not remoteData:FindFirstChild(player.Name) then return "NoFolder" end
	
	local debounce = remoteData[player.Name].Debounce

	if not debounce.Value then
		
		debounce.Value = true

		player.leaderstats.XP.Value = player.leaderstats.XP.Value + 230

		wait(cooldown)
		
		debounce.Value = false
		
	end
					
end)

local cooldown = 1

replicatedStorage.Remotes.Lift5.OnServerEvent:Connect(function(player)
	
	if not remoteData:FindFirstChild(player.Name) then return "NoFolder" end
	
	local debounce = remoteData[player.Name].Debounce

	if not debounce.Value then
		
		debounce.Value = true

		player.leaderstats.XP.Value = player.leaderstats.XP.Value + 50

		wait(cooldown)
		
		debounce.Value = false
		
	end
					
end)

local cooldown = 1

replicatedStorage.Remotes.Lift6.OnServerEvent:Connect(function(player)
	
	if not remoteData:FindFirstChild(player.Name) then return "NoFolder" end
	
	local debounce = remoteData[player.Name].Debounce

	if not debounce.Value then
		
		debounce.Value = true

		player.leaderstats.XP.Value = player.leaderstats.XP.Value + 160

		wait(cooldown)
		
		debounce.Value = false
		
	end
					
end)

local cooldown = 1

replicatedStorage.Remotes.Lift7.OnServerEvent:Connect(function(player)
	
	if not remoteData:FindFirstChild(player.Name) then return "NoFolder" end
	
	local debounce = remoteData[player.Name].Debounce

	if not debounce.Value then
		
		debounce.Value = true

		player.leaderstats.XP.Value = player.leaderstats.XP.Value + 120

		wait(cooldown)
		
		debounce.Value = false
		
	end
					
end)

local cooldown = 1

replicatedStorage.Remotes.Lift8.OnServerEvent:Connect(function(player)
	
	if not remoteData:FindFirstChild(player.Name) then return "NoFolder" end
	
	local debounce = remoteData[player.Name].Debounce

	if not debounce.Value then
		
		debounce.Value = true

		player.leaderstats.XP.Value = player.leaderstats.XP.Value + 130

		wait(cooldown)
		
		debounce.Value = false
		
	end
					
end)

local cooldown = 1

replicatedStorage.Remotes.Lift9.OnServerEvent:Connect(function(player)
	
	if not remoteData:FindFirstChild(player.Name) then return "NoFolder" end
	
	local debounce = remoteData[player.Name].Debounce

	if not debounce.Value then
		
		debounce.Value = true

		player.leaderstats.XP.Value = player.leaderstats.XP.Value + 90

		wait(cooldown)
		
		debounce.Value = false
		
	end
					
end)

local cooldown = 1

replicatedStorage.Remotes.Lift10.OnServerEvent:Connect(function(player)
	
	if not remoteData:FindFirstChild(player.Name) then return "NoFolder" end
	
	local debounce = remoteData[player.Name].Debounce

	if not debounce.Value then
		
		debounce.Value = true

		player.leaderstats.XP.Value = player.leaderstats.XP.Value + 50

		wait(cooldown)
		
		debounce.Value = false
		
	end
					
end)

local cooldown = 1

replicatedStorage.Remotes.Lift11.OnServerEvent:Connect(function(player)
	
	if not remoteData:FindFirstChild(player.Name) then return "NoFolder" end
	
	local debounce = remoteData[player.Name].Debounce

	if not debounce.Value then
		
		debounce.Value = true

		player.leaderstats.XP.Value = player.leaderstats.XP.Value + 240

		wait(cooldown)
		
		debounce.Value = false
		
	end
					
end)

local cooldown = 1

replicatedStorage.Remotes.Lift12.OnServerEvent:Connect(function(player)
	
	if not remoteData:FindFirstChild(player.Name) then return "NoFolder" end
	
	local debounce = remoteData[player.Name].Debounce

	if not debounce.Value then
		
		debounce.Value = true

		player.leaderstats.XP.Value = player.leaderstats.XP.Value + 60

		wait(cooldown)
		
		debounce.Value = false
		
	end
					
end)
	
	while wait() do
		
		local levelBar = player.PlayerGui:WaitForChild("LevelBar")	
		if level.Value < 1 then 
			
			expToLevelUp = 100 
		else
			expToLevelUp = math.floor(level.Value ^ 1.3) * 200 + math.floor(level.Value ^ 4)
		end
		
		if XP.Value >= expToLevelUp then
			level.Value = level.Value + 1	
		end
		
		expForPreviousLevel = math.floor((level.Value - 1) ^ 1.3) * 200 + math.floor((level.Value - 1) ^ 4)
		
		local expDifference = expToLevelUp - expForPreviousLevel
		local expDifference2 = XP.Value - expForPreviousLevel
			
		levelBar.Bar:TweenSize(UDim2.new(levelBar.BarBackground.Size.X.Scale * (expDifference2 / expDifference), 0, levelBar.BarBackground.Size.Y.Scale, 0), Enum.EasingDirection.InOut, Enum.EasingStyle.Quint, 0.001)
		levelBar.Experience.Text = expDifference2 .. "/" .. expDifference
		levelBar.Level.Text = "Level: " .. level.Value
		
		XP.Value = XP.Value + 1
	
	end
end)
	

Players.PlayerRemoving:Connect(function(player)
	
	local errormsg = SavePlayerData(player)
	
	if errormsg then	
		warn(errormsg)		
	end
end)

game:BindToClose(function()
	
	for i, player in pairs(Players:GetPlayers()) do	
		
		local errormsg = SavePlayerData(player)
		if errormsg then
			warn(errormsg)
		end
	end
	wait(2)	
end)
2 Likes
#2

Just a nitpick, but - Are you letting clients raise their own XP?

#3

From the looks of it, yes, they do.

@MegaTheMagnificent You should try raising the XP server-sided, rather than trust the client. I understand if you find it more ‘easy’ to do so as the player, but using the server will be more safe. Also, UpdateAsync requires a function, rather than a save location, which is why it errors. You can read more about it here.

1 Like
#4

Yeah there’s several glaring issues, among nitpicks, with your code.

No variable organisation

There exists a lack of variable organisation. Services are not together and it generally looks messy. I recommend separating your top-level variables by services, objects then non-constant variables. Readability is a very important factor in coding and sometimes can even help you see where exactly you’re going wrong.

Script yields at the start

You have a yielding call at the top of your script and later have a function listen to PlayerAdded. First off, WaitForChild is not required for the server because instances will implicitly exist and it’s only useful if you’re unsure of if something will exist and you need it to exist. Second off, this will delay when the PlayerAdded function gets hooked, thus not fire for all players. Account for this case by creating a named function and connecting it later as well as removing WaitForChild.

Wrong use of pcall

The only thing that should be present in your pcall is the DataStore call. Everything else is unnecessary and should be done outside of the call.

Wrong use of UpdateAsync

You need to check your console when something is bugged, confirm that you are using API correctly and do your own debugging before posting to the forums. UpdateAsync only accepts a function value with the current saved data passed as a parameter. Either fix how you use UpdateAsync or change this to SetAsync.

No ipairs on an array

It’s idiomatic to use ipairs to iterate through an array. This is in reference to collecting the values in leaderstats and putting them into a table. pairs is for dictionaries. I recommend switching to ipairs so it’s more clear what you’re iterating over. You don’t need to know they key. This also allows you to iterate in sequential order.

Using the parent argument of Instance.new

You’ve introduced a performance issue into your code. Depending on how your systems are architectured as well, you could be rooting other issues. Don’t use the second argument of Instance.new. Set the properties of all your stats first, parent them to the leaderstats folder, then parent the leaderstats folder to the player. This ensures that the children exist as in when leaderstats is added and all the values are replicated as one set, not separate sets.

Inconsistent presence of pcall

You have some DataStore calls that are not wrapped in pcall. You must wrap them in pcall so that you’ll be able to account for any errors that happen during the process. Especially now when Roblox servers are experiencing frequent outages due to platform iteration, it’s important that you don’t allow your players to lose data. That will not be good for your game.

OnServerEvent is a signal, not a function

Your guard clauses in your RemoteEvents return a value. This value will be discarded. OnServerEvent is not a callback, it’s an RBXScriptSignal and the function connected to it is housed in an RBXScriptConnection.

Gross amount of redundancy and lack of security

You have 8 remotes that do the exact same thing, except that only certain values are changed. There is also a lack of any security on these remotes which means exploiters can continually fire all 8 remotes and massively inflate their stats. Redundant code can be consolidated and you can apply other strategies for the server to determine how much EXP to give and if a client is actually clear to be firing an event, as well as applying cooldowns for a certain tool instead of a cooldown that’s global across all tools.

Overwriting declarations

You have several instances of local cooldown. Only one of them is actually going to be able to be used in this case and that’ll affect all your remotes. This can be resolved with the application of the techniques in the above block. Don’t use a global cooldown, do it on a per-tool basis.

PRIMARY CAUSE
While wait loop blocks PlayerRemoving

First of all, stop using while wait. Second of all, this while loop is the PRIMARY CAUSE of your data not saving. When a while loop is running, it will not execute any code after it until it has finished. In the case of a non-terminating loop, the code below it never gets reached. So, now, PlayerRemoving and BindToClose become nulled chunks of code.

Second of all, I noticed exactly what the while loop is doing. You need to get rid of it, period. The server should not be charged with managing a player’s Gui, that is something you leave to the client. You also do not use loops here: loops where unnecessary are evil. Adopt an event-based structure so you aren’t needlessly running code when it doesn’t need to be running. Have the client connect to a changed event on EXP and THEN, only THEN update the bar FROM THE CLIENT.

That’s all. You have a lot of refactoring to do, mate.

4 Likes