Custom PlayerData Module Advice

Help and Feedback Code Review
#1
  • What does the code do and what are you not satisfied with?
    This is a module that stores all player data for players in a server
  • What potential improvements have you considered?
    Looking here for advice or any recommendations
  • How (specifically) do you want to improve the code?
    To reduce the amount of redundant functions
local Players = game:GetService("Players")
local ServerStorage = game:GetService("ServerStorage")

local playerData = {}

ServerPlayerProfiles = {}
StarterProfile = {
	["General_Data"] = {
		["UserId"] = 0,
		["Current_Username"] = "_",
		["Current_Displayname"] = "_",
	},
	["Game_Data"] = {
		["Level"] = 1,
		["EXP"] = 0,
		["Gold"] = 0
	},
	["Weapons"] = {},
	["Items"] = {}
}

function playerData.GetProfiles()
	return ServerPlayerProfiles
end

function playerData.GetPlayerProfile(playerId)
	if type(playerId) == 'number' then
		if ServerPlayerProfiles[playerId] then
			return ServerPlayerProfiles[playerId]
		else 
			warn("Player does not exist in data base.")
			return nil
		end
	else
		warn("GetPlayerProfile argument must be a integer.")
		return nil
	end
end

function playerData.UpdateGameStats(playerId, stat, change, option)
	if option == "Add" then
		ServerPlayerProfiles[playerId]["Game_Data"][stat] = ServerPlayerProfiles[playerId]["Game_Data"][stat] + change
	else if option == "Update" then
		ServerPlayerProfiles[playerId]["Game_Data"][stat] = change
	else if option == "Subtract" then
		ServerPlayerProfiles[playerId]["Game_Data"][stat] = ServerPlayerProfiles[playerId]["Game_Data"][stat] - change
	else
		warn("Error with option argument")
	end
	end
	end
end

function GiveWeapon(player, newWeapon)
	local weaponExists = ServerStorage.Weapons:FindFirstChild(newWeapon)
	if weaponExists then 
		local clonedWeapon = weaponExists:Clone()
		clonedWeapon.Parent = player.StarterGear
		ServerPlayerProfiles[player.UserId]["Weapons"][clonedWeapon.Name] = clonedWeapon
	end
end

function InitializePlayer(player)
	ServerPlayerProfiles[player.UserId] = StarterProfile;
	ServerPlayerProfiles[player.UserId]["General_Data"]["UserId"] = player.UserId;
	ServerPlayerProfiles[player.UserId]["General_Data"]["Current_Username"] = player.Name;
	ServerPlayerProfiles[player.UserId]["General_Data"]["Current_Displayname"] = player.DisplayName;
	GiveWeapon(player, "ClassicSword")
end

local function OnPlayerAdded(player)
	InitializePlayer(player)
end

Players.PlayerAdded:Connect(OnPlayerAdded)

return playerData
#2

I’m totally not late at all.
You have some return nil that are unnecessary, but can be nice to read, so it’s redundant but also personal preference.
The else if should be elseif instead, so you dont need the weird end end end

#3

Wanted to add onto what odin said.

  1. Within InitializePlayer you can create a local variable to make it more readable. local playerProfile = ServerPlayerProfiles[player.UserId]. This approach can be applied to UpdateGameStats as well.
  2. For the OnPlayerAdded function, you can instead just place the contents of InitializePlayer into it and remove InitializePlayer. However if you plan on using InitializePlayer in other locations then disregard this suggestion.