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:
____
F_eIix aka. IchbinFelix
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
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:
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.
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).
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.
