How can I simplify this?

Help and Feedback Scripting Support
,
1

is there any way i can add a table to shorten it? lmk please

here’s the code:

-- services
local DSS = game:GetService("DataStoreService"):GetDataStore("PlayerData")
local Players = game:GetService("Players")

-- on player added
Players.PlayerAdded:Connect(function(player)
	-- create the folders to store all the save values
	
	local leaderstats = Instance.new("Folder", player)
	leaderstats.Name = "leaderstats"

	local playerstats = Instance.new("Folder", player)
	playerstats.Name = "playerstats"
	
	local playersettings = Instance.new("Folder", player)
	playersettings.Name = "playersettings"

	-- create save values (bounty, kills, cash, bank)
	local bounty, kills = Instance.new("NumberValue", leaderstats), Instance.new("NumberValue", leaderstats)
	bounty.Name, kills.Name = "Bounty", "Kills"
	bounty.Value, kills.Value = 0, 0

	local cash, bank = Instance.new("NumberValue", playerstats), Instance.new("NumberValue", playerstats)
	cash.Name, bank.Name = "Cash", "Bank"
	cash.Value, bank.Value = 0, 0
	
	-- create settings values
	local firstPlay = Instance.new("BoolValue", playersettings) -- will pull up a UI on the first time playing for a user
	firstPlay.Name = "FirstPlay"
	firstPlay.Value = true
	
	
	-- saving the player's data
	local playerKey = "id_"..player.UserId
	local getSaved = DSS:GetAsync(playerKey)

	local save1, save2, save3, save4 = kills, cash, bank, firstPlay

	if getSaved then
		save1.Value, save2.Value, save3.Value, save4.Value = getSaved[1], getSaved[2], getSaved[3], getSaved[4]
	end

	game.Players.PlayerRemoving:Connect(function(player2) -- makes sure the same player who joined & the same player who left have the same savedata
		if player == player2 then
			DSS:SetAsync(playerKey, {save1.Value, save2.Value, save3.Value, save4.Value})
		end
	end)

	-- counts player kills
	player.CharacterAdded:Connect(function(char)
		char.Humanoid.Died:Connect(function()
			if char.Humanoid:FindFirstChild("creator") and char.Humanoid:FindFirstChild("creator") ~= nil and char.Humanoid:FindFirstChild("creator").Value ~= nil then
				char.Humanoid:FindFirstChild("creator").Value:FindFirstChild("leaderstats").Kills.Value += 1
			end
		end)
	end)
end)

any help would be appreciated, thanks in advance!

2 Likes
2

Hey you can use module script and store all the folders and values you want. After, you can require the module in the server script and loop through the module and create each folder and value in the player. Also, try wrapping the GetAsync in a pcall as it’s much safer.

If you need help with using pcalls with datastore look at the documentation.

3 Likes
3

Additional to the reply above, specifically for the counts player kills part, you may simplify it by saving the resulting value of the FindFirstChild directly into a variable so you aren’t constantly invoking the method, besides, removing the check of this ‘creator’ instance against ~= nil since the condition just before it already checks for that, so it’s redundant.

All of the described above would look like so:

-- counts player kills
player.CharacterAdded:Connect(function(char)
	char.Humanoid.Died:Connect(function()
		local creator = char.Humanoid:FindFirstChild("creator")
		if creator and creator.Value ~= nil then
			creator.Value:FindFirstChild("leaderstats").Kills.Value += 1
		end
	end)
end)

Furthermore, noticed that you assign multiple variables separated by commas, like this:

x1, y1 = x2, y2

I personally do like that format when values are related or alike, so you save up space.
Following that line of thought, I also like saving lines by reducing to a single line those if statements that only take up 3 or 4 lines:

if x then -- 1
   foo() -- 2
end -- 3

Into a single line, for instance, in your script

if getSaved then
		save1.Value, save2.Value, save3.Value, save4.Value = getSaved[1], getSaved[2], getSaved[3], getSaved[4]
end

-- Reduced into:

if getSaved then save1.Value, save2.Value, save3.Value, save4.Value = getSaved[1], getSaved[2], getSaved[3], getSaved[4] end

Some will say that it makes it less readable, and while that can be true, it does look tidier (to be fair, the multiple declaration of four values you’re doing there isn’t particularly considered readable neither, but it boils down to the same plate: Some, like myself, do read better the code when it’s organized that way).

Follow whatever floats your boat in the best way possible.

2 Likes
4

Kind of a side-note here, but the way you are handling players leaving is causing a memory leak.
You’re creating a new connection on the PlayerRemoving event every time a player joins, but you never disconnect any.

You really should only have one connection to this event, and then have that handle all players.

2 Likes
5

Doesn’t the connection disconnect automatically when the player leaves ( I.E. The player instance gets destroyed )?

OP doesn’t ssem to be storing the connection in a table ( this make it so connections doesn’t disconnect automatically )

Agree.

1 Like
6

Connections are automatically disconnected when the event they belong to is destroyed. This happens when the instance that the event belongs to is destroyed. So the CharacterAdded event and the Died event will automatically disconnect once the player respawns/disconnects. However the PlayerRemoving event belongs to the Players service, which only gets destroyed when the server shuts down. So no.

The things about OP not having any references is garbage collection, which applies to variables and tables that no longer have any references. This does not apply to connections being disconnected.

2 Likes
7

Oh, I’ve learned something new thanks.

2 Likes
8

Trimmed the script a bit

Script 
local DSS = game:GetService("DataStoreService"):GetDataStore("PlayerData")
local Players = game:GetService("Players")

Players.PlayerAdded:Connect(function(player)
	local leaderstats = Instance.new("Folder", player)         leaderstats.Name = "leaderstats"
	local playerstats = Instance.new("Folder", player)         playerstats.Name = "playerstats"
	local playersettings = Instance.new("Folder", player)   playersettings.Name = "playersettings"
	local bounty = Instance.new("NumberValue", leaderstats)	        bounty.Name = "Bounty"
	local kills = Instance.new("NumberValue", leaderstats)           kills.Name = "Kills"
	local cash = Instance.new("NumberValue", playerstats)             cash.Name = "Cash"
	local bank = Instance.new("NumberValue", playerstats)             bank.Name = "Bank"
	local firstPlay = Instance.new("BoolValue", playersettings)  firstPlay.Name = "FirstPlay"
	
	local key = "id_"..player.UserId
	local data = DSS:GetAsync(key)
	
	if data then
		kills.Value, cash.Value, bank.Value, firstPlay.Value
			= data[1], data[2], data[3], data[4]
	else firstPlay.Value = true
	end

	Players.PlayerRemoving:Connect(function(p)
		if p == player then
			DSS:SetAsync(key, {kills.Value, cash.Value, bank.Value, firstPlay.Value})
		end
	end)

	player.CharacterAdded:Connect(function(char)
		char:WaitForChild("Humanoid").Died:Connect(function()
			local tag = char.Humanoid:FindFirstChild("creator")
			if tag and tag.Value then
				local ls = tag.Value:FindFirstChild("leaderstats")
				if ls then ls.Kills.Value += 1
				end
			end
		end)
	end)
end)
1 Like
9

Hey can you please use pcall when you do GetAsync as getting data from the data stores is not always secure and can result an error.

Here’s an example:

local Players = game:GetService(“Players”)
local DataStoreService = game:GetService("DataStoreService")

local DataStore = DataStoreService:GetDataStore("DataStore")

Players.PlayerAdded:Connect(function(Player:Player)
       local success, errorMessage = pcall(function()
	      DataStore:GetAsync(Player.UserId)
       end)

       if success then
          —Load the data
          print("Data Loaded:", success)
       else
	      print(errorMessage)
           —If you want you can kick the player
           Player:Kick("Server could not load your data! Try again later")
       end
end

Note you are going to have modify it for your game

1 Like
10

I just trimmed up his script… changed 6 things. If you want to add that, add it.
I try to stick to the script posted. pcall is good for seeing an error but more so, pcall throttles.

2 Likes
11

I tried out your script, and I also modified it a little bit with @099ersf2’s snippet of code. I think this is honestly way better than the code I started off with. I’m still a bit curious about storing connections in a table juxtaposed to using .PlayerRemoving, as @stef0206 said at the top of the post. Would you know how to implement that into the code below? I’m not really too sure on how to do that.

Anyways, here’s the code:

Script 
-- services
local DSS = game:GetService("DataStoreService"):GetDataStore("PlayerData")
local Players = game:GetService("Players")

-- on player added
Players.PlayerAdded:Connect(function(player)
	-- create instances
	local leaderstats = Instance.new("Folder", player)         leaderstats.Name = "leaderstats"
	local playerstats = Instance.new("Folder", player)         playerstats.Name = "playerstats"
	local playersettings = Instance.new("Folder", player)   playersettings.Name = "playersettings"
	local bounty = Instance.new("NumberValue", leaderstats)	        bounty.Name = "Bounty"
	local kills = Instance.new("NumberValue", leaderstats)           kills.Name = "Kills"
	local cash = Instance.new("NumberValue", playerstats)             cash.Name = "Cash"
	local bank = Instance.new("NumberValue", playerstats)             bank.Name = "Bank"
	local firstPlay = Instance.new("BoolValue", playersettings)  firstPlay.Name = "FirstPlay"
	
	-- get playerdata
	local key = "id_"..player.UserId
	local data = DSS:GetAsync(key)
	local success, errorMsg = pcall(function()
		DSS:GetAsync(key)
	end)
	
	if success and data then
		print(player.Name.."'s PlayerData loaded status: "..tostring(success))
		kills.Value, cash.Value, bank.Value, firstPlay.Value
			= data[1], data[2], data[3], data[4]
	elseif not success then
		print(player.Name.."'s PlayerData loaded status: "..tostring(errorMsg))
		player:Kick("Roblox DataStores may be suffering connection difficulties as of current, please try again later to avoid player data loss.") -- replace with ui thing later
	elseif not data then
		firstPlay.Value = true
	end
	
	Players.PlayerRemoving:Connect(function(p) -- checks if the player who is leaving is the same player as the one who was already ingame
		if p == player then
			DSS:SetAsync(key, {kills.Value, cash.Value, bank.Value, firstPlay.Value})
		end
	end)
	
	-- on character added
	player.CharacterAdded:Connect(function(char)
		char:WaitForChild("Humanoid").Died:Connect(function()
			local tag = char.Humanoid:FindFirstChild("creator")
			if tag and tag.Value then
				local ls = tag.Value:FindFirstChild("leaderstats")
				if ls then ls.Kills.Value += 1
				end
			end
		end)
	end)
end)
12

Maybe a dataCache

Script 
local DSS = game:GetService("DataStoreService"):GetDataStore("PlayerData")
local Players = game:GetService("Players")
local dataCache = {}

Players.PlayerAdded:Connect(function(player)
	local leaderstats = Instance.new("Folder", player)         leaderstats.Name = "leaderstats"
	local playerstats = Instance.new("Folder", player)         playerstats.Name = "playerstats"
	local playersettings = Instance.new("Folder", player)   playersettings.Name = "playersettings"
	local bounty = Instance.new("NumberValue", leaderstats)         bounty.Name = "Bounty"
	local kills = Instance.new("NumberValue", leaderstats)           kills.Name = "Kills"
	local cash = Instance.new("NumberValue", playerstats)             cash.Name = "Cash"
	local bank = Instance.new("NumberValue", playerstats)             bank.Name = "Bank"
	local firstPlay = Instance.new("BoolValue", playersettings)  firstPlay.Name = "FirstPlay"

	local key, data = "id_"..player.UserId, nil
	local success, err = pcall(function() data = DSS:GetAsync(key) end)

	if success and data then
		kills.Value, cash.Value, bank.Value, firstPlay.Value = data[1], data[2], data[3], data[4]
	elseif not success then player:Kick("Roblox DataStores may be down. Try again later.")
	else firstPlay.Value = true
	end

	dataCache[player] = {key = key, kills = kills, cash = cash, bank = bank, firstPlay = firstPlay}

	player.CharacterAdded:Connect(function(char)
		char:WaitForChild("Humanoid").Died:Connect(function()
			local tag = char.Humanoid:FindFirstChild("creator")
			if tag and tag.Value then
				local ls = tag.Value:FindFirstChild("leaderstats")
				if ls then ls.Kills.Value += 1 end
			end
		end)
	end)
end)

Players.PlayerRemoving:Connect(function(player)
	local d = dataCache[player]
	pcall(function()
		DSS:SetAsync(d.key, {d.kills.Value, d.cash.Value, d.bank.Value, d.firstPlay.Value})
	end) dataCache[player] = nil
end)
1 Like
13

Would this fix the memory leak that Stef was referring to?

14

im not sure if this will work but I tried to make it as clean and readable as possible:

local DataStoreService = game:GetService("DataStoreService")
local DataStore = DataStoreService:GetDataStore("PlayerData")
local Players = game:GetService("Players")

local dataPrefix = "id_"
local maxTries = 5
local retryInterval = 2
local playersData = {}

local defaultData = {
	["leaderstats"] = {
		["bounty"] = 0,
		["kills"] = 0,
	},

	["playerstats"] = {
		["cash"] = 0,
		["bank"] = 0,
	},

	["playersettings"] = {
		["fristPlay"] = true,
	},
}

local function Retry(func, ...)
	local tries = 0
	local success, err

	while tries < maxTries or not success do
		success, err = func(...)
		tries += 1
		task.wait(retryInterval * tries)
	end

	return success, err
end

local function SavePlayerData(key, data)
	return pcall(function()
		DataStore:UpdateAsync(key, function(oldData)
			return data
		end)
	end)
end

local function GetPlayerData(key)
	return pcall(function()
		return DataStore:UpdateAsync(key, function(oldData)
			return nil -- don't overwrite, we only want to get the player's data, thats it
		end)
	end)
end

local function CreateObjectFromValue(value)
	if type(value) == "number" then
		return Instance.new("NumberValue")

	elseif type(value) == "string" then
		return Instance.new("StringValue")

	elseif type(value) == "boolean" then
		return Instance.new("BoolValue")
	end
end

local function Load(player)
	print("Getting player data...")

	local playerKey = dataPrefix..player.UserId
	local success, result = Retry(GetPlayerData, playerKey)

	print("Result: ", result)

	local storedData = table.clone(defaultData)

	for name, list in storedData do
		local newFolder = Instance.new("Folder")
		newFolder.Name = name
		newFolder.Parent = player

		for statName, value in list do
			local objectValue = CreateObjectFromValue(value)
			if not objectValue then continue end
			local savedValue = result[name][statName]

			objectValue.Name = statName
			objectValue.Value = savedValue or objectValue.Value
			list[statName] = objectValue.Value

			objectValue:GetPropertyChangedSignal("Value"):Connect(function()
				list[statName] = objectValue.Value
			end)
		end
	end

	if success then
		playersData[player] = storedData
	end
end

Players.PlayerAdded:Connect(function(player)
	task.spawn(Load, player) -- prevent yielding the player.CharacterAdded

	player.CharacterAdded:Connect(function(character)
		local humanoid = character:WaitForChild("Humanoid")
		
		humanoid.Died:Once(function()
			local creator = humanoid:FindFirstChild("creator")
			if not creator or not creator.Value then return end
			creator.leaderstats.Kills.Value += 1
		end)
	end)
end)

Players.PlayerRemoving:Connect(function(player)
	local playerData = playersData[player]
	if not playerData then return end
	
	local playerKey = dataPrefix..player.UserId
	local success, err = Retry(SavePlayerData, playerKey, playerData)

	if not success then
		warn("Failed to save "..player.Name.."'s data: ", err)
	end
end)
1 Like
15

Is it fine if I replace what you have for setting the initial player dataCache entry with this snippet of code?

dataCache[player] = data

I feel it just shortens it down a bit so I don’t have to keep writing to it when I eventually add more values.

16

Are you asking if changing playersData to dataCache? If so that will still work. By the way this section that contains firstPlay is spelled wrong so you might want to fix it in your code if you use it.

You might want to also save player data in intervals, like every 5 minutes to prevent data loss.

Also, I recommend Including this to the bottom of the script, as it ensures that players data is saved when the server is shut down:

local RunService = game:GetService("RunService")

if not RunService:IsStudio() then -- Check that current server is not studio
	game:BindToClose(function() -- If server is shutting down start saving player data
		for _, player in pairs(Players:GetPlayers()) do -- Save each player's data
			task.spawn(function()
				local playerData = playersData[player]
				if not playerData then return end

				local playerKey = dataPrefix..player.UserId
				local success, err = Retry(SavePlayerData, playerKey, playerData)

				if not success then
					warn("Failed to save "..player.Name.."'s data: ", err)
				end
			end)
		end
	end)
end
17

Do anything you like with it… All these scripts are offered as help on the forum.
Best script would most likely be a combination of a few.

18

I implemented the saving of player data every five minutes & the server shutdown section. It was much needed, so thank you!

Though, I do feel my code is very messy, and I’m still looking for a way to shorten down my code or increase the visual appeal of it (more this).

Here’s the code now:

Summary 
-- services
local DSS = game:GetService("DataStoreService"):GetDataStore("PlayerData")
local Players = game:GetService("Players")
local RS = game:GetService("RunService")

-- define
local dataCache = {}


-- on player added
Players.PlayerAdded:Connect(function(plr)
	-- create instances
	local leaderstats = Instance.new("Folder", plr)         leaderstats.Name = "leaderstats"
	local playerstats = Instance.new("Folder", plr)         playerstats.Name = "playerstats"
	local playersettings = Instance.new("Folder", plr)   playersettings.Name = "playersettings"
	local bounty = Instance.new("NumberValue", leaderstats)	        bounty.Name = "Bounty"
	local kills = Instance.new("NumberValue", leaderstats)           kills.Name = "Kills"
	local cash = Instance.new("NumberValue", playerstats)             cash.Name = "Cash"
	local bank = Instance.new("NumberValue", playerstats)             bank.Name = "Bank"
	local firstPlay = Instance.new("BoolValue", playersettings)  firstPlay.Name = "FirstPlay"
	
	-- get playerdata
	local key, data = "id_"..plr.UserId, nil
	local success, err = pcall(function()
		data = DSS:GetAsync(key)
	end)
	
	if success and data then
		kills.Value, cash.Value, bank.Value, firstPlay.Value
			= data[1], data[2], data[3], data[4]
	elseif not success then
		plr:Kick("DataStoreService may be suffering connection difficulties at this moment, please try playing again later to avoid data loss.") -- replace with ui thing later
	else
		firstPlay.Value = true
	end
	
	dataCache[plr] = {key = key, kills = kills, cash = cash, bank = bank, firstPlay = firstPlay}
	
	-- saves player data every 5 mins to prevent data loss
	task.spawn(function()
		while task.wait(300) do
			pcall(function()
				DSS:SetAsync(key, {kills.Value, cash.Value, bank.Value, firstPlay.Value})
			end)
		end
	end)
	
	-- on character added
	plr.CharacterAdded:Connect(function(char)
		char:WaitForChild("Humanoid").Died:Connect(function()
			local tag = char.Humanoid:FindFirstChild("creator")
			if tag and tag.Value then
				local ls = tag.Value:FindFirstChild("leaderstats")
				if ls then ls.Kills.Value += 1 end
			end
		end)
	end)
end)

-- on player removing
Players.PlayerRemoving:Connect(function(plr)
	local d = dataCache[plr]
	pcall(function()
		DSS:SetAsync(d.key, {d.kills.Value, d.cash.Value, d.bank.Value, d.firstPlay.Value})
	end)
	dataCache[plr] = nil
end)

-- saves every player's data on server shutdown
if not RS:IsStudio() then
	game:BindToClose(function()
		for _, plr in (Players:GetPlayers()) do
			task.spawn(function()
				local key, d = "id_"..plr.UserId, dataCache[plr]
				pcall(function()
					DSS:SetAsync(d.key, {d.kills.Value, d.cash.Value, d.bank.Value, d.firstPlay.Value})
				end)
			end)
		end
	end)
end```
2 Likes
19

So, I looked at your new code and noticed somethings you can do to make it cleaner. I recommend separating things into functions, and trying to attempt to get from datastores a few times before backing off.

So, I took some of my ideas and @GameMaster4268 to create this feel free to change it up for your project:

-- services
local DSS = game:GetService("DataStoreService"):GetDataStore("PlayerData")
local Players = game:GetService("Players")
local RS = game:GetService("RunService")

-- settings

local SAVE_INTERVAL = 300
local dataPrefix = "id_"

-- pcall trys
local maxTries = 5

-- define

local dataCache = {}

local TemplateData = {
	["leaderstats"] = {
		["Bounty"] = 0,
		["Kills"] = 0,
	},
	["playertats"] = {
		["Cash"] = 0,
		["Bank"] = 0
	},
	["playersettings"] = {
		["FirstPlay"] = false,
		["DataLoaded"] = false
	}
}

-- functions

local function LoadData(plr:Player, Data)
	
	if Data then
		
		-- Load Data
		
		for folderName, folderData in pairs(Data) do
			local folder = plr:FindFirstChild(folderName)
			if folder then
				for childName, Value in pairs(folderData) do
					local child = folder:FindFirstChild(childName)
					if child then
						child.Value = Value
					end
				end
			end
		end
		dataCache[plr.UserId] = Data
		
		print("Loaded "..plr.UserId.." Data!")
	else
		-- New player no need to load any data
		
		dataCache[plr.UserId] = TemplateData
		
		plr.playersettings.FirstPlay.Value = true
		
		print("Created New Data For "..plr.UserId.."!")
	end
	
	-- Set DataLoaded true
	plr.playersettings.DataLoaded.Value = true
end

local function SaveData(plr:Player)
	
	-- Check if data loaded
	
	local DataLoaded = plr:FindFirstChild("playersettings").DataLoaded
	
	if DataLoaded and DataLoaded.Value == false then
		warn("Player data not loaded! Trying to save!")
		return
	end
	
	local key = dataPrefix..plr.UserId
	
	local Data = {}
	
	-- Loop through the folders 
	
	for folderName, children in TemplateData do 
		local folder = plr:FindFirstChild(folderName)
		
		local folderData = {}
		
		if folder then
			for childName, _ in children do
				local child = folder:FindFirstChild(childName)
				if child then
					folderData[childName] = child.Value
				end
			end
		end
		Data[folderName] = folderData -- Add data to a dictionary
	end
	
	-- Save Data
	
	local success, err = pcall(function()
		DSS:SetAsync(key, Data)
	end)
	
	if not success then
		warn("Could not save "..plr.UserId.." data! Error:"..err)
	else
		print("Saved "..plr.UserId.." data!")
	end
end


-- on player added
Players.PlayerAdded:Connect(function(plr:Player)
	-- create instances
	for folderName, children in TemplateData do 
		
		local folderObject = Instance.new("Folder")
		folderObject.Name = folderName
		for childName, value in children do
			
			local typeOfValue = typeof(value) -- Type of value
			local childObject 
				
			if typeOfValue == "number" then
				childObject = Instance.new("NumberValue")
			elseif typeOfValue == "boolean"  then
				childObject = Instance.new("BoolValue")
			elseif typeOfValue == "string" then
				childObject = Instance.new("StringValue")
			end
			
			childObject.Name = childName
			childObject.Value = value
			childObject.Parent = folderObject
		end
		
		folderObject.Parent = plr
	end

	-- get playerdata
	local key, data = dataPrefix..plr.UserId, nil
	local success, err
	
	local trys = 0
	
	-- Atempt to get save data
	repeat
		success, err = pcall(function()
			data = DSS:GetAsync(key)
		end)
		trys += 1
	until success or trys == maxTries -- if success or reached max trys stop attempting to get data

	if success then
		LoadData(plr, data)
	elseif not success then
		plr:Kick("DataStoreService may be suffering connection difficulties at this moment, please try playing again later to avoid data loss.") -- replace with ui thing later
	end

	-- saves player data every 5 mins to prevent data loss
	task.spawn(function()
		while task.wait(SAVE_INTERVAL) do
			pcall(function()
				SaveData(plr)
			end)
		end
	end)

	-- on character added
	plr.CharacterAdded:Connect(function(char)
		char:WaitForChild("Humanoid").Died:Connect(function()
			local tag = char.Humanoid:FindFirstChild("creator")
			if tag and tag.Value then
				local ls = tag.Value:FindFirstChild("leaderstats")
				if ls then ls.Kills.Value += 1 end
			end
		end)
	end)
end)

-- on player removing
Players.PlayerRemoving:Connect(function(plr)
	SaveData(plr)
	dataCache[plr] = nil
end)

-- saves every player's data on server shutdown
if not RS:IsStudio() then
	game:BindToClose(function()
		for _, plr in (Players:GetPlayers()) do
			task.spawn(function()
				SaveData(plr)
				dataCache[plr] = nil
			end)
		end
	end)
end

By the way I tested it, and it works!

20

Also, to make it more organized put TemplateData in a module.