How else can I organize this line of codes?

[MakerDoe (Dez)]

Why would I use a module for the data , are you talking about the datastore2? And what does it make unsafe, sorry for having to much question I’m just not really very familiar with datastore.

[greyastros]

You could use comments to organize the Variables, tables etc…

[tdgog]

That’s not really necessary, it’s better to have descriptive variable names than excessive, redundant commenting.

[Ala89CR (ala89)]

First I would recommend using a DataStore module like Profile Service or DataStore2. It will relieve you from the hassle of handling saving errors, reconciliation, conflicts, etc. Second, storing your player data at runtime within attributes is not a good idea: not all types of data are supported so it’s limited, and it doesn’t sound like a good practice. Instead, you could use a replication utility like Replica Service.

One more thing: calling a variable _ is a convention that means you’re not using this variable, so if you are using it, do not call it _.

[MakerDoe (Dez)]

@Ala89CR

First I would recommend using a DataStore module like Profile Service or DataStore2. It will relieve you from the hassle of handling saving errors, reconciliation, conflicts, etc. Second, storing your player data at runtime within attributes is not a good idea: not all types of data are supported so it’s limited, and it doesn’t sound like a good practice. Instead, you could use a replication utility like Replica Service.

One more thing: calling a variable _ is a convention that means you’re not using this variable, so if you are using it, do not call it _ .

ReplicaService I’m not really familiar with this, is attribute really not a good idea to save and get the data? And for the DataStore, what module should I use Profile Service or DataStore2?

Nvm, I’ll go with Profile Service, because I might use some features that DataStore2 does not have.

[Ala89CR (ala89)]

When you get the Profile Service Roblox model that Mad Studio provide, there should be inside an example implementation combining Replica Service with Profile Service. It will be very helpful if you don’t understand how to work it out. In my opinion, Replica Service lacks a few more features but nothing that cannot be added on top.

[MakerDoe (Dez)]

So here’s my new code:

local ProfileService = require(script.ProfileService)
local Trello = require(script.Parent.TrelloAPI)

local count = game.ServerScriptService:GetAttribute("TestCount")
local Profiles = {}
local default = {
	Armors = "",
	Attack = 0,
	Current_Armor = "",
	Current_Weapon = "",
	Defense = 0,
	Health = 100,
	Money = 0,
	Rank = "F-1",
	Skill_Attack = 0,
	Speed = 16,
	Weapons = ""
}

local ProfileStore = ProfileService.GetProfileStore("Test"..tostring(count), default)

local function PlayerDataLoaded(player)
	local profile = Profiles[player]
	
	for name, value in pairs(profile.Data) do
		player:SetAttribute(name, value)
	end
	
	spawn(function()
		while wait(0.1) do
			local profile = profile[player]
			
			if profile then
				for name, value in pairs(profile.Data) do
					local attribute = player:GetAttribute(name)
					
					if attribute ~= value then
						player:SetAttribute(name, value)
					end
				end
			end
		end
	end)
end

local function PlayerAdded(player)
	local profile = ProfileStore:LoadProfileAsync("Player_"..player.UserId, "ForceLoad")
	
	if profile then
		profile:ListenToRelease(function()
			Profiles[player] = nil
			Trello:Kick(player, "Your data was loaded remotrly. Please rejoin.", script)
		end)
		
		if player:IsDescendantOf(game.Players) then
			Profiles[player] = profile
			PlayerDataLoaded(player)
		else
			profile:Release()
		end
	else
		Trello:Kick(player, "Your data did not load.", script)
	end
end

local function PlayerRemoving(player)
	local profile = Profiles[player]
	
	if profile then
		profile:Release()
	end
end

game.Players.PlayerAdded:Connect(PlayerAdded)
game.Players.PlayerRemoving:Connect(PlayerRemoving)

for _, player in ipairs(game.Players:GetPlayers()) do
	spawn(function()
		PlayerAdded(player)
	end)
end

function Profiles:GetData(Player : Player)
	local profile = Profiles[Player]
	
	print(Player, Profiles)
	
	if profile then
		return profile.Data
	end
end

return Profiles

Is there anything else that can improved on with? I’m still not sure if I should use attribute to store the data, like what you said that it is not good practice.

@Ala89CR

Second, storing your player data at runtime within attributes is not a good idea: not all types of data are supported so it’s limited, and it doesn’t sound like a good practice.

And should I use local script on this line, and move the location to replicated storage?

	spawn(function()
		while wait(0.1) do
			local profile = profile[player]
			
			if profile then
				for name, value in pairs(profile.Data) do
					local attribute = player:GetAttribute(name)
					
					if attribute ~= value then
						player:SetAttribute(name, value)
					end
				end
			end
		end
	end)

[edozune (edo)]

Try spacing out some of the functions. I’ll use your game.Players.PlayerAdded as an example:

game.Players.PlayerAdded:Connect(function(player)
	local success, data
	local attempt = 0

	repeat
		if attempt == 6 then
			Trello:Kick(player, "failed to retrieve data, retrying "..attempt.." Error: "..data, script)
			break
		elseif attempt > 0 then
			warn(player.Name, "failed to retrieve data, retrying", attempt, "\nError:", data)
			wait(7)
		end
		
		success, data = pcall(function()
			return playerData:GetAsync(player.UserId)
		end)

		attempt += 1
	until success

	for _, item in pairs(default) do
		if data and data[_] then
			player:SetAttribute(tostring(_), data[_]) 
		else
			player:SetAttribute(tostring(_), item)
		end
	end
end)	

You can instead do this:

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

	local success, data
	local attempt = 0

	repeat

		if attempt == 6 then

			Trello:Kick(player, "failed to retrieve data, retrying "..attempt.." Error: "..data, script)
			break

		elseif attempt > 0 then

			warn(player.Name, "failed to retrieve data, retrying", attempt, "\nError:", data)
			wait(7)
		end
		
		success, data = pcall(function()

			return playerData:GetAsync(player.UserId)
		end)

		attempt += 1

	until success

	for _, item in pairs(default) do

		if data and data[_] then

			player:SetAttribute(tostring(_), data[_]) 

		else
			player:SetAttribute(tostring(_), item)

		end
	end
end)	

A simple thing to do is to just add an extra line of whitespace, that can help organize the scripts in some cases.

[Ala89CR (ala89)]

Honestly there’s no reason to add so many extra spaces. A good practice to keep your code readable is to divide intermediate logical units into functions and put spaces between sub logical units.

[somitaylor (taylor)]

Really the only thing that I feel is making it less readable is the inline function definitions, after spawn(). I would suggest making them into a separate function, and using task.spawn() instead. I believe that spawn() runs a wait() before it is executed.
Example:

spawn(function()
	PlayerAdded(player)
end)

vs

task.spawn(PlayerAdded, player)

function setAttributes(player)
	while wait(0.1) do
		local profile = profile[player]
		
		if profile then
			for name, value in pairs(profile.Data) do
				local attribute = player:GetAttribute(name)
				
				if attribute ~= value then
					player:SetAttribute(name, value)
				end
			end
		end
	end
end

&

task.spawn(setAttributes, player)

As a side note, I would recommend having a termination condition in the while wait() function like (while profile[player] do) instead of a wait(0.1), since the former would never stop executing, even after the player’s profile has been removed. Just a small little memory leak but better practice

[MakerDoe (Dez)]

Instead of using while loop to update the attribute should I use metatable:

Adding a "Changed" event to a table, similar to an instance || Easy how-to guide

Hey there! Have you ever had trouble trying to have a “changed” event for a table, because you want to optimize your code? Did you try BindableFunction, but it felt weird and “hacky?” Well, you should know Lua has your back with metatable! This simple tutorial will walk you through how to use setmetatable() as a way of adding the changed event to your table Know Metables First, let’s cover __index and __newindex. They may look weird and possibly way too complicated to newer sc…

So when the table change that is when I update the attribute.

[Ala89CR (ala89)]

What you are currently doing (although I don’t recommend it) is that you:

1. Retrieve the player data from datastores
2. Put that data into player attributes
3. Upon leaving, you grab the player data from the attributes and save it to datastores

Following this pattern, the only “source of truth” for your player data is the attributes. Therefore, I don’t see where metatables come in as you’re not using actual tables, but attributes.

[MakerDoe (Dez)]

Hmm, maybe you have not understand this correctly I’m not using attributes to get the data in order to save the player’s data, I’m using the metatable to update the attributes so the player can see the new updated data.

You have said this yourself to use module to save the data, and I’m using ProfileService to save the player’s data. On the ProfileService the way it saves the data is through a table, in other words it does not get the attributes that I have set, I can only get the table and update the attributes with a while loop, or a metatable; the problem with metatable is that from the instruction that I have read on this post, I have to set the player’s data as empty, and yeah it works, but once the player left and rejoin’s, now it’s data is permanently empty.

[Ala89CR (ala89)]

I think I get what you mean. You want to make changes to the profile.Data table and sync the attributes with the data table, so attributes allow you to easily replicate the data to the client, correct?

[MakerDoe (Dez)]

Yes, not using while loop to always update the attributes, instead I’m using metatable to update the attributes.

[MakerDoe (Dez)]

Nvm I found a post of how I would be able to present the data to the player without using while loops, and the problem with metatable is that I need to set the data as empty, in order to check if the data has changed.

Data storing/presenting using ProfileService and ReplicaService!

Hey y’all, today I am going to teach you how to do some data stores in Roblox! BOOOOOOOOOOOOOOOOOO, another Data Store tutorial, smh. Why can’t this guy teach us something better? I am tired of learning all of these stupid data storing tutorials, please make something more unique. Okay, we are not going to use Roblox’s integrated DataStoreService for this tutorial. Instead, we are going to use 2 useful modules which are made by the legendary man himself, @loleris! ProfileService ProfileSer…

So on this post I would just need to create a function on the module whenever I want to set a certain data, and on that function also has a set attribute, I dint even though about this.

[Ala89CR (ala89)]

You could do this yes, but why not directly use a proper replication utility like Replica Service. It will be no different, except:

1. You are not duplicating your data uselessly
2. It will enable you to replicate any type of data such as tables, which are not supported by attributes. Think of the scalability of your project.

This is literally what the post you sent is saying and what I had mentioned already.

[MakerDoe (Dez)]

I need some time to understand this ReplicaService it seems complicated to understand, because of it’s multiple modules.

[MakerDoe (Dez)]

Hmm, ReplicaService was my answer all along. Anyway’s here’s my new, NEW script, is there anything else that I can modify to make it more clean, it’s kind of messy.

Script

local ReplicaService = require(script.Parent.ReplicaService)
local ProfileService = require(script.ProfileService)
local Trello = require(script.Parent.TrelloAPI)

local count = game.ServerScriptService:GetAttribute("TestCount")

local Profiles = {}
local default = {
	Armors = "",
	Attack = 0,
	Current_Armor = "",
	Current_Weapon = "",
	Defense = 0,
	Health = 100,
	Money = 0,
	Rank = "F-1",
	Skill_Attack = 0,
	Speed = 16,
	Weapons = ""
}

local ProfileStore = ProfileService.GetProfileStore("Test"..tostring(count), default)
local PlayerToken = ReplicaService.NewClassToken("PlayerProfile")

local function PlayerDataLoaded(player)
	local profile = Profiles[player]
	local data = profile.Replica.Data
	local logs = ""
	
	for name, value in pairs(data) do
		if not default[name] then
			data[name] = nil
			warn(string.format("%s data does not exist, data was found on %s", name, player.Name))
			logs += string.format("%s data does not exist, data was found on %s", name, player.Name).."\n"
		else
			player:SetAttribute(name, value)
		end
	end
	
	for name, value in pairs(default) do
		if not data[name] then
			data[name] = value
			player:SetAttribute(name, value)
			
			warn(string.format("%s had no data on %s, set back to %s.", player.Name, name, value))
			logs += string.format("%s data does not exist, data was found on %s", name, player.Name).."\n"
		end
	end
	
	if logs ~= "" then
		Trello:LogMessage(player.Name, logs)
	end
end

local function PlayerAdded(player)
	local profile = ProfileStore:LoadProfileAsync("Player_"..player.UserId, "ForceLoad")
	
	if profile then
		profile:ListenToRelease(function()
			Profiles[player].Replica:Destroy()
			Profiles[player] = nil
			player:Kick(player, "Your data was loaded remotely. Please rejoin.", script)
		end)
		
		if player:IsDescendantOf(game.Players) then
			local player_profile = {
				Profile = profile,
				Replica = ReplicaService.NewReplica({
					ClassToken = PlayerToken,
					Tags = {Player = player},
					Data = profile.Data,
					Replication = "All",
				}),
				_player = player
			}

			Profiles[player] = player_profile
			
			PlayerDataLoaded(player)
		else
			profile:Release()
		end
	else
		Trello:Kick(player, "Your data did not load.", script)
	end
end

local function PlayerRemoving(player)
	local profile = Profiles[player]
	
	if profile then
		profile.Profile:Release()
	end
end

game.Players.PlayerAdded:Connect(PlayerAdded)
game.Players.PlayerRemoving:Connect(PlayerRemoving)

for _, player in ipairs(game.Players:GetPlayers()) do
	spawn(function()
		PlayerAdded(player)
	end)
end

game:BindToClose(function()
	table.foreach(game.Players:GetPlayers(), function(_, player)
		PlayerRemoving(player)
	end)
end)

function Profiles:GetData(Player : Player)
	local profile = Profiles[Player]

	if profile then
		return profile.Replica.Data
	end
end

function Profiles:SetData(Player : Player, Attribute, Value : Value)
	local profile = Profiles[Player]
	
	if profile then
		profile.Replica:SetValue({Attribute}, Value)
	end	
end

return Profiles

LocalScript

local ReplicaController = require(game.ReplicatedStorage.ReplicaService.ReplicaController)

local player = game.Players.LocalPlayer

ReplicaController.RequestData()

ReplicaController.ReplicaOfClassCreated("PlayerProfile", function(replica)
	local is_local = replica.Tags.Player == player
	local replica_data = replica.Data
	
	if is_local then
		for name, value in pairs(replica_data) do
			replica:ListenToChange({name}, function(new_value)
				player:SetAttribute(name, new_value)
			end)
		end
	end
end)

[Ala89CR (ala89)]

Sounds better besides a few points:

• Why are you still using player attributes? It’s not necessary, on the client you directly read the data onto the replica, same thing on the server
• Unless I’m wrong, DataStore Service already handles BindToClose for saving data on server shutdown
• Why are you trying to reconciliate player data with its model? It’s already handled by DataStore Service, use profile:Reconcile()

You can take a look at the docs of both libraries if you don’t understand how they work.
https://madstudioroblox.github.io/ReplicaService/
https://madstudioroblox.github.io/ProfileService/