I have this DataStore script and I'm sure there are things that need to be changed to make it better

Help and Feedback Code Review
1

I want know all the inefficiencies in this code, so I can make this script better.

DATASTORE SCRIPT: 
local Players = game:GetService("Players")
local DataStoreService = game:GetService("DataStoreService")
local PointsDataStore = DataStoreService:GetDataStore("PointsDataStore")

local autosaveInterval = math.random(240, 360)

Players.PlayerAdded:Connect(function(player)
	
	local leaderstats = Instance.new("Folder")
	leaderstats.Name = "leaderstats"
	leaderstats.Parent = player
	
	local points = Instance.new("IntValue")
	points.Name = "Points"
	points.Parent = leaderstats
	
	local data
	local success, err = pcall(function()
		data = PointsDataStore:GetAsync(player.UserId.."-points")
	end)
	
	if success then
		if data and data ~= 0 then
			print("Data loaded successfully!")
			player:WaitForChild("leaderstats"):WaitForChild("Points").Value = data
		else
			print("New player! Creating data...")
			player:WaitForChild("leaderstats"):WaitForChild("Points").Value = 0
		end
	else
		warn("There was an error whilst loading data!")
		warn("Error code: "..err)
	end
	
	spawn(function()
		while true do
			wait(autosaveInterval)
			print(points.Value)
			local success, err = pcall(function()
				PointsDataStore:SetAsync(player.UserId.."-points", points.Value)
			end)
			if success then
				print("Autosave complete!")
			else
				warn("Unable to save data!")
				warn("Error code: "..err)
			end 			
		end
	end)
end)

Players.PlayerRemoving:Connect(function(player)
	
	local points = player:WaitForChild("leaderstats"):WaitForChild("Points")
	
	local tries = 0
	local success, err
	repeat
		tries = tries + 1
		success, err = pcall(function()
			PointsDataStore:SetAsync(player.UserId.."-points", points.Value)
		end)
		print("Tries..: "..tries)
		if not success then wait(1) end
	until success or tries == 3
	
	if success then
		print("Data saved succesfully!")
	else
		warn("There was an error whilst saving data!")
		warn("Error code: "..err)
	end
	
end)

game:BindToClose(function()
	
	for _, player in pairs(Players:GetPlayers()) do
		DataStoreService:SetAsync(player.UserId.."-points", player:WaitForChild("leaderstats"):WaitForChild("Points").Value)
	end
	
end)

(I’m not too experienced with DataStores which is why I’m asking for help.)

EDIT: Looking around it seems SetAsync() is pretty destructive and it’s much better to use UpdateAsync(), however I don’t know how to use UpdateAsync() so I’ll need some help there.

EDIT2: Just experienced this warning, I hope there can be a way to prevent it.
DataStore request was added to queue. If request queue fills, further requests will be dropped. Try sending fewer requests.Key = 277320496-points

2

Nice! I haven’t read through it all but it looks decent! My only question though is, why is the auto-save interval a random number? It serves no purpose whatsoever to me, since it’s not a number that players see.

3

Well I saw a post by @berezaa that said auto saves should be random between 3~5 minutes, which is why I did it. :grin:

1 Like
4

Oh well I suppose it may serve some purpose if there are many players in your game, just thought about that haha. My apologies. :sweat_smile:

5

Is there really nothing to improve though, there’s nothing wrong with my datastore…?

6

Well you used SetAsync(), which does not consider the previous value. So for example, if the points haven’t loaded and you use SetAsync(), then it does not matter and they will just set the value to the new intended one. However this may be risky, players may lose their data since DataStores error occasionally. So the use of UpdateAsync() will error if the points haven’t loaded (from my experience), so that you do not update a false value.

1 Like
7

Issues:

  • DON’T USE SPAWN. Spawn is unoptimized, as it runs slower and does not immediately fire. Expect lag to source from it.
  • Avoid auto-saving, it could cause issues with datastore exaustion.
  • In your player-removing event, store the player id and data beforehand, so that it’s there even if the player instance is gone.
  • You use player:WaitForChild("leaderstats"):WaitForChild("Points") multiple times, despite already having a variable that references it.
  • You don’t pcall or wait in the close-bind.
  • There are random whitespaces (enters, tab, space) that don’t belong and make the code harder to read.
  • There are some repetitive lines of code.
8

Okay, so what would be a good substitute for spawn, as I really do want some auto-saving action. (Would a longer interval between auto-saves be good? Like 6-8 minutes.)

In your player-removing event, store the player id and data beforehand, so that it’s there even if the player instance is gone.

How would I go about doing that, I’m not to experienced with this… :sweat_smile:

You do have really good advice, and I’ll make sure to apply all of your advice in my script.

9

Use coroutine. It’s made in vanilla Lua and meant for seperate threads, like yours.
However, an auto-save is redundant and even harmful to your game if you already save on leave.

Make variables! Variables are your friends that remember things for you, and don’t complain about it.
You could have variables for the player id, points value, etc.

Hope I helped :slight_smile:

10

Use coroutine . It’s made in vanilla Lua and meant for seperate threads, like yours. However, an auto-save is redundant and even harmful to your game if you already save on leave.

Okay, I guess I have to learn coroutine as well, thanks for your help.

Make variables! Variables are your friends that remember things for you, and don’t complain about it.
You could have variables for the player id, points value, etc.

Oh… forgot about those…

1 Like