Is this code optimised? (Capture Points)

[81MasterGamer]

I have a capture point class.
This class has a method to count nearby players. (so that I can assess the number of players belonging to team A, and the number of players belonging to team B, and then work out which team should own the capture point).

To do this, my algorithm is:

• get all characters (by looping through all the children of the Players container, and seeing if they have a character associated with them, if so, add that character to a table)
• loop through all the characters, and check to see if the character is within range, if so, add them to another table
• loop through that table (all characters within range) and get their associated player object and add that player to a table
• return that table.

Here is the code:

function CapturePoint:GetPlayersInRange() -- Returns a table of all players close enough to the point
	local players = PlayerService:GetChildren()
	local characters = {}
	for _, player in pairs(players) do
		local character = player.Character
		if not (player.Character) then continue end
		
		table.insert(characters, character)
	end
	
	local inRangeCharacters = {}
	for _, character in pairs(characters) do
		local distance = (character.PrimaryPart.Position - self.anchor.Position).Magnitude
		if distance <= RANGE then
			table.insert(inRangeCharacters, character)
		end
	end
	
	local inRangePlayers = {}
	for _, character in pairs(inRangeCharacters) do
		local player = PlayerService:GetPlayerFromCharacter(character)
		if not player then continue end
		
		table.insert(inRangePlayers, player)
	end
	
	return inRangePlayers
end

function CapturePoint:Update(deltaTime) --> runs every frame
	print(unpack(self:GetPlayersInRange()))
end

Note that the method will be called in the update method - which is called on heartbeat event.

Is this unoptimised? It feels like it is. Consider if I had 5 capture points, every frame we have to run all these checks.

I had some ideas for optimising it, would any of these be worth it?

1. If a player is in range of say capture point A, it will naturally be impossible for them to be in range of capture point B or C, (the points will be spread far enough apart so that their ranges don’t overlap), so we can just skip the checks for the other points.
2. To avoid the third for loop, I could store the players and characters in a dictionary perhaps?
3. Instead of running the method every frame, perhaps only run it every 0.1 seconds? Maybe even stagger the method calls for each capture point. (eg: 0.1 seconds go by in which A checks, in the next 0.1 seconds only B checks and so on)

I’ve never really delved into optimization before, so I’m really at a lost.

[Kitesnor (Kites)]

As you may expect running that on every frame considering having multiple loops as well as querying all users on server a relatively expensive task especially considering its simple task.

Since it is likely that a large portion of players are not going to be within range for a majority of the time, you are better off using a touch event, at which point you add player to a cached dictionary.

Then when you need run CapturePoint:Update() you loop through the cached dictionary of players who have touched the part to:

• A verify they are within acceptable distance.
• B are still alive.

If conditions are met I presume points are given. Otherwise you remove player from the dictionary.

[81MasterGamer]

Thanks for the quick response!

I’ll make sure to change it.