Script turns on all items without request

Hi! I recently made a script that controls lighting on a stage, however for some reason when I join the game, it turns on all the actions in the script, even though it was never requested to be turned on. If you’re unsure what I mean, the code below should give you some more information. If you’re still confused, i will be watching this topic for any questions you may have~!

local DJLightCoro = coroutine.create(function()
	
	while wait(0.1) do
	for _,Panel in pairs(game.Workspace.WaterparkDestination.Stage:WaitForChild("DJLights"):GetChildren()) do
		
		Panel.LED.Color.BackgroundColor3 = Color3.fromRGB(255,0,0)
		wait(0.2)
		Panel.LED.Color.BackgroundColor3 = Color3.fromRGB(0,255,0)
		wait(0.2)
		Panel.LED.Color.BackgroundColor3 = Color3.fromRGB(0,0,255)
		wait(0.2)
		Panel.LED.Color.BackgroundColor3 = Color3.fromRGB(0,255,0)
		wait(0.1)
		
	end
	end
end)

local BackLightCoro = coroutine.create(function()
	
	while wait(0.1) do
	for _,Panel in pairs(game.Workspace.WaterparkDestination.Stage:WaitForChild("BackWallLights"):GetChildren()) do
		
		Panel.LED.Color.BackgroundColor3 = Color3.fromRGB(255,0,0)
		wait(0.2)
		Panel.LED.Color.BackgroundColor3 = Color3.fromRGB(0,255,0)
		wait(0.2)
		Panel.LED.Color.BackgroundColor3 = Color3.fromRGB(0,0,255)
		wait(0.2)
		Panel.LED.Color.BackgroundColor3 = Color3.fromRGB(0,255,0)
		wait(0.1)
		
	end
	end
end)

game.ReplicatedStorage.StageLights.OnServerEvent:Connect(function(plr, Action)
	
	if Action == "DJLightsOn" then
		
		coroutine.resume(DJLightCoro)
		
	elseif Action == "BackLightsOn" then
		
		coroutine.resume(BackLightCoro)
		
	elseif Action == "ConfettiOn" then
		
		for _,Confetti in pairs(game.Workspace.WaterparkDestination.Stage.Confetti:GetChildren()) do
			
			for _,Particle in pairs(Confetti:GetChildren()) do
				
				if Particle:IsA("ParticleEmitter") then
					
					Particle.Enabled = true
					
				end
				
			end
			
		end
		
	elseif Action == "BubblesOn" then
		
		for _,Bubble in pairs(game.Workspace.WaterparkDestination.Stage.Bubbles:GetChildren()) do
			
			Bubble.Stick.ParticleEmitter.Enabled = true
			
		end
		
	elseif Action == "FireOn" then
		
		for _,Fire in pairs(game.Workspace.WaterparkDestination.Stage.Fire:GetChildren()) do
			
			Fire.ParticleEmitter.Enabled = true
			
		end
	
	elseif Action == "FStageLightsOn" then
		
		for _,Light in pairs(game.Workspace.WaterparkDestination.Stage.FrontStageLights:GetChildren()) do
			
			for _,Lights in pairs(Light.Head.Lights:GetChildren()) do
				
				Lights.SurfaceLight.Color = game.Workspace.WaterparkDestination.Stage.DJLights.Panel.LED.Color.BackgroundColor3
				Lights.Color = game.Workspace.WaterparkDestination.Stage.DJLights.Panel.LED.Color.BackgroundColor3
				
			end
			
		end
	else
		
		return
	
	end
	
end)

1 Like

As far as I can tell, I don’t see any errors with this script. Maybe post the script that fires the server event to start an action?

Also, why use coroutines? Does a simple function for turning on the stage lights not work for you? There’s nothing necessarily wrong with doing it the way you have it, but it might get pretty messy in the future trying to manage the different threads.

From the code you posted, I see nowhere an action to “turn off” the effects.

So once the effects have been turned on, and a player leaves then rejoins (the same server), the effects will still be “turned on”.

Also your RemoteEvent could be abused by exploiters, as they will be able to call the RemoteEvent over and over again. - Perhaps you should add a ‘debounce’ or ‘cooldown-timer’ in the server script, so once an effect have been “turned on”, it cannot be “turned on” again… only if it has explicitly been “turned off”.

For your two coroutine methods, I suggest that you do not use the ‘while wait(..) do’. It is effectively an infinite loop that never stops… well, only until it errors, which wont be visible.

My suggestion would be, that you refactor those “changing light effect” methods, so they can instructed to stop.

Example:

local activationIteration_DJLight = 0 -- outer-scope variable
local isTurnedOn_DJLight = false

local function DJLightCoro(thisIteration)
  isTurnedOn_DJLight = true
  -- Continue looping, as long as the argument's value matches the outer-scope variable's value
  while  thisIteration == activationIteration_DJLight  do
    for -- the rest of your light effect code here
      ...
    end
    wait(0.1)
  end
  isTurnedOn_DJLight = false
end

StageLights.OnServerEvent:Connect(function(plr, Action)
  -- TODO: Some 'debounce / cooldown-timer' may be needed.

  if Action == "DJLightsOn" then
    -- Only turn on the DJLights, when they are 'turned off'.
    if not isTurnedOn_DJLight then
      -- Must increase the 'iteration number', so it becomes "different" each time.
      activationIteration_DJLight = activationIteration_DJLight + 1
      spawn(function() DJLightCoro(activationIteration_DJLight) end)
    end
  elseif Action == "DJLightsOff" then
    -- Turn off DJLights.
    -- Unfortunately cannot be "instantly" due to how the 'DJLightCoro' method is coded,
    -- so have to 'indicate' to the method, that it should stop running at its next `while`-statement check.
    activationIteration_DJLight = activationIteration_DJLight + 1
  end
end)

There is no reason to prefer this over the more common and a lot more descriptive while true do wait() loop.

Edit: Didn’t realise that this was an old post, was just sent this on Discord by someone looking for help.