Help with complicated script?

Eliminating 25 lines of difficult to type code

The way you are defining lp is pretty scary. You don't need so much code!

We can use a simple loop over GetChildren to make the list without having you type out all of the names! These few lines (which also name no names!) replace the first 25 lines of the code you posted:

local P = script.Parent.Parent; -- Same as before
local lp = {};
for _,object in pairs(P:GetChildren()) do
	-- object is a child of P
	if object:FindFirstChild("Light") and
	object.Light:FindFirstChild("SpotLight") then
		table.insert(lp, object.Light.SpotLight);
	end
end

The fade in/out is actually a moderate annoyance to program, because it is triggered by an event but continues working a time after that. In order for it to work really well, you would have to make sure multiple events don't interfere with each other, which means the events themselves probably shouldn't be doing the fading*.

*There's a solution to this that I will show here, but first I'll detail the solution without it first

Solution One

Our initial solution will have two parts: One is a loop constantly setting the brightness of every light. The second is a click event very similar to what you have.

local brightness = 0;
local lightsOn = false;
-- {and lp defined as from above}

btn = script.Parent.ClickDetector
function onClick()
	lightsOn = not lightsOn; -- Light switch
end
btn.MouseClick:connect(onClick)

function updateBrightness()
	local oldBrightness = brightness;
	if lightsOn then
		brightness = math.min(1, brightness + 1 / (1 / .03));
		-- The amount is so that the fade will take 1 second
	else
		brightness = math.max(0, brightness - 1 / (1 / .03));
	end
	if brightness == oldBrightness then
		return true; -- For later;
		-- also avoids setting redundantly all of
		-- the values below which wastes time /
		-- network availability
	end
	for _,light in pairs(lp) do
		light.Brightness = brightness * 1;
		-- If max brightness should be more than 1,
		-- multiply by something bigger.
	end
end

while true do
	wait(0.03);
	updateBrightness();
end

This code will work as you would expect it would as a user no matter how much you click the on / off button. It will fade all the lights on / off together, which is what I'm assuming you wanted. (A very different approach is needed for separately...)

Improved Solution (Solution Two)

The downside to this is that we have this while true do loop working constantly, even when things aren't changing; at the same time, we want to keep it so that if the light switch is pressed again, that click is registered and we don't get conflicts. Here is a solution to that problem:

local brightness = 0;
local lightsOn = false;
local controller = nil;
function onClick()
	local myController = {};
	controller = myController;
	lightsOn = not lightsOn; -- Light switch
	while controller == myController and not updateBrightness() do
		wait(0.03);
	end
end

-- (using same definition of `updateBrightness`as before)

What this does is run the loop until either

1. The brightness stops changing (all the way on or all the way off)
2. controller is different from myController: Since controller is set to a new value whenever the button is clicked again, so this check is the same as "as long as it has not been clicked again"

I am very disappointed in the quality of answers I see on this site. Just because a script seems unimportant doesn't mean you should do it badly! If someone's asking for help, don't just sham something together, give the best answer you can give. If you aren't going to give the best answer they could receive then you probably aren't the person to give the answer..