[ERROR] Leaderboard storing "Shirt Sales"

Hey,

I’m currently working with a developer on a leaderboard, which shows how many clothes someone bought in my store. We’re trying to store it, but something isn’t working as expected. The script works fine without the ModuleScript.

Normal Script:

game.Players.PlayerAdded:Connect(function(plr)
local l = Instance.new(“IntValue”, plr)
l.Name = “l”
local Key = plr.UserId
local ds = game:GetService(“DataStoreService”):GetOrderedDataStore(“GlobalLB6”)
local GetSaved = ds:GetAsync(Key)
if GetSaved then
print(GetSaved)
l.Value = GetSaved
else
l.Value = 0
local ds = game:GetService(“DataStoreService”):GetOrderedDataStore(“GlobalLB6”)
ds:SetAsync(plr.UserId, l.Value)
end
local ownitems = {}
local data = ds:GetAsync(plr.UserId)
if data then
ownitems = data
end
end)

for i = 1, 20 do
local sam = script.Sample:Clone()
sam.Name = i
sam.Parent = script.Parent.Frame.ScrollingFrame
sam.Top.Text = tostring(i)
sam.uName.Text = “”
sam.Buys.Text = “…”
sam.LayoutOrder = i
script.Parent.Frame.ScrollingFrame.CanvasSize = UDim2.new(0, 0, 0, 50 * i)
end

function updateGui()
for i,v in pairs (game.Players:GetPlayers()) do
local Data = v:WaitForChild(“l”).Value
local ds = game:GetService(“DataStoreService”):GetOrderedDataStore(“GlobalLB6”)
ds:SetAsync(“Id-”…v.UserId, Data)
end
wait(.5)
local ds = game:GetService(“DataStoreService”):GetOrderedDataStore(“GlobalLB6”)
local pages = ds:GetSortedAsync(false, 20)
local data = pages:GetCurrentPage()
for k, v in pairs (data) do
print(v.key)
if tonumber(v.key) >= 1 then
local frame = script.Parent.Frame.ScrollingFrame:FindFirstChild(tostring(k))
if frame then
frame.uName.Text = game.Players:GetUserIdFromNameAsync()(v.key).Name
frame.Buys.Text = ds:GetAsync(“Id-”…v.UserId)
end
end
end
end

game.Players.PlayerRemoving:Connect(function(plr)
local ds = game:GetService(“DataStoreService”):GetOrderedDataStore(“GlobalLB6”)
ds:SetAsync(“Id-”…plr.UserId, plr.l.Value)
end)

while true do
updateGui()
wait(15)
end

Can’t transfer it into a normal script. Sorry
Module Script:
local mainmodule = {}

    mainmodule.AddItem = function(plr)
    	game.Players[plr.Name].l.Value = game.Players[plr.Name].l.Value + 1
    	local ds = game:GetService("DataStoreService"):GetOrderedDataStore("GlobalLB6")
    	ds:SetAsync(plr.UserId, plr.l.Value)
    end

    return mainmodule


We didn't find the issue. Here is a screenshot of the developer console. I appreciate every help.

Developer console:
![image|347x500](upload://bS1U76HKw8nMjmLpQGnJtvK2IPS.png) 

____

F_eIix aka. IchbinFelix
1 Like

Can you please format this correctly.

print("HI")
1 Like

It’s a little hard to read, but the script says the problem is in line 1 of a script called “14661” as well as a few other number sequences which seem very random. And it’s a script stored in the player. Maybe check that out?

Also just a concern, you might want to use pcalls in your GetOrderedDataStore requests. You call on it twice when a player joins alone, which in itself would have a 6 second delay and trach any other request within that time. I highly recommend you take a look at the DataStore limits

1 Like

I can’t change the first script to a “code”

Now it’s showing a diffrent error:
image

```Lua
-code
`` < with a third `

2 Likes

So check lines 48 and 61. What does it say?
Edit after some counting I found that this is line 48:

frame.uName.Text = game.Players:GetUserIdFromNameAsync()(v.key).Name

The function getting the players name from user id is blank. You need to put the “v.key” inside the brackets

2 Likes

It looks like you are trying to work with @aStRoJim. A late reply in his post contained some feedback you might find helpful:

2 Likes

By the way, I went through the trouble of rewriting your script in my format, fixing some issues along the way. But, instead of just showing you the script, I will show you what’s wrong with your own script, and you can fix it using your own wonderful researching skills, because nobody is supposed to give you free code here:


Source: Through-out the script really

You don’t have to get this datastore every single time, you only have to get it once at the beginning of the script.


Source: Under game.Players.PlayerAdded

Source: Under game.Players.PlayerAdded

You have to implement error-catching methods here because data stores fail sometimes, and when they do fail, they throw an error. You can catch errors using pcall functions. The “saving player data” tutorial on the wiki uses loops whenever the data store fails to return or set data.

Also, there is this article here:


Source: End of for i = 1, 20 do loop

Fairly minor, but you only have to set the scrolling frame’s size once, for what it equals at the end of the for loop, since it is never broken.


Source: Under game.Players.PlayerAdded

Source: Under function updateGui()

So first, the correct concatenation operation uses two periods, not three. You already knew this though, it’s just that Discourse auto-formats it to three, which is an important reason to put this in a code block, and not a quote block.
Secondly, you are using both "Id-"..UserId and UserId as a key to the data store, which is wrong and confusing, and that is what will cause this to error:

Source: Under function updateGui()


The most glaring issue that is surely causing problems:

Source: Under function updateGui()

This should straight-up be replaced with:

frame.uName.Text = game.Players:GetNameFromUserIdAsync(v.key)

Source: Under function updateGui()

So not only is this incorrect because v would not have a .UserId field, but it is already covered by v.value, so you could just replace this line with:

frame.Buys.Text = v.value

Source: Under game.Players.PlayerAdded

This looks like a work-in-progress, so I won’t talk about this.


Other than all that, this script could really use some variable management, but that’s really it.
Have fun working it all out I guess.

2 Likes

Out of wild curiosity, even though two people have mentioned how to correct this segment of code, are you trying to feed the UserId into a function and then convert it back into a name to get their current username? You can do that by using its counterpart, GetNameFromUserIdAsync.

This is one of the more prominently pointed-out issues with your code which is why I wanted to talk about this piece first.

local Players = game:GetService("Players")

local UserId = Players:GetUserIdFromNameAsync(v.key)
local CurrentUsername = Players:GetNameFromUserIdAsync(UserId)

This aside, you have one messy script. Just like @goldenstein64 did, I had to rewrite this script so that I was able to see clearly what was being done and offer up a response.

There are lots of conventional issues and basic mistakes that not only are causing you the issue you’re receiving but are affecting the code’s readability. For your own sanity’s sake, as well as that of potential collaborators, try polishing up your code some.

In the future, please be sure to format your code correctly to make it easier for responses to be made to your questions. Here is a thread to help you out in doing so:

Since golden already gave you a neat round-up of some issues regarding your code, I’ll post what’s essentially an extension of his response. I don’t want to be parroting anything that’s already been said unnecessarily.


The first thing to go over is your lack of upvalues and variables. You should store any multi-use references in variables instead of repeatedly calling services. This serves to improve readability and make your code more performant over constantly calling functions.

Gold describes this as:

While it’s applicable to quite frankly many variables used in your script.

-- All services at the top. Canonical way to get services is via DataModel.GetService.
local Players = game:GetService("Players")
local DataStoreService = game:GetService("DataStoreService")

-- Constants below. These are your next most-used variables.
local DataStore = DataStoreService:GetOrderedDataStore("GlobalLB6")

The next is to address the odd mess of ordering you have for code. Your current code runs these operations in order:

  • Connect a function to PlayerAdded
  • Perform some kind of iteration for a process that looks like something you can do precompile instead of at run time (do you have any specific reason for allocating leaderboard placements when the server begins running instead of having them implicitly available?)
  • Create a function defined to updateGui
  • Connect a function to PlayerRemoving
  • Begin a non-terminating while loop

Personally, I’ve grown more accustomed to creating functions and then connecting them all at the end. Not only does this help for readability and organisation (once again), but it also allows you to use the functions to catch objects potentially added before functions connect.

local function onPlayerAdded(Player)
    -- Code
end

Players.PlayerAdded:Connect(onPlayerAdded)

for _, Player in pairs(Players:GetPlayers()) do
    onPlayerAdded(Player)
end

Point is though that you should think about the ordering of your functions a little more carefully or what you’re running. The iterative loop can be done in Studio instead of at run time, since it seems that each part of it is hard coded. The rest of it is thinking about the organisation and practicality of your code.


The next item is that you need to think about caching data in a session by creating a table storing player’s data. You update their session data in this table. You are making far too many calls to DataStores where unnecessary, which is what is going to cause you to hit Data Store limitations. This means the following:

  • Throttled DataStores
  • Failure to save data
  • Desynchronised data
  • Improper data techniques
  • Retaining poor data management techniques

Make sure to brush up some on DataStores and think about best practices. Ask around if you’re confused about something or you want an alternative way to do something.

Your code uses three GetAsync calls in the following manners:

  • Twice when the player initially joins over caching the result of the initial call, which means n*2 calls per player that joins your server.
  • Once when updateGui is called, which acts upon every player in the server(?). This means n calls per player, per 15 seconds.

This is a good example of how to run out of DataStore request budgets per minute. You must keep the budget in mind while working with your code, so you can work around those and prevent your code from hitting limits where it shouldn’t. At the very least, I don’t know what your second GetAsync call is quite doing or if it’s actually necessary, but the first point can be slashed down to a single GetAsync call.

Then of course, there’s the lack of pcalls, to which you can look to gold’s response for.

The second point is your use of SetAsync. The only concerning scenario is why you’re calling it per player every time updateGui is called. If it’s meant to be some kind of auto update loop that also updates the board, that’s fine, but the time interval is much too short for your use case. 30 is good, 15 is not. You should keep some requests left over.


And, finally, there’s your ModuleScript. You can use IncrementAsync instead of whatever weird implementation you’re currently working with. Though this ModuleScript also goes back to addressing data management and using a cache instead of calling SetAsync whenever data changes (which is the implications of the name AddItem).


That’s all from me. I guess.

4 Likes

Ok, the real issure here, as I explained in my post, is that the key that I save (player’s id) in the beggining using :SetAsync is not the same that the one that I extract from the DataStorePages, and that is causing all the trouble. Instead of f.e.“1234567” it shows me “947462682” as the key

There’s nothing wrong with using SetAsync. The “article” is either over-exaggerating in order to try to get the point across that UpdateAsync has uses or it’s just wrong. If you’re only one script is saving to that datastore/changing the values that would be saved, there is no purpose in using UpdateAsync.