AutoSave module

Help and Feedback Code Review
#1

Hi! I’m making an AutoSave module, can someone review the code please?

local Players = game:GetService("Players")
local DataStoreService = game:GetService("DataStoreService")
local DataStore = DataStoreService:GetDataStore("DataStore", 1)
local AutoSave = {}
local shouldSave = {}

function AutoSave.SetNewIndex(index)
	DataStore = DataStoreService:GetDataStore("DataStore", index)
end

function AutoSave.New(object)
	shouldSave[object.Name] = object
end


Players.PlayerAdded:Connect(function(plr)
	local saved
	repeat
		local success, errormessage = pcall(function()
			saved = DataStore:GetAsync(plr.UserId)
		end)
	until success and not errormessage

	if saved then
		for name, value in pairs(saved) do
			local object = shouldSave[name]
			object.Value = value
		end
	end
end)


Players.PlayerRemoving:Connect(function(plr)
	local userId = plr.UserId
	local saveToPlayer = {} --this table will contain everything that should be saved
	--ex. : saveToPlayer = {["Cash"] = 300}
	for name, object in pairs(shouldSave) do
		saveToPlayer[name] = object.Value
	end
	repeat
		local success, errormessage = pcall(function()	
			DataStore:SetAsync(userId, saveToPlayer)
		end)
	until success and not errormessage
end)

game:BindToClose(function()
	for _, plr in ipairs(Players:GetPlayers()) do
		coroutine.wrap(function()
			local userId = plr.UserId
			local saveToPlayer = {}
			for name, parent in pairs(shouldSave) do
				saveToPlayer[name] = parent[name].Value
			end
			repeat
				local success, errormessage = pcall(function()	
					DataStore:SetAsync(userId, saveToPlayer)
				end)
			until success and not errormessage
		end)()
	end
end)

return AutoSave

If the player joins and leaves way too quickly, the value might be 0 and then his data is lost?
Thanks!

3 Likes
#3

Isn’t he already using UserID?

Anyways, really nice data implementation

1 Like
#4

Shoot ye he is, my bad lol. Did not see the above line properly.

1 Like
#5 
repeat
		local success, errormessage = pcall(function()
			saved = DataStore:GetAsync(plr.UserId)
		end)
	until success and not errormessage

repeat
		local success, errormessage = pcall(function()	
			DataStore:SetAsync(userId, saveToPlayer)
		end)
	until success and not errormessage

I wouldn’t do this if I were you. I think that the max tries that you can do with data store is 3 or else it would break. So I would do this:

local MaxTries = 3

repeat
        local Tries = 0

		local success, errormessage = pcall(function()
			saved = DataStore:GetAsync(plr.UserId)
		end)

        Tries = Tries +1
	until Tries >= MaxTries or success

local MaxTries = 3

repeat
        local Tries = 0

		local success, errormessage = pcall(function()	
			DataStore:SetAsync(userId, saveToPlayer)
		end)

        Tries = Tries +1
	until Tries >= MaxTries or success

I hope this helps.

3 Likes
#6

I’m pretty sure 3 isn’t the max tries you can do, it might be that after 3 that occur after each other in a short time it puts the calls in a queue (that warn-message will appear in the output) but that should be fine.

#7

In nearly every scenario, using a loop with DataStores to brute force a request to go through is a terrible idea and unnecessary waste of requests; it is also problematic in the case of a real outage because you aren’t properly handling an error (which GetAsync does not throw unless there’s a throttle, outage or unexpected corruption).

What you should be doing is using GetRequestBudgetForRequestType if you aren’t sure whether the game server has sufficient budget to perform a request or not. If there’s enough requests left in the current minute, then you may proceed with your request.

Please also make sure that you aren’t using anonymous functions. Any code that’s subject to yielding prior to the connection will result in players potentially joining before the connection and will not have any kind of DataStore code ran on them.

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)
    waitForRequestBudget(Enum.DataStoreRequestType.GetAsync)

    local success, result = pcall(function ()
        return DataStore:GetAsync(player.UserId)
    end

    if success then
        if result then
            for index, value in pairs(result) do
                local object = shouldSave[index]
                object.Value = value
            end
        end
   else
        -- Do some handling here, there was a DataStore error.
        -- For example, you can block saving or kick the player.
    end
end

local function playerRemoving(player)
    -- All your save code up until the repeat in the original goes here.
    -- Then, instead of a repeat loop, use UpdateAsync.
    -- Use waitForRequestBudget(Enum.DataStoreRequestType.UpdateAsync) first.
    -- UpdateAsync automatically calls itself multiple times until success.
    -- This same process needs to be used in your BindToClose.
end

Players.PlayerAdded:Connect(playerAdded)
Players.PlayerRemoving:Connect(playerRemoving)
for _, player in ipairs(Players:GetPlayers()) do
    playerAdded(player)
end
5 Likes
Help with datastores
#9

Would I just return the new value in the UpdateAsync function?
I don’t need the oldValue parameter.
Also I wouldn’t need a pcall, right? Because it calls itself multiple times until success.

ds:UpdateAsync(key, function()
    return newValue
end)
#10

DataStore calls still need pcall regardless. Whether it internally retries or not doesn’t have any bearing on the potential for the function to fail due to an outage or throttle. pcall isn’t about whether a function retries or not, it’s about handling the case of an error.

The oldValue parameter is not required but it would be helpful to make use of UpdateAsync for its intended purpose which is to mutate values directly. That being said, some developers find it to be more convenient to give back new data like a SetAsync but using the benefits of UpdateAsync, which is fine to do as well. You only need to return the new data to be written in.

1 Like