Feedback on my beginner light switch code

Help and Feedback Scripting Support
#1

I would like some feedback on a Light Switch script I created utilizing CollectionService.

I am looking for optimization and efficiency. Eventually, this script will be put into a game with many lightParts in many different models and (once modified a bit) will be a part of a power-grid system.

I have just started out with scripting, so any criticism and/or feedback is greatly appreciated. Sorry if I posted in the wrong category!

local collectionService = game:GetService("CollectionService")

local function controller()
	
	for _, v in pairs(collectionService:GetTagged("LightPart")) do
		if v.Color == Color3.fromRGB(163,162,165) then
			v.Color = Color3.fromRGB(99,95,98)
			print("Lights have been turned off.")
		elseif v.Color == Color3.fromRGB(99,95,98) then
			v.Color = Color3.fromRGB(163,162,165)
			print("Lights have been turned on.")
		end
	end
end

local button = workspace.Button

button.ClickDetector.MouseClick:Connect(controller)
1 Like
#2

I think the category is OK, but code review might be a better category perhaps.

Overall, I think it looks concise, but I might suggest using a state to track whether you want all the lights on or off. For example, what if a light gets added mid-game that is not in the same state as the rest of the lights? It will be permanently be in the opposite state as the rest of the lights.

To do that, replace the conditional statements to check the desired light state, and just set the light color to what you want them to be. Don’t check what the lights currently are, (unless that is the desired behavior, but it doesn’t sound like it based on the description.)

Hope this is useful!

1 Like
#3

Pretty good! The only thing I would suggest is storing the light colors in a variable. Would be better than constantly seeing Color3.fromRGB…

2 Likes
#4

Oh this was nice , you can make it as a gui button using proximity… wrong reply

#5 
local collections = game:GetService("CollectionService")

local button = workspace.Button
local click = button.ClickDetector

local toggle = false

local function toggleLights()
	toggle = not toggle
	for _, light in ipairs(collections:GetTagged("LightPart")) do
		light.Color = if toggle then Color3.fromRGB(85, 85, 85) else Color3.fromRGB(170, 170, 170)
	end
end

click.MouseClick:Connect(toggleLights)
#6

Thanks! For some reason, I didn’t think of this. It will definitely make my script look cleaner. :slight_smile:

1 Like