Script doesn't work as intended on second loop

Help and Feedback Scripting Support
1

I have a GUI which has a certain player pick 4 players to compete in a challenge, which works on the first loop game loop (this is a while loop). The GUI system works perfectly throughout the entire game but when the game starts a new round (second loop and onward) it only lets a player pick one player and then it randomizes the rest of the options. I’m not sure on why this is occurring because there is not errors in the output.

–ServerScript:

--Player picking screen
				--Send picking gui
				for i, player in pairs(thronePlayer) do
					voteEVENT:FireClient(player, contestants)
				end
				--Recieve the votes
				EliminationPlayers = {}
				
				
				function recievevote(i, player)
					 	player = game.Players[player]
						if table.find(EliminationPlayers, player) == nil then
							table.insert(EliminationPlayers, player)
						end
						if #EliminationPlayers == 4 then
							for i, v in pairs(thronePlayer) do
								voteguiEVENT:FireClient(v)
								end
						end
				end
				voteEVENT.OnServerEvent:Connect(recievevote)
				
				local Judgement = 20
				GuiStat.Value =	"Judgement"	
				repeat
				Judgement = Judgement - 1
				Timer.Value = (Judgement)
				wait(1)
				until Judgement == 0 or #EliminationPlayers == 4
				for i, v in pairs(thronePlayer) do 
					voteguiEVENT:FireClient(v)
				end
				--Put the current picked players in a table
				local index = 1;
					while contestants[index] do
						local didRemove = false
							for i,plr in pairs(EliminationPlayers) do
								if contestants[index] == plr then
					      		table.remove(contestants,index)
					      		didRemove = true
					      		break
					    	end
					  		end
						if didRemove == false then
					    index = index + 1
					  	end;
					end
				---Check if there is less than 4 players, if there is put them in a table				
				
				local autoELIM = false
				if #EliminationPlayers < 4 then
				autoELIM = true
					repeat
						elimpick = math.random(1, #contestants)
						pickedelim = contestants[elimpick]
						table.insert(EliminationPlayers, pickedelim)
						table.remove(contestants, elimpick)
						until #EliminationPlayers == 4
					end
				
				--moving picked players into eliminationplayers table
				if autoELIM == false then
					local index = 1;
						while contestants[index] do
							local didRemove = false
								for i,plr in pairs(EliminationPlayers) do
									if contestants[index] == plr then
					      			table.remove(contestants,index)
					      			didRemove = true
					      			break
					    		end
					  		end
						if didRemove == false then
					    index = index + 1
					  	end;
					end
				else
					autoELIM = false
				end

–Local Script

function voteupdate(contestants)
    local avaliableBTNS = script.Parent.Frame:GetChildren()
    
    for i, player in pairs(contestants) do
        script.Parent.Enabled = true
        open()
        local button = avaliableBTNS[1]
        button.Visible = true
        button.Text = player.Name
     
		button.MouseButton1Down:Connect(function()
        voteEVENT:FireServer(button.Text)
        button.Visible = false
        end)
     

        table.remove(avaliableBTNS, 1)
    end    
end

function hideVote(v)
    close()
    script.Parent.Enabled = false
end

voteguiEVENT.OnClientEvent:Connect(hideVote)
voteEVENT.OnClientEvent:Connect(voteupdate)
2

1.
Your code has some improper indentation, which makes it difficult to read and follow its structure

May I suggest that you read up on making correct code indentation, as it will help you in the future, when trying to both understand your own code and what others make.

2.
Without knowing the entire setup of your objects in your game, I suspect you may have a problem with the voteupdate function (and probably the hideVote too), as it never disconnects its “listeners” on the buttons.

So when the next voteEVENT.OnClientEvent is received by the client, then yet another (exactly the same) anonymous-function is connected to the button.MouseButton1Down which should be avoided, to avoid performing the second (and third, and fourth …) calls to voteEVENT:FireServer, for the exact same button.

A possible solution to that, including some other suggested improvements, could be:

-- Associative array of "button-object" => "signal-handler connection"
local buttonsListeners = {}

-- Method that is used both in anonymous-function and the hideVote.
local function buttonHideAndDisconnect(button)
  button.Visible = false

  local buttonListener = buttonsListeners[button]
  if buttonListener then
    -- There was a "signal-handler", so disconnect it and remove from associative array
    buttonListener:Disconnect()
    buttonsListeners[button] = nil
  end
end

local function voteUpdate(contestants)
  open() -- I suspect this function does not use anything within the for-loop,
         -- and therefore no need to call multiple times, which is why I
         -- placed it here before the for-loop.

  script.Parent.Enabled = true -- No need to have this inside the for-loop

  local availableButtons = script.Parent.Frame:GetChildren()
  local buttonIndex = 0
  for _, player in ipairs(contestants) do
    buttonIndex = buttonIndex + 1
    local button = availableButtons[buttonIndex]
    
    button.Visible = true
    button.Text = player.Name

    -- We need the "signal-handler" value, that Connect() returns,
    -- to be able later to disconnect it, so we don't just keep adding new
    -- connections for this button's MouseButton1Down event.
    local rbxSignal = button.MouseButton1Down:Connect(function()
      voteEvent:FireServer(button.Text)
      -- No need for listening on this button anymore, so hide and disconnect it.
      buttonHideAndDisconnect(button)
    end)

    -- Store the "signal-handler" in associative array, so we can find it later
    buttonsListeners[button] = rbxSignal
  end
end

local function hideVote()
  -- Make sure all remaining buttons are hidden and disconnected
  for button, _ in pairs(buttonsListeners) do -- Important: using 'pair' and NOT 'ipair'
    buttonHideAndDisconnect(button)
  end

  close()
  script.Parent.Enabled = false
end

3.
For your server-sided code, there is a risk of being able to add more than four players to the EliminationPlayers array. - You should change that function, so it verifies number of elements in the array, before adding a new item to it:

function receiveVote(playerSendingTheEvent, nameOfChosenPlayer)
  -- Verify first that array has less than 4 items
  if #EliminationPlayers < 4 then
    local player = game.Players[nameOfChosenPlayer]
    -- Player with that name could have left the server, or a exploiter could
    -- be sending garbage values, so better verify that player-object is "still here"
    if player ~= nil then
      if table.find(EliminationPlayers, player) == nil then
        table.insert(EliminationPlayers, player)
        -- No need for the for-loop here, as the "Judgement" while-loop will end
        -- once there are 4 items in array (or timeout) and call the voteguiEVENTs.
      end
    end
  end
end

There is a few other problems with the above suggested code, as it never considers what to do, when a player leaves or gets disconnected from the game, during the time of “Judgement” while-loop. So you could end up with having 4 items in the EliminationPlayers array, where one or more of the players is “not on the server”.

Nor does the above code even verify, that the chosen players are actually from the (valid?) list of possible contestants, so there could be an improvement there too.

There may still be issues with other parts of the code you posted, but my three points written above should be enough for now, to get you thinking (I hope :wink:).

1 Like