Issue with a Queue Display

So I have this simple queue display that will display players currently in the queue before they’re teleported into the game. My issue is that when I walk onto the platform multiple of a player’s name will show up. I’ve tried moving where the update display functions go but I’m not sure where it’s going wrong.

Here’s the code:

local Template = game.ReplicatedStorage.QueueTemplate

local Gui = script.Parent.Board.SurfaceGui

local Queue = {}

local QueueOpen = true

local Players = game:GetService("Players")

local function UpdateDisplay()
	for i, object in pairs(Gui:GetChildren()) do
		if object:IsA("Frame") then
			object:Destroy()
		end
	end
	for i = 1, #Queue do
		local Clone = Template:Clone()
		Clone.PlayerLabel.Text = Queue[i]
		local UserId = 0
		local ThumbType = Enum.ThumbnailType.HeadShot
		local ThumbSize = Enum.ThumbnailSize.Size100x100
			
			
		for i, player in ipairs(game.Players:GetPlayers()) do
			if player.Name == Queue[i] then
				UserId = player.UserId
			end
		end
		local content, isReady = Players:GetUserThumbnailAsync(UserId, ThumbType, ThumbSize)
		Clone.PlayerImage.Image = content
		Clone.Parent = Gui
	end
end

game.ReplicatedStorage.QueueClose.Event:Connect(function()
	QueueOpen = false
end)

game.ReplicatedStorage.QueueOpen.Event:Connect(function()
	QueueOpen = true
end)

script.Parent.QueuePart.Touched:Connect(function(plr)
	local Human = plr.Parent:FindFirstChild("HumanoidRootPart")
	if Human and QueueOpen == true then
		local Duplicate = false
		for i = 1, #Queue do
			if Queue[i] == plr.Parent.Name then
				Duplicate = true
			end
		end
		if Duplicate == false then
			table.insert(Queue, plr.Parent.Name)
		end
	end
	UpdateDisplay()
end)

script.Parent.ExitQueue.Touched:Connect(function(plr)
	local Human = plr.Parent:FindFirstChild("HumanoidRootPart")
	if Human and QueueOpen == true then
		for i = 1, #Queue do
			if plr.Parent.Name == Queue[i] then
				table.remove(Queue, i)
			end
		end
	end
	UpdateDisplay()
end)

game.Players.PlayerRemoving:Connect(function(plr)
	for i = 1, #Queue do
		if plr.Name == Queue[i] then
			table.remove(Queue, i)
		end
	end
	UpdateDisplay()
end)

UpdateDisplay()
1 Like
for i = 1, #Queue do
	...
	
	for i, player in ipairs(game.Players:GetPlayers()) do
		if player.Name == Queue[i] then
			UserId = player.UserId
		end
	end
	
	...
end

the inner i is shadowing/redefining the outer i, so player.Name == Queue[i] doesn’t make much sense as the i that’s referenced is the player’s index in the list of players, which isn’t associated with the queue in any way. If you use an editor with better static analysis tools you’d get a warning telling you when you shadow variables. E.g. from VS Code:

image

You can solve the problem by changing either variable name. Here’s an example of that, which also uses table for loops (e.g. for k, v in t do) instead of numeric for loops (e.g. for i = 1, 10 do):

for _, playerInQueue in Queue do
	...
	
	for _, player in game.Players:GetPlayers() do
		if player == playerInQueue then
			UserId = player.UserId 
                        break
		end
	end
	...
end

However, since you can just access the player as the values of the key-value pairs in Queue, you don’t need to search for the user ID at all:

for _, player in Queue do
	local ThumbType = Enum.ThumbnailType.HeadShot
	local ThumbSize = Enum.ThumbnailSize.Size100x100

	local listEntry = listEntryTemplate:Clone()
	listEntry.PlayerLabel.Text = player.Name
	listEntry.PlayerImage.Image = Players:GetUserThumbnailAsync(player.UserId, ThumbType, ThumbSize)
	listEntry.Parent = Gui
end

However this means you need to store players in the queue instead of player names, which is generally a better approach. Here’s how that could look:

local queueGui = script.Parent.Board.SurfaceGui
local listEntryTemplate = game.ReplicatedStorage.QueueTemplate

local queue = {}
local canQueueChange = true

--Search for a value in a table, removing it if present.
--Returns true/false indicating if an element was removed
local function removeIfIn(t, value)
	local index = table.find(t, value)
	if index then
		table.remove(t, index)
		return true --Did change
	end
	return false --Didn't change
end

--Search for a value in a table, inserting it if not present.
--Returns true/false indicating if the element was inserted.
local function insertIfNotIn(t, value)
	local index = table.find(t, value)
	if not index then
		table.insert(t, value)
		return true --Did change
	end
	--Didn't change
end

local function updateDisplay()
	for _, object in queueGui:GetChildren() do
		if object:IsA("Frame") then	object:Destroy() end
	end

	for _, player in queue do
		local ThumbType = Enum.ThumbnailType.HeadShot
		local ThumbSize = Enum.ThumbnailSize.Size100x100

		local listEntry = listEntryTemplate:Clone()
		listEntry.PlayerLabel.Text = player.Name
		listEntry.PlayerImage.Image = game.Players:GetUserThumbnailAsync(player.UserId, ThumbType, ThumbSize)
		listEntry.Parent = queueGui
	end
end

game.ReplicatedStorage.QueueClose.Event:Connect(function()
	canQueueChange = false
end)

game.ReplicatedStorage.QueueOpen.Event:Connect(function()
	canQueueChange = true
end)

script.Parent.QueuePart.Touched:Connect(function(touchingPart)
	if not canQueueChange then return end

	local character = touchingPart.Parent
	local player = game.Players:GetPlayerFromCharacter(character)
	if not player then return end --E.g. if an NPC touched

	local didChange = insertIfNotIn(queue, player)
	if didChange then
		updateDisplay()
	end
end)

script.Parent.ExitQueue.Touched:Connect(function(touchingPart)
	if not canQueueChange then return end

	local character = touchingPart.Parent
	local player = game.Players:GetPlayerFromCharacter(character)
	if not player then return end --E.g. if an NPC touched

	local didChange = removeIfIn(queue, touchingPart)
	if didChange then
		updateDisplay()
	end
end)

game.Players.PlayerRemoving:Connect(function(player)
	local didChange = removeIfIn(queue, player)
	if didChange then
		updateDisplay()
	end
end)

updateDisplay()
1 Like