How could I improve this code?

Help and Feedback Code Review
,
#1

I have found a video made a few years ago now and it shows you how to make a simulator UI effect. This is the code:

cp = 0

local player = game.Players.LocalPlayer

while wait() do
	if cp ~= player.leaderstats.Strength.Value then
		local random = math.random(1, 900)
		local xnew = random / 1000
		local new = game.ReplicatedStorage.ShowAdd:Clone()
		new.Parent = script.Parent.ScreenGui
		new.Position = UDim2.new(xnew, 0, 1, 0)
		new.Text = "+" .. player.leaderstats.Strength.Value
		cp = player.leaderstats.Strength.Value
	end
end

Yes, I know it is awfully organized and structured and there are multiple ways of improving this. I want to see what you guys have got to say on a better way of structuring this code and improving it.

1 Like
How would I make a simulator UI effect like this?
#2

Unless it has a problem, I wouldn’t recommend making any changes. I can point out several things I would change. It would still function the same, but become more complex and over engineered.

1 Like
#3

Ok. Could you show me the things you wanted to change?

#4

I’d swapout the loop structure with a changed event signal. With this approach, I’d probably want to make some kind of debounce system so that it doesn’t get flooded with changed events.

3 Likes
#5

Sounds great. I have a debounce system attached to my tool which gives the players cash every 2 seconds and this onscreen UI effects code fires when the leaderstats gets updated so it will have a 2 second delay. So I wouldn’t need to add a debounce system?

#6

Yeah, I don’t think a debounce system is necessary in this case.

1 Like
#7

you should remove the text label when your done with it

local cp = 0
local player = game:GetService("Players").LocalPlayer

player.leaderstats.Strength:GetPropertyChangedSignal("Value"):Connect(function()
	if cp ~= player.leaderstats.Strength.Value then
		local random = math.random(1, 900)
		local xnew = random / 1000
		local new = game.ReplicatedStorage.ShowAdd:Clone()
		new.Parent = script.Parent.ScreenGui
		new.Position = UDim2.new(xnew, 0, 1, 0)
		new.Text = "+" .. player.leaderstats.Strength.Value
		cp = player.leaderstats.Strength.Value
        task.wait(1)
        new:Destroy()
	end
end)
3 Likes
#8

Use heartbeat instead. RunService.Heartbeat:Connect(Function()

end)

2 Likes