Datastore save script

[Pxxm (Driver)]

Anyway I could optimize this?

function dataStoreModule:SaveAsync()
    local mainPlayer    = self.Player
    local playerKey     = self.PlayerKey
    local saveAttempts  = 0
    local passedSucc, mainErr
    globalPlayerData[mainPlayer] = nil
    self.Player     = nil;
    self.PlayerKey  = nil;
    repeat
        saveAttempts += 1
        passedSucc, mainErr = pcall(function()
            return mainDataStore:SetAsync(playerKey, self)
        end)
    until (passedSucc or saveAttempts >= 3)
    if (passedSucc) then
        return print(mainPlayer.Name..": Sucessfully Saved Data")
    end
    return warn(mainPlayer.Name..": Failed To Save Data", mainErr)
end

globalPlayerData is just so playerRemoving and BindToClose don’t save at the same time.

[DavidNet22 (Dvd)]

I’ve heard that using UpdateAsync is much more useful

@BanTech

Most people use UpdateAsync when they have a table and want to update specific keys while keeping any other data in the table as it was. Or when incrementing or decrementing a numerical value. Or when there is some sort of check, for example if you have XP which can never go down, you might use UpdateAsync to return the largest of the old and new values, math.max(old, new).

[angrybino]

Do you even know how to use pcalls properly? You are calling another function which calls SetAsync, even though this doesn’t matter, you still aren’t using them the right way.

 passedSucc, mainErr = pcall(mainDataStore.SetAsync, mainDataStore, playerKey, self)

Setting variables allocated to the stack to nil is redundant, since they will be cleared when the stack ends assuming they have no strong references.

-- useless
 self.Player     = nil;
 self.PlayerKey  = nil;

[Pxxm (Driver)]

Having an extra check before incrementing the save attempts is redundant. The setasync function yields, therefore if it indeed did fail the loop would just repeat again.

[Pxxm (Driver)]

Yes it is, I already check if it fails with the until (passedSuc or Attempts >= 3). There is no need for an extra check just to increment saveAttempts.

[angrybino]

Oh I didn’t see that you were using a repeat loop, my bad.

[colbert2677 (ImagineerColbert)]

Never force a DataStore request through with a loop. If there’s a legitimate outage or throttle preventing the request from being run, you’re just tripling the error immediately. Use GetRequestBudgetForRequestType if you need to work around request budgets.

Otherwise no, not really, optimisation seems fine. Why are you requesting if there’s any way if you can optimise this, did you benchmark it and you’re not content with the speed or? If you wanted an optimisation-related Code Review then you should know which parts specifically aren’t up to par, shouldn’t be our job to figure that out.