This definitely isn’t a safe data store, a lot can be improved and changed.
- Not waiting for UpdateAsync’s write cooldown per each key to pass will result in a data store throttle:
repeat
success, errormessage = pcall(function()
if not dontWaitForRequest then --I use this so the requestbudget function doesnt yield when a server is shutting down, there is only 30 seconds to save every players data
waitForRequestBudget(Enum.DataStoreRequestType.GetAsync)
end
retries += 1
data = DataStore:UpdateAsync(plr.UserId, function()
return cashValue
end)
end)
if not success then
wait(6) --> THIS IS IMPORTANT
end
until success or retries > 5
You’re also saving data without checking if the data was updated since it was saved / loaded, therefore making the use of UpdateAsync completely useless. This is something you should implement your self.
- Saving data on BindToClose isn’t guaranteed and therefore you should yield until the function has finished saving data for all the players, it won’t stall the shutdown because after 30 seconds, regardless if the function has done it’s job, the server will shutdown forcefully.
game:BindToClose(function()
local saveJobs = #Players:GetPlayers()
local completed = Instance.new("BindableEvent")
for i,v in ipairs(Players:GetPlayers()) do
coroutine.wrap(function()
saveData(v, true)
saveJobs -= 1
end)()
if saveJobs == 0 then
completed:Fire()
end
end
if saveJobs > 0 then
completed:Wait()
end
end)
- Auto saving every 15 seconds is a bit too low, because the code will have high chances of throttling save requests since you’re also retrying each save request if it failed and that is for every player in the game. Consider auto saving every 30 seconds instead.
Also make sure to name unneeded variables as _ and instead change plr to player as it makes more sense and is more readable. Also name all your functions in PascalCase and consider adapting your code style to LuauStyleGuide.
while true do
wait(30)
for _, player in ipairs(Players:GetPlayers()) do
saveData(player)
end
end
- Explicitly defining variables to
nil is completely redundant and you are just generating an extra instruction.
local success, errormessage -- good!
- Loading the data doesn’t account for keys which have no data saved which will end up not saving data for newbies in your game.
-- make sure to check if data is nil and then set it to a default data template
data = DataStore:GetAsync(player.UserId)
Lastly, consider implementing session locking as you will not be guaranteed to have the latest data if the last server takes time / fails to save the data. You’re also not using pcalls correctly, there is no need to call another function which does the actual job.
You also aren’t handling data errors properly, consider reading DataStore Error and Limits.
How I would refactor your code
local Players = game:GetService("Players")
local DataStoreService = game:GetService("DataStoreService")
local DEFAULT_DATA_VALUE = 0
local IS_BIND_TO_CLOSE = false
local DataStore = DataStoreService:GetDataStore("YOUR_DATASTORE_NAME")
local failedToGet = {}
local function WaitForRequestBudget(requestType, requestsNeeded)
local currentBudget = DataStoreService:GetRequestBudgetForRequestType(requestType)
while currentBudget < (requestsNeeded or 1) do
currentBudget = DataStoreService:GetRequestBudgetForRequestType(requestType)
wait(5)
end
end
local function PlayerAdded(player)
local success, errormessage
local retries = 0
repeat
retries += 1
success, errormessage = pcall(function()
WaitForRequestBudget(Enum.DataStoreRequestType.GetAsync)
return DataStore:GetAsync(player.UserId) or DEFAULT_DATA_VALUE
end)
until success or retries >= 5
if retries >= 5 then
failedToGet[player.Name] = true
warn(("Error loading %s's data: %s"):format(player.Name, errormessage))
end
--do stuff with data
end
local function SaveData(player, dontWaitForRequest)
if not failedToGet[player.Name] then
local success, errormessage
local retries = 0
repeat
if not dontWaitForRequest then --I use this so the requestbudget function doesnt yield when a server is shutting down, there is only 30 seconds to save every players data
WaitForRequestBudget(Enum.DataStoreRequestType.GetAsync)
end
success, errormessage = pcall(function()
DataStore:UpdateAsync(player.UserId, function(data)
-- Only save data if it was updated since it was loaded
return data ~= player.leaderstats.Cash.Value and player.leaderstats.Cash.Value or nil
end)
retries += 1
end)
until success or retries >= 5
if retries >= 5 then
warn(("Error saving %s's data: %s"):format(player.Name, errormessage))
end
end
failedToGet[player.Name] = nil
end
Players.PlayerAdded:Connect(PlayerAdded)
Players.PlayerRemoving:Connect(SaveData)
game:BindToClose(function()
IS_BIND_TO_CLOSE = true
for _, player in ipairs(Players:GetPlayers()) do
coroutine.wrap(SaveData)(player, true)
end
end)
for _, player in ipairs(Players:GetPlayers()) do
coroutine.wrap(PlayerAdded)(player) --in case someone joined before these events got connected
end
while not IS_BIND_TO_CLOSE do
wait(30)
for _, player in ipairs(Players:GetPlayers()) do
if IS_BIND_TO_CLOSE then -- dont save data if server is about to shutdown
return
end
coroutine.wrap(SaveData)(player)
end
end
