There’s no evidence of debugging or if the console is throwing anything so actually knowing what your problem is here is impossible. Please do some debugging here and make sure your Guis are set up properly, otherwise we can’t really discern any causes of error.
From the client’s end, use some variables to avoid redundancy and don’t let your code assume that instances exist. It would also be more preferable to have your LocalScript directly descend the PlayerGui rather than a nested element because it becomes worse off to work with.
In either case, assuming you’re still working with your current setup:
**Note: Changed this code sample. It now fires with initial state.
local LocalPlayer = game:GetService("Players").LocalPlayer
-- Because you don't parent Coins before leaderstats, Coins is not guaranteed to
-- be replicated with the folder, so now you have to unnecessarily use two waits.
local leaderstats = LocalPlayer:WaitForChild("leaderstats")
local coins = leaderstats:WaitForChild("Coins")
-- Create a function so we can avoid redundancy later
local function updateCurrencyUI(newValue)
script.Parent.Text = tostring(newValue) .. "$"
end
-- Changed on ValueObjects will fire with the new value, which will be
-- newValue in the function.
coins.Changed:Connect(updateCurrencyUI)
-- Run it for the first time so we can fire with initial state.
updateCurrencyUI()
From the server’s end, there are various improvements to be made.
There’s a thread you may or may not be aware of, to not use the parent argument of Instance.new. While you don’t do that here, you are still setting the properties of objects after parenting them, which still invites that performance loss. You ideally only want to parent when everything is set up.
With the way you’re using the pcall, you aren’t utilising the full benefits of it. A pcall is meant to run a function in protected mode and catch any errors thrown by it. It returns two values: whether the function successfully ran or not and any returned values. You should make good use of those values as if there is a problem (e.g. DataStore outage), not only can you handle that case but report on it as well so you’re actually aware there is a problem.
Continuing on with the above, if the pcall throws an error, it will return an error message. As you do not check for the success value, the code below will throw an error because you’ll be attempting to set the value of a ValueObject to a string when it is expecting a number. You need to handle that case. Just because they don’t return anything, doesn’t mean they’re new players.
Incorporating all this feedback together, here’s what you can expect your code to look like:
local Players = game:GetService("Players")
local DataStoreService = game:GetService("DataStoreService")
local DataStore = DataStoreService:GetDataStore("TestDataStore")
local CURRENCY_NAME = "Coins"
-- It would be good to incorporate retrying as well, or a failure redundancy
-- method that calls UpdateAsync over SetAsync for failed saves.
local dataErrorFor = table.create(Players.MaxPlayers)
local function playerAdded(player)
local playerKey = CURRENCY_NAME .. player.UserId
local folder = Instance.new("Folder")
folder.Name = "leaderstats"
local currency = Instance.new("IntValue")
currency.Name = CURRENCY_NAME
local success, ret = pcall(function ()
return DataStore:GetAsync(playerKey)
end)
if success then
print("DataStore fetched successfully")
if ret then
print("Data found, setting to value")
currency.Value = ret
else
warn("No data in successful fetch, new data created")
currency.Value = 1000
end
else
warn("Unsuccessful fetch for userId=" .. player.UserId)
dataErrorFor[player.UserId] = true
currency.Value = 1000
end
currency.Parent = folder
folder.Parent = player
end
local function playerRemoving(player)
if not dataErrorFor[player.UserId] then
local playerKey = CURRENCY_NAME .. "-" .. player.UserId
local currency = player.leaderstats[CURRENCY_NAME]
DataStore:SetAsync(playerKey, currency.Value)
else
-- To avoid data loss and overwriting with false data, we do not save
-- for players who had a fetch error. Look to incorporating a
-- retry or rebounding system.
warn("Data not saved for userId=" .. player.UserId)
end
-- Ensure player key gets removed from our tracker table
dataErrorFor[player.UserId] = nil
end
-- Consider writing a saving function to call on all players instead, kicking
-- them doesn't guarantee PlayerRemoving will fire. I'll leave this unchanged.
local function onClose()
for _, player in ipairs(Players:GetPlayers()) do
player:Kick("Game update")
end
-- IIRC wait will not stall server closure because at this point, there are
-- no other scheduled tasks, meaning the server can now close.
wait(5)
end
Players.PlayerAdded:Connect(playerAdded)
Players.PlayerRemoving:Connect(playerRemoving)
game:BindToClose(onClose)
for _, player in ipairs(Players:GetPlayers()) do
playerAdded(player)
end
