Feedback for my Factions System

Help and Feedback Code Review
#1

Sup everyone!

So I started working on a new project, on a factions system for my game, I normally just started scripting until it worked, but because this is quite “bigger” than my usual projects, I decided to ask here on the devforum, for optimizations and feedback for my code, and maybe I learn something new from it.

Code for loading and saving 
local PlayerData = {}
--[[local PlayerData = {
	Cash	= 0,
	Xp		= 0,
	Level	= 1,
	["Faction"]	={}
}]]
local LoadedFactions ={}
--[[local LoadedFactions ={

		--That's only how it's supposedly to look like--
		
	["Sentinel Clan"] = {
		012351233141,
		01235411233141,
	},
}]]


Players.PlayerAdded:Connect(function(Player)
	PlayerData[Player.UserId] = DataService.LoadPlayerData(Player)
	local factionName = PlayerData[Player.UserId]["Faction"].Name

	if factionName then
		-- Initialize faction table if it doesn't exist
		if not LoadedFactions[factionName] then
			LoadedFactions[factionName] = {}
		end

		-- Add player to faction
		if not table.find(LoadedFactions[factionName], Player.UserId) then
			table.insert(LoadedFactions[factionName], Player.UserId)
		end
	end

	Player.CharacterAdded:Once(function()
		ReplicatePlayerDataRE:FireClient(Player, PlayerData[Player.UserId])
	end)
end)
Players.PlayerRemoving:Connect(function(Player)
	DataService.SavePlayerData(Player, PlayerData[Player.UserId])
	local factionName = PlayerData[Player.UserId]["Faction"].Name

	-- Save player data

	if factionName and LoadedFactions[factionName] then
		-- Remove player from faction list
		local index = table.find(LoadedFactions[factionName], Player.UserId)
		if index then
			table.remove(LoadedFactions[factionName], index)
		end

		-- Check if the faction is empty and remove it
		if #LoadedFactions[factionName] <= 0 then
			LoadedFactions[factionName] = nil
		end
	end
	
	PlayerData[Player.UserId] = nil
end)
Datastore Modulescript 
local self = {}

self.Template = {
	Cash	= 0,
	Xp		= 0,
	Level	= 1,
	["Faction"]	={}
}

function self.LoadPlayerData(Player:Player)
	local userId = Player.UserId
	local success, data = pcall(function()
		return PlayerDataStore:GetAsync(tostring(userId))
	end)

	if success then
		if data then
			-- Merge the loaded data with the template
			for key, value in pairs(self.Template) do
				if data[key] == nil then
					data[key] = value
				end
			end
			return data
		else
			return self.Template
		end
	else
		Player:Kick("Failed to retrieve data for player " .. Player.Name .. ": " .. data)
	end
end

function self.SavePlayerData(Player:Player, Data)
	local userId = Player.UserId
	local success, error = pcall(function()
		return PlayerDataStore:SetAsync(tostring(userId), Data)
	end)
	if not success then
		warn("Failed to save data for player " .. Player.Name .. ": " .. error)
	end
end

The provided code snippet aims to when a player is loaded, and belongs to a faction, to retreive the data and save it on a table, on the server. I also provided the functions for Saving and Loading the PlayersData from the Datastore.

I’d really like to know, if there’s stuff to optimize and improve.

#2 
local dataModule = {}

dataModule.Template = {
	Cash = 0,
	Xp = 0,
	Level = 1,
	Faction	= {}
} --Remove unneeded formatting

function dataModule.LoadPlayerData(Player: Player)
	local userId = Player.UserId

	local success, data = pcall(function()
		return PlayerDataStore:GetAsync(tostring(userId))
	end)

    if not success then  --Guard statements to remove nesting
        Player:Kick("Failed to retrieve data for player " .. Player.Name .. ": " .. data)
        return 
    end

    if not data then return dataModule.Template end --Guard statements to remove nesting

    for key, value in pairs(dataModule.Template) do
        if not data[key] then -- Add a not instead of == nil
            data[key] = value
        end
    end

    return data
end

function dataModule.SavePlayerData(Player: Player, Data)
	local userId = Player.UserId

	local success, error = pcall(function()
		return PlayerDataStore:SetAsync(tostring(userId), Data)
	end)

	if not success then
		warn("Failed to save data for player " .. Player.Name .. ": " .. error)
	end
end

Mostly fine, just formatting and code neatness stuff.

Also, self is reserved for OOP. You’re not using that here. Don’t use self. Thanks.

2 Likes