What would be some ways to make this code simpler and better?

Help and Feedback Scripting Support
1

  1. What do you want to achieve? To find a better way to do the same thing.

  2. What is the issue? The code repeats itself and I’m quite sure there ought to be some much better ways to code it.

  3. What solutions have you tried so far? Heard something about using functions but didn’t really get far.

Here’s the code:

local CurrentColor = script.Parent.vari
local vihrea = script.Parent.green
local punainen = script.Parent.red
local keltainen = script.Parent.yellow
local pinkki = script.Parent.pink
local musta = script.Parent.black
local valkoinen = script.Parent.white
local ruskea = script.Parent.brown
local sininen = script.Parent.blue
local violetti = script.Parent.purple

sininen.ClickDetector.MouseClick:Connect(function()
	CurrentColor.BrickColor = BrickColor.new ("Really blue")
end)
punainen.ClickDetector.MouseClick:Connect(function()
	CurrentColor.BrickColor = BrickColor.new ("Really red")
end)
keltainen.ClickDetector.MouseClick:Connect(function()
	CurrentColor.BrickColor = BrickColor.new ("New Yeller")
end)
musta.ClickDetector.MouseClick:Connect(function()
	CurrentColor.BrickColor = BrickColor.new ("Really black")
end)
vihrea.ClickDetector.MouseClick:Connect(function()
	CurrentColor.BrickColor = BrickColor.new ("Lime green")
end)
pinkki.ClickDetector.MouseClick:Connect(function()
	CurrentColor.BrickColor = BrickColor.new ("Hot pink")
end)
ruskea.ClickDetector.MouseClick:Connect(function()
	CurrentColor.BrickColor = BrickColor.new ("Burnt Sienna")
end)
valkoinen.ClickDetector.MouseClick:Connect(function()
	CurrentColor.BrickColor = BrickColor.new ("Institutional white")
end)
violetti.ClickDetector.MouseClick:Connect(function()
	CurrentColor.BrickColor = BrickColor.new ("Eggplant")
end)
2 Likes
2 
-- Your going to need to rename the clickdetectors into proper BrickColor strings for this to work
-- Like changing blue to Really blue

for _,Color in ipairs(script.Parent:GetChildren()) do -- This creates an array table and loops through it
    Color.ClickDetector.MouseClick:Connect(function()
        CurrentColor.BrickColor = BrickColor.new(Color) 
    end)
end

-- Alternaive method to writing this is
local Colors = script.Parent:GetChildren() -- Creates an array table
for i = 1,#Colors do -- Loops through array table starting from first index -> last index
    local Color = Colors[i]
    Color.ClickDetector.MouseClick:Connect(function()
        CurrentColor.BrickColor = BrickColor.new(Color)
    end)
end

-- Feel free to use the method you understand the most

Here a link on tables if you struggle on using tables

Feel free to DM me for more indepth-help Thedagz#4176 (On discord)

2 Likes
3 
local brickColors = {"Really blue", "Really red", "New Yeller", "Really black", "Lime green", "Hot pink", "Burnt Sienna", "Institutional white", "Eggplant"}
local parent = script.Parent
local clickDetectors = {parent.vari, parent.green, parent.red, parent.yellow, parent.pink, parent.black, parent.white, parent.brown, parent.blue, parent.purple}
local CurrentColor = parent.vari

for i, v in ipairs(clickDetectors) do
	v.MouseClick:Connect(function(player) --added player in case later needed
		CurrentColor.BrickColor = BrickColor.new(brickColors[i])
	end)
end