Global Leaderboard Issue

Here is the code for my global leaderboard:

game.Players.PlayerAdded:connect(function(plr)
    -- Define variables
    local uniquekey = 'id-'..plr.userId

    local leaderstats = Instance.new("IntValue", plr)
    leaderstats.Name = "leaderstats"

    local savevalue =  Instance.new('IntValue')
    savevalue.Parent = leaderstats
    savevalue.Name = "Global KO's"

    -- GetAsync
    local GetSaved = ds:GetAsync(uniquekey)
    if GetSaved then
        savevalue.Value = GetSaved[1]
    else
        local NumbersForSaving = {savevalue.Value}
        ds:SetAsync(uniquekey, NumbersForSaving)
    end
end)

game.Players.PlayerRemoving:connect(function(plr)
    local uniquekey = 'id-'..plr.userId
    local Savetable = {plr.leaderstats.savevalue.Value}
    ds:SetAsync(uniquekey, Savetable)
end)

What I am trying to do here is have a global KO variable that will display across all universes of the same game. The leaderboard works exactly how I want it to; there is no issue there. However it will not save and/or load the the KO’s, so am I using incorrect syntax somewhere? Roblox Studio and the console are not giving me anything to work with.

4 Likes

Please format your code correctly. Read this topic if you would like to know more about inserting code on the Developer Forum

Inserting Code Snippets

It could be possible that the your player removing function is not getting the player object. As soon as the player leaves the game, the plr in your removing function is nil, hence the value that is saved to the data store is also nil since the player data is stored in the player object.

Edit:
This would probably fix your code

game.Players.PlayerRemoving:connect(function(plr)
    local PlayerReference = plr
    local uniquekey = ‘id-’..PlayerReference.UserId
    local Savetable = {PlayerReference.leaderstats.savevalue.Value}
    ds:SetAsync(uniquekey, Savetable)
    PlayerReference = nil
end)

I beleive the error is here.

you set the name to “Global KO’s” not savevalue

i think it should be local Savetable = {plr.leaderstats["Global KO's"].Value} instead

2 Likes

There are actually a lot of factors that could be causing errors just by looking at the code you posted.

Things like

plr.useId is supposed to be plr.UserId

Your SetAsync and GetAsync must be wrapped in a pcall

3 Likes

Additionally, using :connect is depreciated. You should use :Connect instead

2 Likes

Just a quick thing.

For data loss prevention (and performance), it’s best to only use setAsync on a key which contains nothing as yet (AKA nil).

Instead of this, use updateAsync. The benefits of this are that you can use a function to update the data. Also, the data in the key won’t be whiped before insertion, meaning that if the process fails, there will be a reduced chance of data loss.

Why use the Table though? If you are just setting a datastore to a value of the KO’s, just set the value straight to it like datastore:SetAsync(Key, savevalue.Value). This has worked for all of the leaderboard’s I’ve done, and its more simple then going through a table.

Also, its better to use value.Changed instead of game.Players.PlayerRemoving because it does not save it the player crashes, the .Changed will just do it whenever the value it changed.

But this is a waste of resources (datastore calls are generally quite expensive network operations) and also you might even go over the max request limit.

Instead, adapt this to use .changed but only save every few rounds, few minutes, certain amount of points, etc. It really depends on what suits your game. But, you shouldn’t really go overwriting datastore keys every time a value changes.

A minor pet peeve; try not to use the parent argument of Instance.new. Make it a habit to set properties first before the parent as well. Your objects should already be all set up before being dropped into the DataModel.


You have indeed stumbled across incorrect syntax. You’ve made an awkward error wherein you try indexing “savedvalue” in leaderstats, despite it not existing as a child of the leaderstats. See farizarps’ post for the correction. Remember to review your code carefully, as you can meet with unfortunate oversights like this.


@wevetments

No, it definitely does. The Player instance is parented to nil but it’s not destroyed or garbage collected until everything associated with it is gone. There’s a reason why PlayerRemoving functions pass a Player argument.

Won’t cause an error. userId was deprecated in favour of UserId and keeping a consistent PascalCase naming convention for properties. The most it might do is throw a warning to use UserId instead. This is a non-problem.

They do not have to be, but it’s good practice to do so. Anything that is internally a web call should always be wrapped in a pcall in the instance that it throws an error or fails to function. It also allows you to handle failures using the success boolean returned from a pcall.


@12904

I’m not sure why this is being raised as a concern since it’s not relevant to OP, but to address it anyway, a table can be used if you want to hold several items of data to be fetched or updated. Using a table is fine, even if it’s single-entry. As well, there is no “simplicity” difference between using what value type is fetched from a DataStore.

No it’s not, at all. Changed is an excellent way to exhaust your DataStore budget which is very limited. This is also bad practice in general. You should only save as in when it’s necessary. That means coercing data after a certain circumstance has happened, when a player leaves or in an autosave interval running in the background. Other use cases applicable.


@CodeNinja16

I wouldn’t say they’re expensive. Even if they are, the expenses are handled in the backend and are relatively invisible to you as a developer.

2 Likes