Just created an XP system. Would like some constructive feedback

Help and Feedback Code Review
1

Currently works well as it is, though there is a bit of a delay when the text is changing on the UI meter. But for the most part, it works.

Screen Shot 2021-01-23 at 5.24.16 PM
Screen Shot 2021-01-23 at 5.24.16 PM1434×1324 318 KB

I had make this compatible with NameLabel I would be providing players in my game, so that’s what UI is.

Main script in ServerScriptService:

local RS = game:GetService("ReplicatedStorage")
local UI = RS:FindFirstChild("Nametag_UI")

local Players = game:GetService("Players")

Players.PlayerAdded:Connect(function(player)

	local leaderstats = Instance.new("Folder")
	leaderstats.Name = "leaderstats"
	leaderstats.Parent = player

	local level = Instance.new("NumberValue")
	level.Name = "Level"
	level.Value = 1
	level.Parent = leaderstats

	local Xp = Instance.new("NumberValue")
	Xp.Name = "xp"
	Xp.Value = 0
	Xp.Parent = level

	local XpMax = Instance.new("NumberValue")
	XpMax.Name = "xpMax"
	XpMax.Value = 100
	XpMax.Parent = level

	player.CharacterAdded:Connect(function(character)
		UI:Clone().Parent = player.Character:FindFirstChild("Torso")	
		player.Character.Torso.Nametag_UI.NameLabel.Text = "Lvl " .. level.Value .. " | " .. player.Name
	end)

	Xp.Changed:Connect(function()
		if Xp.Value >= XpMax.Value then
			level.Value += 1
			XpMax.Value += 100
			Xp.Value = 0

			player.Character.Torso.Nametag_UI.NameLabel.Text = "Lvl " .. level.Value .. " | " .. player.Name

		end
	end)
end)

LocalScript for the text in the meter below:

local player = game:GetService("Players").LocalPlayer
local xp = player:WaitForChild("leaderstats").Level.xp
local xpMax = player:WaitForChild("leaderstats").Level.xpMax

while wait(.5) do
    script.Parent.Text = "XP " .. xp.Value .. " / " .. xpMax.Value
end

LocalScript in the meter to help move the UI:

local player = game:GetService("Players").LocalPlayer
local xp = player:WaitForChild("leaderstats").Level.xp
local xpMax = player:WaitForChild("leaderstats").Level.xpMax

xp.Changed:Connect(function()
    script.Parent.Size = UDim2.new(xp.Value/xpMax.Value,0,1,0)
end)

I think I got the barebones down, though, I would like to know how I can make this more efficient.

3 Likes
2

I don’t know how well this will get you in efficiency, but if you want to make your code shorter you could go ahead and create the instances you are creating with Instance.new() and put them in a directory your script can access easily and as fast as it can.

2 Likes
3

All the other code looks fine to me but for the Local Script for the meter needs some improvements.
First, it’s running in a infinite loop every half of a second just to check if the value changes. This will slow the game’s performance.

Here is a example of how it should look like:

local player = game:GetService("Players").LocalPlayer
local xp = player:WaitForChild("leaderstats").Level.xp
local xpMax = player:WaitForChild("leaderstats").Level.xpMax

xpMax.Changed:Connect(function()
    script.Parent.Text = "XP " .. xp.Value .. " / " .. xpMax.Value
end)
3 Likes
4

Looks fine, but in my preference, I would use tables for data, which look way cleaner and nicer, apart from that everything seems fine ^ as said above that would be the only problem.

3 Likes
5

How would I be able to incorporate that into why what I’m doing right now?

I don’t think I’ve ever done this with tables before, though I am curious.

1 Like
6

I recommend do this for readability.

local XPFormat = "XP %s / %s"

script.Parent.Text = string.format(XPFormat, xp.Value, xpMax.Value)
2 Likes
7

While loops are a waste of resources.

Change to:

xp:GetPropertyChangedSignal("Value"):Connect(funciton()
    -- this code run when xp's value property change
    script.Parent.Text = "XP " .. xp.Value .. " / " .. xpMax.Value
end)
2 Likes
8

As mentioned above, the delay is from the while wait(0.5) do part.

I would recommend using a simple exponential experience curve. Currently, the linear curve of progression adds 100 to the max needed, so at level 1, you need 100 points for level 2 and at level 10, you would need 1000 points for level 11. This means that as you progress, you will be earning the same amount of experience (maybe slightly more) per action as when you first started. It is completely optional, but gives players more feeling of scaling and gaining power over time imo. A basic one is LevelLevel100, so you would need 100 at level 1, but 10,000 at level 10 for example. You can also do this where 10,000 is the experience needed to get from level 1 to 10 (explained below)

Also, you should take into account remainder xp. If you are at 95 XP and do something that rewards you with 20 XP, you will level up and the left over 15 xp will go to waste. Instead of resetting xp to 0, you could subtract it from the old xpmax value and make that the new xp value.

For mine, I find out total experience needed for the next level. So level 5 would be the experience required to get to level 2 + level 3 + level 4 + level 5. I will then subtract the total experience required to get to level 4 and the result is xp needed to get from level 4 to 5.

edit:
Sorry it’s late and I forgot to provide an example:

local x = level*level*100 --amount of total xp needed
local  xpneeded = x-((level-1)*(level-1)*100)
2 Likes
9

To add onto what @devaleporbet said, you can use tables instead. You can also use Roblox’s new beta feature, attributes, which are pretty good for this too. Tables can be implemented through making a module script which only contains a table that it returns. You can put it under the player, this way allowing for any script to access it, require it, and read / write its data. On the other hand, attributes can be done through adding them on player join and then accessing them through the player and read and write them easily.

2 Likes