Code Optimization

Help and Feedback Code Review
1

Technically, this code works fine. But it’s really ugly to look at and I feel like it can be more optimized, as well as run off on a more dynamic number system. Any ideas or pointers would be amazing! Thank you

local RefillEvent = game.ReplicatedStorage.Refill

local fill1 = script.Parent:WaitForChild('fill1')
local fill2 = script.Parent:WaitForChild('fill2')
local fill3 = script.Parent:WaitForChild('fill3')
local fill4 = script.Parent:WaitForChild('fill4')
local fill5 = script.Parent:WaitForChild('fill5')
local fill6 = script.Parent:WaitForChild('fill6')
local fill7 = script.Parent:WaitForChild('fill7')
local FillVal = script.Parent.FillVal


-- This is some real impractical yandere dev code
FillVal.Changed:connect(function()
	if FillVal.Value <= 0 then
		FillVal.Value = 0
		fill1.Transparency = 1
		fill2.Transparency = 1
		fill3.Transparency = 1
		fill4.Transparency = 1
		fill5.Transparency = 1
		fill6.Transparency = 1
		fill7.Transparency = 1
	elseif FillVal.Value == 1 then
		fill1.Transparency = 0
		fill2.Transparency = 1
		fill3.Transparency = 1
		fill4.Transparency = 1
		fill5.Transparency = 1
		fill6.Transparency = 1
		fill7.Transparency = 1
	elseif FillVal.Value == 2 then
		fill1.Transparency = 1
		fill2.Transparency = 0
		fill3.Transparency = 1
		fill4.Transparency = 1
		fill5.Transparency = 1
		fill6.Transparency = 1
		fill7.Transparency = 1
	elseif FillVal.Value == 3 then
		fill1.Transparency = 1
		fill2.Transparency = 1
		fill3.Transparency = 0
		fill4.Transparency = 1
		fill5.Transparency = 1
		fill6.Transparency = 1
		fill7.Transparency = 1
	elseif FillVal.Value == 4 then
		fill1.Transparency = 1
		fill2.Transparency = 1
		fill3.Transparency = 1
		fill4.Transparency = 0
		fill5.Transparency = 1
		fill6.Transparency = 1
		fill7.Transparency = 1
	elseif FillVal.Value == 5 then
		fill1.Transparency = 1
		fill2.Transparency = 1
		fill3.Transparency = 1
		fill4.Transparency = 1
		fill5.Transparency = 0
		fill6.Transparency = 1
		fill7.Transparency = 1
	elseif FillVal.Value == 6 then
		fill1.Transparency = 1
		fill2.Transparency = 1
		fill3.Transparency = 1
		fill4.Transparency = 1
		fill5.Transparency = 1
		fill6.Transparency = 0
		fill7.Transparency = 1
	elseif FillVal.Value == 7 then
		fill1.Transparency = 1
		fill2.Transparency = 1
		fill3.Transparency = 1
		fill4.Transparency = 1
		fill5.Transparency = 1
		fill6.Transparency = 1
		fill7.Transparency = 0
	elseif FillVal.Value > 7 then
		FillVal.Value = 7
	end
end)

Prompt.Triggered:Connect(function(player)
	if player.Character:FindFirstChildOfClass('Tool') then
		local Tool = player.Character:FindFirstChildOfClass('Tool')
		if Tool:GetAttribute("Type") == "Water" then
			RefillEvent:FireClient(player)
			Prompt.Parent.Drip.PlaybackSpeed = (math.random(1,1.3))
			Prompt.Parent.Drip:Play()
		end
	end
end)
2

I think your Changed event can be changed to this, and shoudl still work the same

local fillParts = {}

for _, fill in ipairs(script.Parent:GetChildren()) do
	if not fill.Name:match("fill") then
		continue
	end
	table.insert(fillParts, fill)
end

FillVal.Changed:Connect(function()
	if FillVal.Value > #fillParts then
		FillVal.Value = #fillParts
		return
	end

	-- Set all fill parts to be invisible
	for _, fill in ipairs(fillParts) do
		fill.Transparency = 1
	end
	
	-- Stop here if FillVal is less or equal to 0
	if FillVal.Value <= 0 then
		FillVal.Value = 0
		return
	end
	
	-- Get the part with the value and make part visible
	script.Parent["fill" .. FillVal.Value].Transparency = 0
end)

@Judgy_Oreo Thank you for pointing out the mistake I did

We store the fillparts immediately in a table, then we first check in the changed event if our fillVal is greater than how many parts we have, if yes we changed the value and stop there. Then we make them all invisible, we stop there if value is less or equal to 0, finally we set that one specific part to be visible with the value

3 Likes
3

I think that’s just about right, but my implementation is wrong. Upon testing I’m receiving the error “attempt to compare number < instance” on line 16

local RefillEvent = game.ReplicatedStorage.Refill
local Charge = 4
local FillVal = Prompt.FillVal

local fillParts = {}

for _, fill in ipairs(Prompt:GetChildren()) do
	if not fill.Name:match("fill") then
		continue
	end
	table.insert(fillParts, fill)
end

FillVal.Changed:Connect(function()
	if FillVal > #fillParts then
		FillVal.Value = #fillParts
		return
	end

	-- Set all fill parts to be invisible
	for _, fill in ipairs(fillParts) do
		fill.Transparency = 1
	end

	-- Stop here if FillVal is less or equal to 0
	if FillVal.Value <= 0 then
		FillVal.Value = 0
		return
	end

	-- Get the part with the value and make part visible
	Prompt["fill" .. FillVal.Value].Transparency = 0
end)

Prompt.Triggered:Connect(function(player)
	if player.Character:FindFirstChildOfClass('Tool') then
		local Tool = player.Character:FindFirstChildOfClass('Tool')
		if Tool:GetAttribute("Type") == "Water" and FillVal.Value > 0 then
			if Charge > 0 then
				Charge = Charge - 1
				RefillEvent:FireClient(player)
				Prompt.Parent.Drip.PlaybackSpeed = (math.random(1,1.3))
				Prompt.Parent.Drip:Play()
			elseif Charge <= 0 then
				Charge = 4
				
				FillVal.Value = FillVal.Value - 1
				RefillEvent:FireClient(player)
				Prompt.Parent.Drip.PlaybackSpeed = (math.random(1,1.3))
				Prompt.Parent.Drip:Play()
			end
			if FillVal.Value < 0 then
				FillVal.Value = 0
			end
		end
	end
end)```
4

Change this to

if FillVal.Value > #fillParts then
3 Likes
5

Woops, that was my mistake sorry!

1 Like