Can I get some feedback on a quick leveling system I made?

Help and Feedback Code Review
#1

Hi, I made a quick leveling system, could I get some feedback on how I am going about it, and the way I am writing this as a whole? Thanks in advance.

local function apply(Element,Properties)
	if Element and Properties then
		for _,v in pairs(Properties) do
			Element[_] = v
		end
	end
end

local function new(ClassName,Name,Properties)
	local Element = Instance.new(ClassName)
	Element.Name = Name or ClassName
	if Properties then apply(Element,Properties) end
	return Element
end

local replicatedstorage = game:GetService("ReplicatedStorage")
local RemoteEvent = replicatedstorage:FindFirstChild("RemoteEvent") or new("RemoteEvent","RemoteEvent",{Parent = replicatedstorage})

local funcs = {}
funcs["Kill"] = function(player,...)
	local leaderstats = player:WaitForChild("leaderstats")
	local kills = leaderstats:WaitForChild("Kills")
	kills.Value += 1
end

RemoteEvent.OnServerEvent:Connect(function(player,msg,...)
	if funcs[msg] then
		funcs[msg](player,...)
	end
end)

local function PlayerAdded(player)
	local leaderstats = new("Folder","leaderstats")
	local level = new("NumberValue","Level",
		{
			Parent = leaderstats,
			Value = 0
		}
	)
	
	local function getXpNeeded(lvl)
		return lvl * 50
	end
	
	local function leveledUp()
		level.Value += 1
		level:SetAttribute("ExpNeeded",getXpNeeded(level.Value))
	end
	
	local function changed()
		local diff = level:GetAttribute("Exp") - level:GetAttribute("ExpNeeded")
		if diff >= 0 then
			task.wait(0.1)
			leveledUp()
			level:SetAttribute("Exp",diff)
			changed()
		end
	end
	
	level.Value = 1
	level:SetAttribute("Exp",0)
	level:SetAttribute("ExpNeeded",getXpNeeded(level.Value))
	
	level:GetAttributeChangedSignal("Exp"):Connect(changed)
	
	local kills = new("NumberValue","Kills",
		{
			Parent = leaderstats
		}
	)
	
	kills:GetPropertyChangedSignal("Value"):Connect(function()
		level:SetAttribute("Exp",level:GetAttribute("Exp")+15)
	end)
	
	leaderstats.Parent = player
end

local function PlayerRemoved(player)
	
end

local function Shutdown()
	for _,v in pairs(game.Players:GetPlayers()) do
		PlayerRemoved(v)
	end
end

for _,v in pairs(game.Players:GetPlayers()) do
	PlayerAdded(v)
end

game.Players.PlayerAdded:Connect(PlayerAdded)
game.Players.PlayerRemoving:Connect(PlayerRemoved)
game:BindToClose(Shutdown)
1 Like
#2

I’m just gonna throw some basic suggestions out there for you, hopefully it’s helpful to some extent.

  • Maybe try to be more consistent with your identifier naming conventions. I notice in some of your function identifiers they’re all lowercased, while others have the first letter capitalized.

  • Try to give your functions/variables more meaningful names so it’s easier to identify what their purposes are. Like maybe for your new function, you could try renaming it as createInstance for example.

  • In your for loop in the apply function, generally “_” is used to infer that an identifier is not going to be used, nor is it needed. Since you actually use it, you should give it a more meaningful name. i is commonly used, as it represents the index in the current iteration.

You also probably don’t have to nest all your functions that handle updating the player’s XP and level, might be better to leave them in the global scope.

Just a few things I put together quickly. Keep up the great work. :+1:

1 Like
#3

thank you for the suggestions!
i nested the functions so i wouldnt have to send parameter everytime, just something to help my unorganized mind do things faster.

1 Like