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:
1 | local P = script.Parent.Parent; |
3 | for _,object in pairs(P:GetChildren()) do |
5 | if object:FindFirstChild("Light") and |
6 | object.Light:FindFirstChild("SpotLight") then |
7 | table.insert(lp, object.Light.SpotLight); |
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.
02 | local lightsOn = false; |
05 | btn = script.Parent.ClickDetector |
07 | lightsOn = not lightsOn; |
09 | btn.MouseClick:connect(onClick) |
11 | function updateBrightness() |
12 | local oldBrightness = brightness; |
14 | brightness = math.min(1, brightness + 1 / (1 / .03)); |
17 | brightness = math.max(0, brightness - 1 / (1 / .03)); |
19 | if brightness == oldBrightness then |
25 | for _,light in pairs(lp) do |
26 | light.Brightness = brightness * 1; |
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:
02 | local lightsOn = false; |
03 | local controller = nil; |
05 | local myController = {}; |
06 | controller = myController; |
07 | lightsOn = not lightsOn; |
08 | while controller == myController and not updateBrightness() do |
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..
0
