Is this a viable way to save data?

Help and Feedback Code Review
#1

The topic pretty much covers it all.

local dataThread = coroutine.wrap(function()
	while true do
		local players = game:GetService("Players"):GetChildren()
		wait(60)
		for _, plr in pairs(game:GetService("Players"):GetChildren()) do
			local success, err = pcall(function()
				buttonBuxStore:SetAsync(plr.UserId.."ButtonBux", plr.leaderstats["Button Bux"].Value)
			end)
			
			if success then
				print("AUTO-SAVED")
			else
				warn(err)
			end
			
		end
	end
end)

dataThread()
#2

Saving them all at once might be pretty risky, and some of them might get dropped. I would keep them on per player loops, something like this:

players.PlayerAdded:Connect(function(player)
    while (player.Parent) do
        wait(60)
        saveData(player)
    end
end)
1 Like
#3

@Pokemoncraft5290 That loop will still run even if the player leaves the game. Simple fix would be to check player.Parent.

@dockboy20006
I would recommend you use :GetRequestBudgetForRequestType() to check if you have enough budget to make the request (aka SetAsync).

Some other “optional” things I would recommend would be to use UpdateAsync over SetAsync. You can actually use UpdateAsync to replace both GetAsync and SetAsync! UpdateAsync returns the most up to date data, so you have less of a risk of data be overwritten by 2 servers. You can read more about the differences in the actual API (linked below).

Another minor change is to use the new task library because its much better! Long story short, use task.wait(60) instead of wait(60).

These suggestions are just my personal preference. You don’t need to implement them but they are just good habits/tips to use in the future:

  • Currently if your request were to error, it would just end there and not save the players data. I would recommend retrying to save that players data. I don’t think its that important for what you’re doing right now as in 60 seconds it will attempt to save the players data again, but I would recommend implementing it for when the player leaves the game (assuming you also save their data when they leave).

  • 100% recommend using tables to save your data instead of just saving 1 value. It will make your life so much easier if you ever want to add another stat/value you wish to save. Will also help reduce calling SetAsync/GetAsync/UpdateAsync constantly to get 1 specific value.

  • Define your services (e.g Players service) at the start of the script since you are using it in multiple places (this also applies to other variables that you will constantly use throughout a script).

  • You define local players = game:GetService("Players"):GetChildren() but then don’t actually use it at all: for _, plr in pairs(game:GetService("Players"):GetChildren()) do

  • Use :GetPlayers() instead of GetChildren()

  • Use ipairs instead of pairs since :GetChildren and :GetPlayers() are both arrays.

API Links:

2 Likes