How to optimize this code to prevent lag issues?

evocul · April 2, 2021, 3:14pm

Hey everyone, I’ve been having some issues with my code when it comes to 15+ people within a server. I’ve tried to before but have failed to do so unfortunately.
How can I optimize this so the code can work for 15-80+ in a server?
Thank you in advanced!

The code is a loop, which is used to create matches within my game

while true do wait(2.5)

local AllPlayers = {}
for i,v in pairs(game.Players:GetPlayers()) do
	--print(v)
	table.insert(AllPlayers, #AllPlayers + 1, v)
end

if enufPlayersToStart <= #AllPlayers then

	local CouldFight = {}
	local Player1
	local Player2

	for i,v in pairs(game.Players:GetPlayers()) do
		if v.Information.CurrentlyAFK.Value == false then
			if v.Information.InMatch.Value == false then
				if v.Information.Died.Value == false then 
					if v.Character then
						table.insert(CouldFight, #CouldFight + 1, v)

					end
				end
			end
		end
	end


	if #CouldFight >= 2 then
		local Picker = math.random(1, #CouldFight) --1
		local Picker2 = math.random(1, #CouldFight)--3


		Player1 = CouldFight[Picker]--1
		Player2 = CouldFight[Picker2]--3

		if Player1 == Player2 then
			repeat wait()
				local Picker = math.random(1, #CouldFight)
				local Picker2 = math.random(1, #CouldFight)

				Player1 = CouldFight[Picker]
				Player2 = CouldFight[Picker2]
			until
			Player1 ~= Player2
		end
	end

	if game.Players:FindFirstChild(tostring(Player1)) and game.Players:FindFirstChild(tostring(Player2)) then
		local Arenaz = nil
		local AvArenas = {}

		for i,v in pairs(game.ServerStorage.Arenas:GetChildren()) do
			if v.Active.Value == false then
				table.insert(AvArenas, #AvArenas + 1, v.Name)
			end
		end

		print(#AvArenas)
		if #AvArenas >= 1 then
			Arenaz = AvArenas[math.random(1, #AvArenas)]
			game.ServerStorage.Arenas[Arenaz].Active.Value = true

			local Mapz = game.ServerStorage.Maps:GetChildren()
			local PickedMap = Mapz[math.random(1, #Mapz)]:Clone()


			PickedMap.Parent = workspace
			PickedMap:MoveTo(workspace:FindFirstChild(tostring(Arenaz)).Core.Position)
			PickedMap.Parent = workspace:FindFirstChild(tostring(Arenaz))

			local cframe = PickedMap.PrimaryPart.CFrame -- Current CFrame

			local rotation = workspace:FindFirstChild(tostring(Arenaz)).Core.Orientation.Y

			PickedMap:SetPrimaryPartCFrame(cframe * CFrame.Angles(0, math.rad(rotation), 0))

			local SetPlayerStats = coroutine.create(function()
				game.Players[tostring(Player1)]:WaitForChild("Information").InMatch.Value = true
				game.Players[tostring(Player2)]:WaitForChild("Information").InMatch.Value = true
				local Player1 = Player1
				local Player2 = Player2
				local Arenaz = Arenaz
				local PickedMap = PickedMap
				wait(2)
				if game.Players:FindFirstChild(tostring(Player1)) and game.Players:FindFirstChild(tostring(Player2)) then
					if game.Players[tostring(Player1)]:WaitForChild("Information").CurrentSword.Value == "" then
						game.ServerStorage.SwordModels.Linked:Clone().Parent = game.Players[tostring(Player1)].Information.Inventory
						game.Players[tostring(Player1)].Information.CurrentSword.Value = "Linked"
					end

					if game.Players[tostring(Player2)]:WaitForChild("Information").CurrentSword.Value == "" then
						game.ServerStorage.SwordModels.Linked:Clone().Parent = game.Players[tostring(Player2)].Information.Inventory
						game.Players[tostring(Player2)].Information.CurrentSword.Value = "Linked"
					end

					if not game.Players[tostring(Player1)]:WaitForChild("Backpack"):FindFirstChildWhichIsA("Tool") then
						local Sword = game.Players[tostring(Player1)].Information.Inventory:WaitForChild(game.Players[tostring(Player1)].Information.CurrentSword.Value, 3):Clone()
						if Sword then
							Sword.Parent = game.Players[tostring(Player1)]:WaitForChild("Backpack")
						end
					end

					if not game.Players[tostring(Player2)]:WaitForChild("Backpack"):FindFirstChildWhichIsA("Tool") then
						local Sword = game.Players[tostring(Player2)].Information.Inventory:WaitForChild(game.Players[tostring(Player2)].Information.CurrentSword.Value, 3):Clone()
						if Sword then
							Sword.Parent = game.Players[tostring(Player2)]:WaitForChild("Backpack")
						end
					end

					game.Players[tostring(Player1)]:WaitForChild("Information").Hostile.Value = Player2
					game.Players[tostring(Player2)]:WaitForChild("Information").Hostile.Value = Player1

					game.Players[tostring(Player1)].Character.Humanoid.Health = 100
					game.Players[tostring(Player2)].Character.Humanoid.Health = 100

					game.Players[tostring(Player1)]:WaitForChild("Information").Arena.Value = Arenaz
					game.Players[tostring(Player2)]:WaitForChild("Information").Arena.Value = Arenaz

					game.Players[tostring(Player1)]:WaitForChild("Information").MapUsed.Value = tostring(PickedMap)
					game.Players[tostring(Player2)]:WaitForChild("Information").MapUsed.Value = tostring(PickedMap)

					game.Players[tostring(Player1)].Character:FindFirstChild("Humanoid").WalkSpeed = 0
					game.Players[tostring(Player2)].Character:FindFirstChild("Humanoid").WalkSpeed = 0

					game.Players[tostring(Player1)].Character:FindFirstChild("HumanoidRootPart").CFrame = game.Workspace[Arenaz]:FindFirstChild(tostring(PickedMap)).Team1Spawn.CFrame
					game.Players[tostring(Player2)].Character:FindFirstChild("HumanoidRootPart").CFrame = game.Workspace[Arenaz]:FindFirstChild(tostring(PickedMap)).Team2Spawn.CFrame



					Events.MatchInfo:FireClient(game.Players[tostring(Player1)], tostring(Player2), tostring(PickedMap))
					Events.MatchInfo:FireClient(game.Players[tostring(Player2)], tostring(Player1), tostring(PickedMap))

					local p1 = game.Players:FindFirstChild(tostring(Player1))
					local p2 = game.Players:FindFirstChild(tostring(Player2))

					if p1 then
						p1.Information.PlayerCountdown.Value = "3"
					end

					if p2 then
						p2.Information.PlayerCountdown.Value = "3"
					end

					wait(1)

					if p1 then
						p1.Information.PlayerCountdown.Value = "2"
					end

					if p2 then
						p2.Information.PlayerCountdown.Value = "2"
					end

					wait(1)

					if p1 then
						p1.Information.PlayerCountdown.Value = "1"
					end

					if p2 then
						p2.Information.PlayerCountdown.Value = "1"
					end

					wait(1)

					if p1 then
						p1.Information.PlayerCountdown.Value = "BEGIN"
					end

					if p2 then
						p2.Information.PlayerCountdown.Value = "BEGIN"
					end

					local PlayerA = tostring(Player1)
					local PlayerB = tostring(Player2)

					if game.Players:FindFirstChild(PlayerA) then
						if game.Players:FindFirstChild(PlayerA).Character then
							game.Players:FindFirstChild(PlayerA).Character.Humanoid.WalkSpeed = 16
						end
					end



					if game.Players:FindFirstChild(PlayerB) then
						if game.Players:FindFirstChild(PlayerB).Character then
							game.Players:FindFirstChild(PlayerB).Character.Humanoid.WalkSpeed = 16
						end
					end

					table.insert(PlayersTableStart.Players, #PlayersTableStart.Players + 1, tostring(Player1))
					table.insert(PlayersTableStart.Players, #PlayersTableStart.Players + 1, tostring(Player2))
				else
					game.ServerStorage.Arenas[Arenaz].Active.Value = false
					PickedMap:Destroy()
				end
			end)
			coroutine.resume(SetPlayerStats)
		end
	end
end

end

SuperZebe1 · April 2, 2021, 3:39pm

Does it keep looping even when a match has started?

evocul · April 2, 2021, 3:40pm

Yes, matches happen simultaneously, so there will have multiple matches happening at once

colbert2677 · April 2, 2021, 7:55pm

What part of this code specifically is lagging for you? It’s more helpful if you can pinpoint a specific place where the issues start occurring and highlight that rather than throw your entire code into the thread and ask for optimisation tips on it. Please do some benchmarking, whether lightweight with a few prints and os.clock uses or with the MicroProfiler.

Additionally, are you certain it’s this code specifically that’s causing you performance issues or are there other underlying problems that you haven’t addressed yet?

mgmchenry · April 2, 2021, 11:24pm

I will agree that learning how to use the profiler is a good tip, and learning how to make the operations in the code run faster in general.

@evocul - I think you should take some time to figure out how long this is taking to run to see if you want to spend some time making it faster. Whatever it’s doing every 2.5 seconds shouldn’t be taking so long it’s lagging people.

If it were me, there are a couple things I would probably spend the time on first:

Stop running that code every 2.5 seconds. Run that code in response to certain events, like when a player joins or a match ends. And if it’s already run in the last 2.5 seconds, schedule it to run when the cooldown is up.
Look for any instances where you are checking every player against every other player. For example:

local eligiblePlayers = {} -- except already a populated list of all the players
local playerCount = #eligiblePlayers
for i1 = 1, playerCount - 1 do
  local player1 = eligiblePlayers[i1]
  for i2 = i1 + 1, playerCount do
    local player2 = eligiblePlayers[i2]
    checkMatch(player1, player2)
    -- and whatever other stuff
  end
end

That way you’re not going through the list checking playerA against playerB then checking playerB against playerA. That gets out of hand if you have 30 players. I don’t see that in the code you have above, but something to keep an eye on with this many players.

You do have

		local Picker = math.random(1, #CouldFight) --1
		local Picker2 = math.random(1, #CouldFight)--3


		Player1 = CouldFight[Picker]--1
		Player2 = CouldFight[Picker2]--3

		if Player1 == Player2 then
			repeat wait()

if picker 1 and 2 are the same, change them. No need to see if they are the same player unless you added players to the array multiple times. You should probably avoid putting in duplicates.

How about

local Picker = math.random(1, #CouldFight)
-- think of picker 2 as an offset from picker1
local Picker2 = math.random(1, #CouldFight -1)
-- so actually
Picker2 = Picker + Picker2
-- but that needs to wrap around the CouldFight array size, so modulo and account for lua doesn't zero-index arrays like most languages
Picker2 = (Picker2 - 1) % #CouldFight + 1

-- or all in one:
local Picker2 = (math.random(1, #CouldFight -1) + Picker1 - 1) % #CouldFight + 1

also, it may be paranoid on my part, but standard lua recalculates table length every time by walking the array from element[1] until it reaches element[i]==nil. I’m not sure if Roblox and luau store a value for table length and only recalculates it when the table is updated, but any time I see more than 2 instances of #someTable, I’m personally going to replace that with

local tableSize = #someTable

In general, you don’t want to optimize much before you measure the code you’re “improving” because you won’t know if you’re helping. For example, maybe you don’t need to clone a new copy of the map into the workspace every time.

If the map is mostly static and anchored, maybe you can have the map in replicated storage, then clone on the client side just for the clients who need to see the arena instead of making a whole new copy and putting it in the workspace to have all the parts replicated to each client, potentially creating a huge bottleneck for something most players won’t see.

However, even without measuring, you’ve got some code I’d recommend changing:



			local Mapz = game.ServerStorage.Maps:GetChildren()
			local PickedMap = Mapz[math.random(1, #Mapz)]:Clone()


			PickedMap.Parent = workspace
			PickedMap:MoveTo(workspace:FindFirstChild(tostring(Arenaz)).Core.Position)
			PickedMap.Parent = workspace:FindFirstChild(tostring(Arenaz))

			local cframe = PickedMap.PrimaryPart.CFrame -- Current CFrame

			local rotation = workspace:FindFirstChild(tostring(Arenaz)).Core.Orientation.Y

			PickedMap:SetPrimaryPartCFrame(cframe * CFrame.Angles(0, math.rad(rotation), 0))

After you are creating your clone, you are parenting the arena model to workspace, then moving it, then parenting it to Arenaz, then setting the clone’s CFrame.

I’m pretty sure all you need is:

  Arenaz = AvArenas[math.random(1, #AvArenas)]
  game.ServerStorage.Arenas[Arenaz].Active.Value = true

  local Mapz = game.ServerStorage.Maps:GetChildren()
  local PickedMap = Mapz[math.random(1, #Mapz)]:Clone()
  local arenaContainer = workspace:FindFirstChild(tostring(Arenaz))

  -- no -- PickedMap.Parent = workspace
  -- no -- PickedMap:MoveTo(workspace:FindFirstChild(tostring(Arenaz)).Core.Position)
  -- not yet -- PickedMap.Parent = workspace:FindFirstChild(tostring(Arenaz))

  local rotation = arenaContainer.Core.Orientation.Y
  local cframe = PickedMap.PrimaryPart.CFrame -- Current CFrame
  -- subtract the clone's position, add the container's position
  cframe = cframe - cframe.Position + arenaContainer.Core.Position
  -- you should now have a cframe with the orientation of the clone, but the position of the container
  -- so now add the rotation from the container
  cframe = cframe * CFrame.Angles(0, math.rad(rotation), 0)
  PickedMap:SetPrimaryPartCFrame(cframe)
  PickedMap.Parent = arenaContainer

I may have gotten the cframe math off, but generally, you only want to move the model once and you don’t want to parent it to something that is getting replicated until it’s where you want it.
You can construct the target cframe in one line, I was just hoping to make it clearer.

evocul · April 3, 2021, 12:52am

I’ll get that benchmark done as soon as possible, thank you

I believe it’s the code, on smaller servers there’s no issues, but as soon as the server goes to 15 or so players, players start not getting into matches due to the code working too slow, their Ping also increases randomly which is a huge game changing issue.

evocul · April 3, 2021, 12:57am

Going to be doing the benchmark as soon as possible.
Thank you for change ideas though I appreciate it a lot, going to be looking into changing those.
Quick question too about this:

Stop running that code every 2.5 seconds. Run that code in response to certain events, like when a player joins or a match ends. And if it’s already run in the last 2.5 seconds, schedule it to run when the cooldown is up.

I have the 5th if statement ( if #CouldFight >= 2 then ) to check before the rest of the code runs.
How would I further change it to prevent it running every 2.5 Seconds?