Now it works! Studio didn’t save my changes to the Script, lol… However does my change to the script look good? Would it interfere with anything? It seems to work perfect now.
Alright, so to start off, I’m seeing many different problems with your current setup. First of all, when creating a loop to auto save data, you should probably use a pcall and have checks to make sure that you aren’t overwriting their data if they haven’t loaded in.
local function autoSave()
while wait(60) do
if player and player:FindFirstChild("DataLoaded") then --Make sure that the player's data is loaded
--Save data
else
break
end
end
end
spawn(autoSave)
Second of all, I recommend using tables rather than values for many reasons like, better organization and more functionality.
Interesting, I just tried implementing this check for DataLoaded, but it won’t save. Also, using tables to store the stats seems interesting, how is this being done? (with minimal change to the code I already have)
Current code so far:
local DataStoreService = game:GetService("DataStoreService")
local stats = DataStoreService:GetDataStore("Stats")
local function attemptRequest(request, retries)
local retries = retries or 0;
local success, data = pcall(function()
return request();
end);
if (success) then
return data;
elseif (not success and retries ~= 0) then
return attemptRequest(request, retries - 1);
else
error('Unable to get requested data!');
end
end
game.Players.PlayerAdded:Connect(function(player)
-- Get saves:
local key = "player-" .. player.userId
local savedPlayerStats = attemptRequest(function()
return stats:GetAsync(key);
end, 5); -- Retries 5 times maximum.
local leaderstatsFolder = Instance.new("Folder",player)
leaderstatsFolder.Name = "leaderstats"
local tokens, points;
if (savedPlayerStats) then -- Player has been here before:
tokens = Instance.new("NumberValue",leaderstatsFolder)
tokens.Name = "Tokens"
points = Instance.new("NumberValue",leaderstatsFolder)
points.Name = "Points"
points.Value = savedPlayerStats[1]
tokens.Value = savedPlayerStats[2]
else -- New player:
tokens = Instance.new("NumberValue",leaderstatsFolder)
tokens.Name = "Tokens"
points = Instance.new("NumberValue",leaderstatsFolder)
points.Name = "Points"
points.Value = 0
tokens.Value = 0
end
while (player.Parent) do
if player:FindFirstChild("DataLoaded") then
-- Saving:
local playerStatsToSave = {points.Value, tokens.Value}
local success = attemptRequest(function()
return stats:SetAsync(key, playerStatsToSave)
end, 0) -- Doesn't retry.
if success then
print('Successfully saved stats for', player.Name);
else
print('Failed to save stats for', player.Name);
end
end
wait(60*5)
end
end)
game.Players.PlayerRemoving:Connect(function(player)
-- MAIN STATS:
local points= player:WaitForChild("leaderstats"):FindFirstChild('Points')
local tokens = player:WaitForChild("leaderstats"):FindFirstChild('Tokens')
-- Saving:
if (points and tokens) then
local key = "player-" .. player.userId
local playerStatsToSave = {points.Value, tokens.Value}
local success, err = attemptRequest(function()
return stats:SetAsync(key, playerStatsToSave)
end, 2) -- Retries twice.
if success then
print('Successfully saved stats for', player.Name, 'upon leaving.');
else
warn('Failed to save stats for', player.Name, 'upon leaving.')
end
end
end)
Alright, this is a good start. First of all, you shouldn’t check the player’s parent as this could error if the player is nil, you can just do while player do. Now, with DataLoaded, I meant you have to create a value and insert it into the player.
local loaded = Instance.new("StringValue")
loaded.Parent = player
--autosave
Tables can really help with organization and functionality, which you can do like so (customize it to your needs)
local data = {}
local function playerAdded(player)
local success, response = pcall(function()
return dataStore:GetAsync("player-"..player.UserId)
end)
if success then --If it's a success
local playerData = response or {} --If there's not any data, then create a new table.
local currency = Instance.new("NumberValue")
currency.Name = "Currency"
currency.Value = playerData.currency or 0 --If there's currency then use that. Else use default, 0
currency.Parent = player
local loaded = Instance.new("StringValue")
loaded.Name = "DataLoaded"
loaded.Parent = player
data[player.UserId] = playerData --store the data in a table
spawn(function()
while wait(60) do
if player and player:FindFirstChild("DataLoaded") then
save(player)
end
end
end)
end
end
function save(player)
if player:FindFirstChild("DataLoaded") then
local playerData = data[player.UserId]
playerData.currency = playerData.currency or 0 --set the value of the currency for next time if data does not already exist
local success, response = pcall(function()
return dataStore:SetAsync("player-"..player.UserId, playerData) --save the table
end)
end
end
If you’d like to learn more about tables, you can go here for documentation on tables and here for how to use tables in data stores.
Please use the condition of the while loop properly.
local dataReady = {} -- Example; format is [UserId] = true
local function autoSave()
-- Don't create an instance for this, use a table @ original implementation
while player and dataReady[player.UserId] == true then
-- Save data
end
end
spawn(autoSave)
Using wait to implicitly pass the condition of a while loop is bad code. Check out my resource thread, The While-Wait-Do Idiom for more information.
As for your second response, you’re contradicting yourself a little here.
Your first post mentioned that ValueObjects should be avoided, but your second response is very ValueObject heavy. I’m not sure whether you’re posting a modification to OP’s current code or whether that’s something you came up with as you were replying.
A little someone once upon a time pointed out to me (though I was only half-aware at the time) that arrays are faster than relying on ValueObjects. In that respect, you can also have all your code directly in your code environment rather than relying on external values. I would definitely recommend a table over ValueObjects for the sake of simplicity, organisation and control.
Moving over to your code, a little protip to wrap the actual method calls whenever possible over creating an anonymous function and wrapping that.
-- You could just do tostring(player.UserId). Kept old key for consistency's sake.
local success, response = pcall(dataStore.GetAsync, dataStore, "player-"..player.UserId)
Also, playerData never gets any entries put into it…? This code seems more reliant on the ValueObjects than the table.
Thank you for correcting me. You went a little overboard there with all the minor mistakes though. I was a bit rushed to create that code. Also, I was trying to show him both ways, with a table and how he was using the value objects. I was explaining in his current code that I had replied to that he needed to create a value object for that code to work and that if he wanted to use values, that was how he could utilize it. One of the reasons I like to use a value object to say that the data is loaded is because it can replicate to the client and server that the data is loaded. There is no significant deficiencies in using a value object to show that the data is loaded. However, I do like to use tables to organize the player’s data, like I said in my previous posts. But, like I said, tables are probably the most efficient and easy to organize.
I don’t think it was overboard. You would be surprised to come to understand how these minor changes have a relatively major impact on your game’s code and developing good practice in the future, whether for Roblox or other programming languages.
This is fairly ambiguous in your code though, because the table is never actually used and it looks more dependent on the ValueObjects. That’s why I raised this.
The code sample I supplied eliminates ValueObjects (well, only really at the DataReady part) and gives you a quick, easy and efficient way to access information.
A table is far more efficient for this use case. You can also access it via a local variable or exposed function without depending on the existence of an instance.
As for what you said about ValueObjects not supposedly causing deficiencies, yes it does. You have to rely on several factors regarding this object, including where it’s located in the hierarchy and you need to ensure it exists in the location you need.
Furthermore, with the above in mind, keep in mind that ValueObjects are instances. Instances incur a cost for all the process they invoke or participate in initially, as well as any done afterward. A table scraps the expense of an instance entirely.
I’m just curious - why does this part of my code not work? The saving doesn’t work, it keeps printing “Failed to save stats for skilpus3000”
while game.Players:FindFirstChild(player.Name) ~= nil do
-- Saving:
local playerStatsToSave = {rescues.Value, tokens.Value}
local success = attemptRequest(function()
return stats:SetAsync(key, playerStatsToSave)
end, 0) -- Doesn't retry.
if success then
print('Successfully saved stats for', player.Name);
else
print('Failed to save stats for', player.Name);
end
wait(10)
end
wrap the autosaving with a new thread, this isnt the most optimized way, but it sure does work.
remember to wrap the :SetAsync() with a pcall, or xpcall.
you can do major optimizations to this script, like changing from making new threads into other ways.
like:
while wait(60*5) do
for _,v in pairs(game:GetService("Players"):GetChildren()) do
local playerStatsToSave = {v.leaderstats.Points.Value, v.leaderstats.Tokens.Value}
stats:SetAsync("player-" .. v.UserId, playerStatsToSave) -- remember to wrap this in a pcall or xpcall.
end
end
this is a possible solution, not the best solution but it would work.
This means that a truth-y value is not being returned by the method attemptRequest. Keep in mind that SetAsync has no return value. Could you post the code behind attemptRequest?
The purpose of wrapping the autosave in a new thread is to effectively make it a background process that won’t interfere with the execution of other threads. Keep in mind that nothing after a while loop runs until the loop is broken either by meeting it’s condition or explicit usage of the break operator.
You didn’t include a pcall despite saying that it should be wrapped. You are correct though. I’m not sure why xpcall is applicable here though (I don’t know what it is either), a regular pcall is fine for this case.
Lastly, like I told Faultsified, please use the condition of a while loop properly. Don’t implicitly pass it by using wait. That’s bad code. Don’t rely on wait to return a truth-y value.
I can provide the whole Script for easier understanding, but this is your version I implemented
local DataStoreService = game:GetService("DataStoreService")
local stats = DataStoreService:GetDataStore("Stats")
local function attemptRequest(request, retries)
local retries = retries or 0;
local success, data = pcall(function()
return request();
end);
if (success) then
return data;
elseif (not success and retries ~= 0) then
return attemptRequest(request, retries - 1);
else
error('Unable to get requested data!');
end
end
game.Players.PlayerAdded:Connect(function(player)
-- Get saves:
local key = "player-" .. player.userId
local savedPlayerStats = attemptRequest(function()
return stats:GetAsync(key);
end, 5); -- Retries 5 times maximum.
local leaderstatsFolder = Instance.new("Folder",player)
leaderstatsFolder.Name = "leaderstats"
local tokens, points;
if (savedPlayerStats) then -- Player has been here before:
tokens = Instance.new("NumberValue",leaderstatsFolder)
tokens.Name = "Tokens"
points= Instance.new("NumberValue",leaderstatsFolder)
points.Name = "Points"
points.Value = savedPlayerStats[1]
tokens.Value = savedPlayerStats[2]
else -- New player:
tokens = Instance.new("NumberValue",leaderstatsFolder)
tokens.Name = "Tokens"
points= Instance.new("NumberValue",leaderstatsFolder)
points.Name = "Points"
points.Value = 0
tokens.Value = 0
end
while game.Players:FindFirstChild(player.Name) ~= nil do
-- Saving:
local playerStatsToSave = {rescues.Value, tokens.Value}
local success = attemptRequest(function()
return stats:SetAsync(key, playerStatsToSave)
end, 0) -- Doesn't retry.
if success then
print('Successfully saved stats for', player.Name);
else
print('Failed to save stats for', player.Name);
end
wait(10)
end
end)
game.Players.PlayerRemoving:Connect(function(player)
-- MAIN STATS:
local points= player:WaitForChild("leaderstats"):FindFirstChild('Points')
local tokens = player:WaitForChild("leaderstats"):FindFirstChild('Tokens')
-- Saving:
if (points and tokens) then
local key = "player-" .. player.userId
local playerStatsToSave = {points.Value, tokens.Value}
local success, err = attemptRequest(function()
return stats:SetAsync(key, playerStatsToSave)
end, 2) -- Retries twice.
if success then
print('Successfully saved stats for', player.Name, 'upon leaving.');
else
warn('Failed to save stats for', player.Name, 'upon leaving.')
end
end
end)
A xpcall, is basically pcall, but with the functionallity to get the error message and format it, like this:
local suc, res = xpcall(function()
x("Hello World!") -- this will error
end, function(err)
error("The example errored! Full Error: " .. err) -- would print something like attempted to index x, a nil value
end)
I didnt think about the idiom, I just slapped that code together fast.
most code I make, I have the while-wait idiom as a rule of thumb, I never rely on wait() in loops, I’ve stumbled across multiple times that it breaks.
How exactly would that look like? I am all new to pcalls and I don’t have too much experience with DataStore, Im still trying to wrap my mind around how this works, lol
Sorry for the late response! It seems I had completely forgotten about this.
What you can do is send back the return values from the pcall that wraps the request function.
local function attemptRequest(request, retries)
local retries = retries or 0
local completed = false
local response
repeat
retries = retries - 1
local success, reply = pcall(request)
if success then
completed = success
response = reply
else
wait(1) -- Give time between a request if it doesn't succeed
end
until completed or retries <= 0
return completed, response
end
With this new function, you can use your new return values within your code to handle what should happen when a requested attempt fails.
For example, in your PlayerAdded function, your script may change to something like this:
local Players = game:GetService("Players") -- Canonical way to get a service
Players.PlayerAdded:Connect(function (player)
-- Reminder that userId is deprecated in favour of UserId
local key = string.format("player-%d", player.UserId) -- Being fancy?
local success, savedPlayerStats = attemptRequest(function ()
return stats:GetAsync(key)
end, 5) -- Would recommend a lower value to preserve your request budget
if success then
-- Request was successful
if savedPlayerStats then
-- Player has stats saved
else
-- A non-truth-y value was returned (no stats)
end
end
-- Your other code
end)
But thanks!