How To Optimize Script?

I’ve made this script which works perfectly as it should but since I’m no pro I was wondering is there anything that can be improved or optimized?

local Parent = script.Parent
local Start = Parent.Start

local SignalStrenght = Parent.NumberImage.Signals.Strenght

local Upgrades = Parent.Upgrades
local IncomeValue = Parent.NumberImage.IncomeDisplay.Available
local LastRate = Parent.NumberImage.RateNum.LastRate

-- Other
local DB = false

Start.MouseButton1Click:Connect(function()
	
	if Upgrades.Ascension.Value == true and DB == false then DB = true
		
		task.wait()
		
		while Start["On/Off"].Value == true do
			
			if SignalStrenght.Value == 2 then
				
				local EntryIncome = math.random(2,10)/100

				IncomeValue.Value += EntryIncome
				LastRate.Value = EntryIncome
				
			elseif SignalStrenght.Value == 3 then
				
				local EntryIncome = math.random(16,26)/100

				IncomeValue.Value += EntryIncome
				LastRate.Value = EntryIncome
				
			elseif SignalStrenght.Value == 4 then
				
				local EntryIncome = math.random(20,30)/100

				IncomeValue.Value += EntryIncome
				LastRate.Value = EntryIncome
				
			end
			
			DB = false
			wait(5)
			
		end
	end
end)
1 Like

Ah, I would not use the while loop in a MouseButton1Click statement, as if they spam the button, they will also spam the while loop, and have it come in multiple times, causing a crash to occur, or a leak. This is a bad practice, and I would change it right away, I would also utilize a function, so you could just simply call said Function() after the MouseButton1Click statement, which would make your life a lot easier, incase you need to refer to this function later in your code.

But please do not use the while statement in the MouseButton1Click. Find another method for the code.

1 Like

Type Annotations. Helps you read code more easily and catch errors early.


local EntryIncome
	--[[Setting up this variable early and then defining it/clearing it later
	can save processing power and reduce some redundancy.]]

Start.MouseButton1Click:Connect(function()
	
	if Upgrades.Ascension.Value == false or DB == true then return end

	--[[in this case, you have the option of returning out of the function early,
	which will stop the game from trying to read any further and thus saving resources.]]

	DB = true

		
	while DB do
	--[[once this gets set to false near the end,
	the loop will automatically break on completion]]

		task.wait()
			
		if SignalStrength.Value == 2 then
				
			EntryIncome = math.random(2,10)/100 -- You only have to set this in the while loop.
				
		elseif SignalStrength.Value == 3 then
				
			EntryIncome = math.random(16,26)/100
				
		elseif SignalStrength.Value == 4 then
				
			EntryIncome = math.random(20,30)/100
			
		end

		IncomeValue.Value += EntryIncome
		LastRate.Value = EntryIncome

		if Upgrades.Ascenscion.Value == true then continue end -- This makes sure not to break the loop while the given parameter is true

		EntryIncome = nil
		
		DB = false
		wait(5)
		
	end
end)

Not necessarily, it simply depends on your syntax.

You can return out of the function before the game tries reading further into it and reaching the while loop. Refer to my other reply to OP

1 Like

Still a bad and not needed practice, you can easily achieve the same outcome without a while loop, I strongly advise against using a while loop in an event, and only using it for when absolutely necessary ( for example, a sword damaging an item per x amount of seconds )

I appreciate you putting your time into this, I will now try to understand the script, perhaps I could improve my other scripts with this.

I think that issue eliminates when you put debounce right?

Thank you, much appreciated, I will check into this.