How good is my datastore?

I am trying to improve my datastores, this datastore script has notifications with discord webhooks when saving data goes wrong for example. Data versioning, and anything you might think is “unnecessary”, is probably because this is supposed to be compatible with older data, which wasn’t shaped the same way, though, if you think that, let me know.

local runService = game:GetService("RunService")
local httpService = game:GetService("HttpService")
local webhook = ""

local ds = game:GetService("DataStoreService"):GetDataStore("PlayerData")

game.Players.PlayerAdded:Connect(function(plr)

	local fold = Instance.new("Folder")
	fold.Name = "leaderstats"

	local cash = Instance.new("IntValue")
	cash.Name = "Cash"
	cash.Parent = fold

	local donations = Instance.new("IntValue")
	donations.Name = "Donations"
	donations.Parent = fold

	local kills = Instance.new("IntValue")
	kills.Name = "Kills"
	kills.Parent = plr

	fold.Parent = plr

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

	local isStoppable = Instance.new("BoolValue")
	isStoppable.Name = "isDropping"
	isStoppable.Value = false
	isStoppable.Parent = plr

	local isNameTagHidden = Instance.new("BoolValue")
	isNameTagHidden.Name = "isNameTagHidden"
	isNameTagHidden.Value = false
	isNameTagHidden.Parent = plr



	local s, x = pcall(function()
		local data = ds:GetAsync(plr.UserId)
		print(data)
		if data then
			if not type(data) == "table" then
				data = httpService:JSONDecode(data)
			end
			if data["leaderstats"] then
				cash.Value = data.leaderstats.Cash
				donations.Value = data.leaderstats.Donations
				kills.Value = data.leaderstats.Kills
			else
				cash.Value = data.Cash
				donations.Value = data.Donations
				kills.Value = kills.Value
			end
			if data["DataId"] then
				dataId.Value = data.DataId
			end
		end
	end)

	if not s and x then
		warn(x)
		local cannotSave = Instance.new("BoolValue")
		cannotSave.Name = "CannotSave"
		cannotSave.Value = true
		cannotSave.Parent = plr
		plr:Kick("Datastores are down, please join later.")
	end
end)

local function SavePlayer(plr)
	local data = {
		leaderstats = {
			Cash = plr.leaderstats.Cash.Value,
			Donations = plr.leaderstats.Donations.Value,
			Kills = plr.Kills.Value
		},
	}
	if not plr:FindFirstChild("CannotSave") then
		local s, x = pcall(function()
			ds:UpdateAsync(plr.UserId, function(oldData)
				if oldData then
					if oldData["DataId"] and oldData.DataId == plr.DataId.Value then
						data.DataId = plr.DataId.Value + 1
						return httpService:JSONEncode(data)
					elseif oldData["DataId"] then
						return nil
					else
						data.DataId = plr.DataId.Value + 1
						return httpService:JSONEncode(data)
					end
				else
					data.DataId = plr.DataId.Value + 1
					return httpService:JSONEncode(data)
				end
			end)
		end)
		if not s then
			warn(x)
			local tries = 0
			local lastError = nil
			local sucess = false
			repeat
				local s, errormsg = pcall(function()
					local webhookData = httpService:JSONEncode({
							embeds = {{
								title = "Data lost!";
								description = "\n UserId: ".. plr.UserId.. "\n Data: ||".. httpService:JSONEncode(data).. "|| \n Error: *".. x.. "* \n Place: https://www.roblox.com/games/".. game.PlaceId;
								color = 1752220;
								url = "https://www.roblox.com/games/".. game.PlaceId
							}}
					})
					httpService:PostAsync(webhook, webhookData)
				end)
				if s then lastError = nil sucess = true else lastError = errormsg end
				tries += 1
				wait()
			until s or tries >= 3
			if tries >= 3 then
				warn(lastError)
			end
		end
	end
end

game.Players.PlayerRemoving:Connect(SavePlayer)

game:BindToClose(function()
	if not runService:IsStudio() then
		for _, plr in pairs(game.Players:GetPlayers()) do
			SavePlayer(plr)
		end
	end
end)

Also, an example of how the webhook message looks:

image

3 Likes

Don’t worry guys, Webhook isn’t “”, I just censored it.

1 Like

I really like this datastore. It’s simple but the code quality is pretty good.
A few tips:

  • In terms of reuseability in other projects this doesn’t seem like a good idea. (You never know if you are going to have the same leaderstats in other projects). This is why it’s good to have modularity. I recommend making this a module.
	local data = {
		leaderstats = {
			Cash = plr.leaderstats.Cash.Value,
			Donations = plr.leaderstats.Donations.Value,
			Kills = plr.Kills.Value
		},
	}
  • repeatedly calling pcall constantly is a terrible idea. pcall is an expensive operation.
  • use runServiceHeartbeat:Wait() over wait(). many posts cover on why.
  • use ipairs to iterate through :GetPlayers(). :GetPlayers() is a method that returns an array. pairs / next is meant to be used on only dictionaries. ipairs is only for arrays. Even though pairs works on arrays, ipairs is still faster.
  • In terms of readability, try to make your variable names specific.
-- not good
local s, x = pcall(function()
end)
-- good 
local success, result = pcall(function()
end)
2 Likes

I’d suggest using…

local s, x = pcall(DataStore.GetAsync, DataStore, id)

if s then -- and checking if succeed to get data
   if x then  -- and then checking if player has data or no

   end
else
  -- you should handle any error there, like retries or not saving data because it failed to load
end

-- saving with SetAsync
local s,x = pcall(DataStore.SetAsync, DataStore, id, value)

-- saving with UpdateAsync
local s,x = pcall(DataStore.UpdateAsync, DataStore, id, transformFunction) -- you can use it the same way like SetAsync but you should use SetAsync if that's the case.

-- then check if data succeeded to save
if s then
  warn("saved")
else
  warn(x) -- in this case x will be the error message
end

Instead of…

local s,x = pcall(function() end) -- creating a new function and doing everything inside of it
1 Like

You’re gonna have a major problem if you’re using Webhooks to a Discord chat to handle data loss. This is because you can only send N amount of messages before clogging up the chat in a short time span per Bot. Now imagine you have a huge player base and the service suddenly goes down, this will clog up the bot and simply not work and I’m pretty sure Discord would get pretty angry for spamming their API (as far as my friends have said when having a similar system, and I know this from personal experience when using in-game chat to Discord).

2 Likes

While I understand discord limitations I don’t think it’s that big of a deal, I kick the player if when they join DataStore Services are down, and also this only sends a message when it had a problem saving, it only retries 3 times; Usually you don’t see HTTPService being “down”, at least that’s what I heard;

I actually was thinking of making it a module, I usually just re make the same code LOL, I think I’m going to get some tips and ideas to implement something into this module before starting to make it;

Thanks for the tip, I’ve heard somewhere they’re the same speed now, but still, thanks. I’m going to try and test that;

Also true, but actually this only happens on the pcall on the “backup” discord webhook, on where it usually doesn’t error out; Like I said people say HTTPService cannot be “down” in a way; And I limit it to 3 tries just in case :wink:

SetAsync is bad, UpdateAsync might look just like a Get/Set combo, but on the backend, when you call UpdateAsync, ROBLOX will try to run these calls in order. So EVEN if you’re using UpdateAsync poorly, it’s still better than SetAsync, UpdateAsync will yield until it’s their turn, at least that’s what people said, also UpdateAsync helps so that you can’t overwrite player data with nil, because if it’s nil, it will not save.

Yes, I know about UpdateAsync, I actually use the Get/Set combo for session locking, for games I need to save tools.

also, about my old post: How good is my datastore? - #4 by LiamKyd

1 Like

Oh yeah, about this, I meant like if you’re going to just overwrite data like SetAsync then I’d recommend just sticking with SetAsync because it would be pointless to use UpdateAsync.

2 Likes

You will have a problem though when you want to save player data progress, you see the people who are in the session WILL lose their data, not because the requests you are sending will have any limitations but because you will clog up the how many messages per X seconds you can send through webhook through 1 Bot. If you go test this out you will see what I mean. The ONLY way you can somewhat prevent this is to make a collection of all the player data and send them in a list but even so, you would have to somehow make it not clog up on Discords end.

Okay but you still want to wrap it in a pcall even if you are making HTTPService requests because something might go wrong on the endpoint or you might reach the request limit, you never know if something might go down.

Yes, ipairs are faster than pairs HOWEVER the difference is too miniscule to actually make a difference. Here’s a short article about it:

There are tons of different articles over the speed difference that you can find but this one is pretty good. They are faster but not THAT much faster and here’s the math behind it in a conversation that I had with someone about it: If name contains certain word (like string.find but names) - #15 by MJTFreeTime

However, you really should use pairs if you are iterating through an associative table ONLY:

Set and GetAsync need pcall so you’re not doing anything wrong here, I actually really like that you have a retry check. However, there is one thing I’d like to recommend…

In your BindToClose, please save all the player data in coroutine.wrap functions. It’ll be faster than having to wait for every players data to save in one by one in a linear way.

1 Like

Doing so in coroutine.wrap() makes it so the server thinks saving was done right away. So it will not save; I had this issue before while making a module;
The server will only close when all BindToClose() calls are done with, wrapping in a coroutine will make it NOT save it; it’s only faster because it’s not saving;

Not true, you must’ve messed up something with the code. If you look at other peoples recommendation with others DataStore code review threads here you’ll see that often people tell to use coroutines with BindToClose.

BindToClose does indeed wait to shut down the server but even that has a limit, it’s not like you can keep a server alive indefinitely. “After 30 seconds, the game will shut down regardless if all bound functions have completed or not.”

Also, when you call the same key twice, you need to wait a certain amount before being able to call it again. I noticed your re-try only has wait() which would not suffice:

I like to not have it mostly because of messing with studio bit thanks for the tip.

They are really necessary in this case, asynchronous functions in Roblox yield the thread and send a HTTP request to an API and and the response can usually be an error from the API itself.

The issue was not that he was using pcall. It’s that he’s spamming it per wait() yield time. Threads are something that should be used wisely since they are expensive and can possibly kill performance (if spammed of course).

The good part is that he was only doing 3 of them at max so it’s not a big deal.

I might update this post to show my current solution using Convert normal datastores into OrderedBackups easily with DataStore+, I wanna add and fix some stuff on it, but mostly right now it’s fine, and better than using the normal DataStoreService.

On the game that I showed the code off, I recently also added a type of session-locking too.

oh that sounds pretty cool. What did you use as a substitute for DataStoreService?

1 Like

Because it kind of works, and “feels” like a normal datastore, you’re supposed to pretend it’s a normal datastore and let the module just do the work. It also makes it easier to understand, and convert already existing games into.

In fact I call it DataStoreService on my own game on ServerScriptService, and I pretend it’s a Service.

It has the same names for the functions, like :GetDataStore()

And the methods like Get, Set, Update, Increment, all of those, very similar to the normal datastore method names, except it doesn’t have the “ASync” part because I find it kind of stupid. But yeah.

1 Like

I’m still confused on how it would save data. “let the module do the work” doesn’t give the specifics on how the data is saved lol.

I would assume it’s hooked to an external database using HTTP service?

No no, it still uses normal roblox datastores, it just does a better job, it has features like backups, and support for onUpdate (because normal datastores have them deprecated)

1 Like

It actually uses the same system for saving data from DS2’s original method, which is keeping every single version of the data you ever saved, plus it has a main version to allow you to edit data / get data from normal datastores easily.

1 Like