Destroying a Team when there are no players left on it?

Help and Feedback Scripting Support
#1

To avoid clutter on my game’s playerlist, I am making it where a Team is destroyed when all the players are removed from it. I’ve made a script that creates and removes teams and placed it in ServerScriptService, but it doesn’t work in the part where Team.PlayerRemoved is used.

local Teams = game:GetService("Teams")
local Players = game:GetService("Players")
local Debris = game:GetService("Debris")

Players.PlayerAdded:Connect(function(player)
	player:GetPropertyChangedSignal("TeamColor"):Connect(function(prop)
		local Children = Teams:GetChildren()
		for i = 1, #Children do
			if Children[i]:IsA("Team") then
				local Members = Children[i]:GetPlayers()
				if Children[i].TeamColor.Name == prop.Name and #Members < 3 then
					player.Team = Children[i]
					return
				end
			end
		end
		
		local Team = Instance.new("Team")
		Team.TeamColor = BrickColor.new(player.TeamColor.Name)
		Team.Name = Team.TeamColor.Name
		Team.AutoAssignable = false
		Team.Parent = Teams
		
		Team.PlayerRemoved:Connect(function()
			print("Removed")
			local Members = Team:GetPlayers()
			if #Members < 1 then
				Team:Destroy()
			end
		end)
	end)
end)

Is there something wrong with the script? Team.PlayerRemoved only fires when I press the “Stop” button in Studio.

1 Like
#2 
local Teams = game:GetService('Teams'):GetChildren()

for _,v in next, Teams do
    v.PlayerRemoved:Connect(function()
        if v:GetPlayers() <= 0 then
            v:Destroy()
        end
    end)
end
#3

The canonical function for fetching a list of teams is Teams.GetTeams rather than Instance.GetChildren.

As GetTeams (or GetChildren) in your case return an array, you should also use ipairs. I don’t really know where this style of “in next” came from but frankly it’d be more readable and proper to be using ipairs. In addition, this guarantees ordered iteration of your array at an i++ basis.

2 Likes
#4

I think it came from the fact that it used to be faster than ipairs, but now it’s slower (possibly due to changes from the new Lua VM?) Now I personally use in next by habit, because I wanted to look cool and have 2% faster code. Unless that’s never been true, and I’ve just been a fool all along. (I don’t think I’m the only one).

#5

Using in, next is technically faster, pairs() returns next, arg1. It’s not worth the readability cost imo.

There’s a typo in the above script with v:GetPlayers() should be #v:GetChildren().

Here’s how I’d solve this problem:

local Teams = game:GetService("Teams")

local function handleTeam(team)
	team.PlayerRemoved:Connect(function()
		if #team:GetPlayers() == 0 then
			team:Destroy()
		end
	end)
end

Teams.ChildAdded:Connect(function(obj)
	if obj:IsA("Team") then
		handleTeam(obj)
	end
end)

for _, team in pairs(Teams:GetTeams()) do
	handleTeam(team)
end
8 Likes
#6

It looks like next is actually slower than in pairs for arrays only (by a ridiculously tiny margin), based on a quick test (1e5 repetitions of looping through an array with 1000 numerical keys). Funnily enough, numeric for is even slower, even though the old “conventional advice” was to use a numerical loop (from what I remember).

I don’t like the feeling of all my old (admittedly pointless to begin with) micro-optimisations becoming useless. At least there’s no more local sin = math.sin for everything

1 Like
#7

Ah my bad with the typo I just wrote it really quick without really thinking and I don’t ever use teams so I wasn’t aware of GetTeams

#8

The new VM optimised pairs and ipairs by a large margin. The benchmarks were shown at RDC 2019 and the latter had a 40% advantage over the former. Most advice regarding iterators is in regards to the old VM and is now antiquated (e.g. ipairs was slow); use ipairs to iterate through arrays.

#9

Thank you for your helpful reply, but I have another issue. This doesn’t work when the player leaves. I saw that this is a bug and it hasn’t been fixed. Any workaround for this?

#10

Adding a wait() is the hacky solution!

#11

Did something else that works too:

local function handleTeam(team)
	function onPlayerRemoved(player)
		if team.Parent ~= nil then
			local Members = team:GetPlayers()
			if #Members == 0 or #Members == 1 and player == Members[1] then
				team:Destroy()
				team.PlayerRemoved:Connect(onPlayerRemoved):Disconnect(onPlayerRemoved)
			end
		end
	end
	team.PlayerRemoved:Connect(onPlayerRemoved)
end
#12

Looks good! Remove this line, it doesn’t do what you think, and may actually memory leak. :Destroy() disconnects all events.

1 Like