How to optimize this code?

Dozetopgg · August 14, 2024, 3:37pm

local ON_COLOR = ColorSequence.new({
	ColorSequenceKeypoint.new(0, Color3.fromRGB(92, 239, 0)),
	ColorSequenceKeypoint.new(1, Color3.fromRGB(163, 253, 28)),
})
local OFF_COLOR = ColorSequence.new({
	ColorSequenceKeypoint.new(0, Color3.fromRGB(223, 50, 116)),
	ColorSequenceKeypoint.new(1, Color3.fromRGB(221, 49, 52)),
})

local BACKGROUND_COLOR_ON = Color3.fromRGB(7, 99, 13)
local BACKGROUND_COLOR_OFF = Color3.fromRGB(99, 9, 13)

local UI_STROKE_ON = Color3.fromRGB(14, 59, 6)
local UI_STROKE_OFF = Color3.fromRGB(59, 9, 13)

local function Trading(data)
	if data["Option"] == "All" then
		data["Option"] = "Friends"
	else
		if data["Option"] == "Friends" or "No One" then
			data["Option"] = "All"
		end
	end
		
	return data["Option"]
end

local function ToggleUI(New, data)
	--New.Toggle.Color.UIGradient = if data["Enabled"] then ON_COLOR else OFF_COLOR
	New.Toggle.BackgroundColor3 = if data["Enabled"] then BACKGROUND_COLOR_ON else BACKGROUND_COLOR_OFF
	New.Toggle.Color.UIStroke.Color = if data["Enabled"] then UI_STROKE_ON else UI_STROKE_OFF
	New.Toggle.Title.Text = if data["Enabled"] then "On" else "Off"
end

local function SelectorUI(New, data)
	--New.Toggle.Color.UIGradient = if data["Option"] == "All" then ON_COLOR else OFF_COLOR
	New.Toggle.BackgroundColor3 = if data["Option"] == "All" then BACKGROUND_COLOR_ON else BACKGROUND_COLOR_OFF
	New.Toggle.Color.UIStroke.Color = if data["Option"] == "All" then UI_STROKE_ON else UI_STROKE_OFF
	New.Toggle.Title.Text = Trading(data)
end

for _, data in pairs(Config) do 
	local Template = Objects:FindFirstChild(data["Type"])

	local New = Template:Clone()
	New.Name = data["Type"]
	New.Title.Text = data["Name"]
	New.Parent = Container
	
	if data["Type"] == "Toggle" then
		ToggleUI(New, data)
		New.Toggle.MouseButton1Click:Connect(function()
			data["Enabled"] = not data["Enabled"]
			ToggleUI(New, data)
		end)
	elseif data["Type"] == "Selector" then
		SelectorUI(New, data)
		New.Toggle.MouseButton1Click:Connect(function()
			data["Enabled"] = not data["Enabled"]
			SelectorUI(New, data)
		end)
	end
end

0786ideal · August 14, 2024, 4:03pm

a lot of statements there, are they really important? this is client, you don’t need that many checks, you need only to suppose that this is visually needed, cheaters can still do whatever they want

HadokeenW · August 14, 2024, 4:12pm

Instead of everytime checking if the data type is selector or toggle , u can just make a fonction called UpdateUI that handle both UI types

and you dont have to create the same MousebuttonEvent twice for each UI Type as its not optimized, just make a single event that handle both

local function UpdateUI(New, data)
	local toggle = New.Toggle
       local isSelector = data["Type"] == "Selector"
	local isOn = data["Enabled"] or (isSelector and data["Option"] == "All")
	
	toggle.BackgroundColor3 = isOn and BACKGROUND_COLOR_ON or BACKGROUND_COLOR_OFF
	toggle.Color.UIStroke.Color = isOn and UI_STROKE_ON or UI_STROKE_OFF
	
	if isSelector then
		toggle.Title.Text = Trading(data)
	else
		toggle.Title.Text = isOn and "On" or "Off"
	end
end

for _, data in pairs(Config) do 
	local Template = Objects:FindFirstChild(data["Type"])

	if Template then
		local New = Template:Clone()
		New.Name = data["Type"]
		New.Title.Text = data["Name"]
		New.Parent = Container

		UpdateUI(New, data)

		New.Toggle.MouseButton1Click:Connect(function()
			data["Enabled"] = not data["Enabled"]
			UpdateUI(New, data)
		end)
	end
end

yesssssssss90900 · August 14, 2024, 5:12pm

local ON_COLOR = ColorSequence.new({
	ColorSequenceKeypoint.new(0, Color3.fromRGB(92, 239, 0)),
	ColorSequenceKeypoint.new(1, Color3.fromRGB(163, 253, 28)),
})
local OFF_COLOR = ColorSequence.new({
	ColorSequenceKeypoint.new(0, Color3.fromRGB(223, 50, 116)),
	ColorSequenceKeypoint.new(1, Color3.fromRGB(221, 49, 52)),
})

local BACKGROUND_COLOR_ON = Color3.fromRGB(7, 99, 13)
local BACKGROUND_COLOR_OFF = Color3.fromRGB(99, 9, 13)

local UI_STROKE_ON = Color3.fromRGB(14, 59, 6)
local UI_STROKE_OFF = Color3.fromRGB(59, 9, 13)

local function UpdateUI(New, isEnabled, isAllOption)
	New.Toggle.BackgroundColor3 = isEnabled and BACKGROUND_COLOR_ON or BACKGROUND_COLOR_OFF
	New.Toggle.Color.UIStroke.Color = isEnabled and UI_STROKE_ON or UI_STROKE_OFF
	New.Toggle.Title.Text = isAllOption and "All" or isEnabled and "On" or "Off"
end

local function Trading(data)
	if data["Option"] == "All" then
		return "Friends"
	elseif data["Option"] == "Friends" or data["Option"] == "No One" then
		return "All"
	else
		return data["Option"]
	end
end

for _, data in pairs(Config) do
	local Template = Objects:FindFirstChild(data["Type"])
	local New = Template:Clone()
	New.Name = data["Type"]
	New.Title.Text = data["Name"]
	New.Parent = Container

	local isEnabled = data["Enabled"]
	local isAllOption = data["Option"] == "All"

	UpdateUI(New, isEnabled, isAllOption)

	New.Toggle.MouseButton1Click:Connect(function()
		if data["Type"] == "Toggle" then
			data["Enabled"] = not isEnabled
			isEnabled = data["Enabled"]
		elseif data["Type"] == "Selector" then
			data["Option"] = Trading(data)
			isAllOption = data["Option"] == "All"
		end
		UpdateUI(New, isEnabled, isAllOption)
	end)
end

This code is similar to the code that HadokeenW gave, but I simplified the Trading function so that it uses early returns to simplify the logic, optimizing it even more.

Dozetopgg · August 14, 2024, 6:53pm

maybe you know how to make a slider and add it here to control the music volume?

cakeycocoa · August 14, 2024, 7:00pm

When using a for loop don’t use pairs or ipairs (unless you have mixed a dictionary and an array together and need to target only one of them), just leave it blank.

for i, v in x do

end

Dozetopgg · August 14, 2024, 7:02pm

what’s the difference? on this method

cakeycocoa · August 14, 2024, 7:03pm

Most of the times this is faster

Dozetopgg · August 14, 2024, 7:24pm

I want to add 3 variations of trading settings, but variation 3 is not added

New.Toggle.Title.Text = isAllOption and "All" or "Friends" or "Disabled"

2112Jay · August 14, 2024, 7:43pm

So optimized it probably isn’t going to work anymore.

local ON_COLOR = ColorSequence.new({
	ColorSequenceKeypoint.new(0, Color3.fromRGB(92, 239, 0)),
	ColorSequenceKeypoint.new(1, Color3.fromRGB(163, 253, 28)),
})
local OFF_COLOR = ColorSequence.new({
	ColorSequenceKeypoint.new(0, Color3.fromRGB(223, 50, 116)),
	ColorSequenceKeypoint.new(1, Color3.fromRGB(221, 49, 52)),
})

local BACKGROUND_COLOR_ON = Color3.fromRGB(7, 99, 13)
local BACKGROUND_COLOR_OFF = Color3.fromRGB(99, 9, 13)

local UI_STROKE_ON = Color3.fromRGB(14, 59, 6)
local UI_STROKE_OFF = Color3.fromRGB(59, 9, 13)

local function ToggleUI(New, isOn)
	New.Toggle.BackgroundColor3 = if isOn then BACKGROUND_COLOR_ON else BACKGROUND_COLOR_OFF
	New.Toggle.Color.UIStroke.Color = if isOn then UI_STROKE_ON else UI_STROKE_OFF
	New.Toggle.Title.Text = if isOn then "On" else "Off"
end

local function Trading(data)
	return if data["Option"] == "All" then "Friends" else "All"
end

local function SelectorUI(New, option)
	local isAll = option == "All"
	New.Toggle.BackgroundColor3 = if isAll then BACKGROUND_COLOR_ON else BACKGROUND_COLOR_OFF
	New.Toggle.Color.UIStroke.Color = if isAll then UI_STROKE_ON else UI_STROKE_OFF
	New.Toggle.Title.Text = if isAll then "Friends" else "All"
end

for _, data in pairs(Config) do 
	local New = Objects:FindFirstChild(data["Type"]):Clone()
	New.Name = data["Type"]
	New.Title.Text = data["Name"]
	New.Parent = Container
	
	local isEnabled = data["Enabled"]
	if data["Type"] == "Toggle" then
		ToggleUI(New, isEnabled)
		New.Toggle.MouseButton1Click:Connect(function()
			isEnabled = not isEnabled
			data["Enabled"] = isEnabled
			ToggleUI(New, isEnabled)
		end)
	elseif data["Type"] == "Selector" then
		local option = data["Option"]
		SelectorUI(New, option)
		New.Toggle.MouseButton1Click:Connect(function()
			option = Trading({Option = option})
			data["Option"] = option
			SelectorUI(New, option)
		end)
	end
end

Simplified the Trading, Combined some logic, removed some clutter, reused some values.