Script drops FrameRate

Hey what’s up, I’m making a system where the players can pair with another player and every time the loop runs I am experiencing some slight drop in frame rates. I am new to parallel and have started using it but I’m not sure if I’ve used it effectively here. I would love some advice on how I can make this loop less laggy.

local Players = game:GetService("Players")
local ReplicatedStorage = game:GetService("ReplicatedStorage")

local player = Players.LocalPlayer

local function checkPlayersToPair(character) 
			local previousCharacter = nil
			local closestCharacter = nil
		while task.wait(1) do 
			local closestDistance = 100
			local playerHasPair = ReplicatedStorage.Functions.CheckPair:InvokeServer()
			if playerHasPair == false then 
				if character.Humanoid.Health == 0 then
				if player.PlayerGui:FindFirstChild("PairPlayer") then 
					player.PlayerGui:FindFirstChild("PairPlayer"):Destroy()
					break
				end
				character:WaitForChild("HumanoidRootPart")
				for index, otherPlayer in ipairs(Players:GetPlayers()) do
					task.desynchronize()
					if otherPlayer ~= player then 
						if otherPlayer.Character:FindFirstChild("HumanoidRootPart") then 
							local distance = (character.HumanoidRootPart.Position - otherPlayer.Character.HumanoidRootPart.Position).Magnitude
							if distance < closestDistance then 
								task.synchronize()
								local otherPlayerPaired = ReplicatedStorage.Functions.CheckPair:InvokeServer(otherPlayer)
								if otherPlayerPaired == false then 
									closestCharacter = otherPlayer.Character
									closestDistance = distance
								end
							end
						end
					end
				end
				task.synchronize()
				print("closest character = "..tostring(closestCharacter))
				print("Closest distance = "..tostring(closestDistance))
				print("previous character = "..tostring(previousCharacter))
				if closestDistance <= 15 then 
					if closestCharacter ~= previousCharacter then 
						print("Closest character isnt old closest character")
						previousCharacter = closestCharacter
						if player.PlayerGui:FindFirstChild("PairPlayer") then 
							player.PlayerGui:FindFirstChild("PairPlayer"):Destroy()
						end
						local clonedUI = ReplicatedStorage.UI.PairPlayer:Clone()
						clonedUI.Parent = player.PlayerGui
						clonedUI.Adornee = closestCharacter:FindFirstChild("UpperTorso")
					end
				else 
					previousCharacter = nil
					if player.PlayerGui:FindFirstChild("PairPlayer") then 
						player.PlayerGui:FindFirstChild("PairPlayer"):Destroy()
					end
				end
			else 
				break
			end
		end
end


player.CharacterAdded:Connect(checkPlayersToPair)
1 Like

Rather than an invoke I’d insist you use Attributes.

You’re wasting valuable resources by using an Invoke, as it has to wait on the server for each call.
You can use Attributes like so:

local PlayerThatIsPaired = Player -- assumes it is a player, it's just an example.
PlayerThatIsPaired:SetAttribute("Paired", true) -- They are currently paired.

and read them like so, in your code.

local Players = game:GetService("Players")
local ReplicatedStorage = game:GetService("ReplicatedStorage")

local Client = Players.LocalPlayer -- Use more descriptive variables, player can be ANY player, Client is clearly you.

local function checkPlayersToPair(character) 
	local previousCharacter = nil
	local closestCharacter = nil
	while task.wait(1) do 
		local closestDistance = 100
		local playerHasPair = Client:GetAttribute("Paired")
		if not playerHasPair then
			if Client.Character == character then -- Might be respawned, which doesn't necessarily kill the Humanoid.
				if Client.PlayerGui:FindFirstChild("PairPlayer") then
					Client.PlayerGui:FindFirstChild("PairPlayer"):Destroy()
					break
				end
				character:WaitForChild("HumanoidRootPart")
				task.desynchronize() -- You no longer have to desynchronize your parallelity every time you go to check a player, so we can desync outside of the loop.
				for index, otherPlayer in ipairs(Players:GetPlayers()) do
					if otherPlayer ~= Client then
						if otherPlayer.Character:FindFirstChild("HumanoidRootPart") then
							local distance = (character.HumanoidRootPart.Position - otherPlayer.Character.HumanoidRootPart.Position).Magnitude
							if distance < closestDistance then
								local otherPlayerPaired = otherPlayer:GetAttribute("Paired")
								if not otherPlayerPaired then
									closestCharacter = otherPlayer.Character
									closestDistance = distance
								end
							end
						end
					end
				end
				task.synchronize()
				print("closest character = "..tostring(closestCharacter))
				print("Closest distance = "..tostring(closestDistance))
				print("previous character = "..tostring(previousCharacter))
				if closestDistance <= 15 then
					if closestCharacter ~= previousCharacter then
						print("Closest character isnt old closest character")
						previousCharacter = closestCharacter
						if Client.PlayerGui:FindFirstChild("PairPlayer") then
							Client.PlayerGui:FindFirstChild("PairPlayer"):Destroy()
						end
						local clonedUI = ReplicatedStorage.UI.PairPlayer:Clone()
						clonedUI.Parent = Client.PlayerGui
						clonedUI.Adornee = closestCharacter:FindFirstChild("UpperTorso")
					end
				else 
					previousCharacter = nil
					if Client.PlayerGui:FindFirstChild("PairPlayer") then 
						Client.PlayerGui:FindFirstChild("PairPlayer"):Destroy()
					end
				end
			else
				break
			end
		end
	end
end


Client.CharacterAdded:ConnectParallel(checkPlayersToPair)
1 Like

Thanks a lot for your solution. I really appreciate the time you put in to help me out.

Because we are connecting parallel now is there any point in the Task.Desynchronize during the loop?

Indeed you’re correct, you should not bother calling task.desynchronize and only use .synchronize when you go to write your serial data.

Edit:
BUT. Let me edit the code really quickly to show you why this must also be placed carefully. I’ll re-edit this with the new code when I am done.

1 Like

Cheers have you edited it? I had a few errors with the code so I adapted it earlier from what you gave me. Connecting the whole function into parallel didn’t work since I was calling a WaitForChild. There is still slight lag on the client when this loop runs but it seems to have improved. Let me know if it’s good or if there is anything that could still be causing the lag. I also do really appreciate the help you have been providing me.

local Players = game:GetService("Players")
local ReplicatedStorage = game:GetService("ReplicatedStorage")

local Client = Players.LocalPlayer 

local function checkPlayersToPair(character) 
	local previousCharacter = nil
	local closestCharacter = nil
	while task.wait(1) do 
		local closestDistance = 100
		local playerHasPair = Client:GetAttribute("Paired")
		if not playerHasPair then
			if Client.Character == character then 
				character:WaitForChild("HumanoidRootPart")
				for index, otherPlayer in ipairs(Players:GetPlayers()) do
					task.defer(function()
						task.desynchronize()
						if otherPlayer ~= Client then
							local otherPlayerPaired = otherPlayer:GetAttribute("Paired")
							if not otherPlayerPaired then
								if otherPlayer.Character:FindFirstChild("HumanoidRootPart") then
									local distance = (character.HumanoidRootPart.Position - otherPlayer.Character.HumanoidRootPart.Position).Magnitude
									if distance < closestDistance then
										closestCharacter = otherPlayer.Character
										closestDistance = distance
									end
								end
							end
						end
					end)
				end
				task.wait()
				task.synchronize()
				print("closest character = "..tostring(closestCharacter))
				print("Closest distance = "..tostring(closestDistance))
				print("previous character = "..tostring(previousCharacter))
				if closestDistance <= 15 then
					if closestCharacter ~= previousCharacter then
						print("Closest character isnt old closest character")
						previousCharacter = closestCharacter
						if Client.PlayerGui:FindFirstChild("PairPlayer") then
							Client.PlayerGui:FindFirstChild("PairPlayer"):Destroy()
						end
						local clonedUI = ReplicatedStorage.UI.PairPlayer:Clone()
						clonedUI.Parent = Client.PlayerGui
						clonedUI.Adornee = closestCharacter:FindFirstChild("UpperTorso")
					end
				else 
					previousCharacter = nil
					if Client.PlayerGui:FindFirstChild("PairPlayer") then 
						Client.PlayerGui:FindFirstChild("PairPlayer"):Destroy()
					end
				end
			else
				if Client.PlayerGui:FindFirstChild("PairPlayer") then
					Client.PlayerGui:FindFirstChild("PairPlayer"):Destroy()
					end 
				break
			end
		end
	end
end


Client.CharacterAdded:Connect(checkPlayersToPair)

I have it mostly finished, I’ve been helping so many people that there’s an overflow of stuff to do. I’ll get back to you when I can, but for now it’s still in the works.

1 Like

Sorry, I’m sick and I can’t bring myself to finish the code, but I will leave it here (it also contains your old code, so that can be used if necessary) for you to finish.

I’ve commented on most of it to give you an idea of what is finished and what needs to be changed for it to work with this code example.

--> We define services first
local Players = game:GetService("Players")
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local RunService = game:GetService("RunService")

--> Next we define our constants ( I use LOUD_SNAKE or otherwise fully capitalize my constants as it helps me readily identify them.)

local CLIENT: Player = Players.LocalPlayer -- Use more descriptive variables, player can be ANY player, Client is more intuitively referring to you.
--> Also constant, so we define it once and don't ever set its value in the code.

local TARGET_GUI_NAME: string = "PairPlayer" -- Similar to State attribute, only change this if you change the name of your GUI.
--> You used "PairPlayer" as a constant a lot, imagine if you changed it and had to go and change all of those!
local BASE_PAIR_PLAYER_GUI = ReplicatedStorage:WaitForChild("UI"):WaitForChild(TARGET_GUI_NAME)

local MAX_DISTANCE_TO_CHECK: number = 100 -- The max distance we consider another character closest from. You set it to 100 so I followed suit.
local STATE_ATTRIBUTE = "Paired" -- Not necessarily needed, you could use a literal constant in place of this, but this makes the code easier to modify.
--> Only change this if you change the name in SetAttribute() on the server

local DISTANCE_TO_PAIR = 15 -- How close do they need to be to get paired to us.

--> Variables

local clientHasRespawned = true; --> We use this as a sanity check to ensure we're not just spamming methods. Default is true so that we do our first passes.
local isClosestWithinDistance = false --> We only change this to true if the closestCharacter is within DISTANCE_TO_PAIR from our own character
local clientCharacter: Model? = CLIENT.Character --> We only change this when the character is replaced (dies, etc.)
local clientGUI: PlayerGui = CLIENT:WaitForChild("PlayerGui") :: PlayerGui --> The client's PlayerGui is stored (It should not change ever.)

local	previousCharacter: Model?, closestCharacter: Model? = nil, nil --> We store previousCharacter and closestCharacter within a global so that it is easily accessible

--old code commented out but kept for sanity checking my code.
--[[
local function checkPlayersToPair(character) 
	local previousCharacter = nil
	local closestCharacter = nil
	while true do
		local closestDistance = 100
		local playerHasPair = Client:GetAttribute("Paired")
		if not playerHasPair then
			if Client.Character == character then -- Might be respawned, which doesn't necessarily kill the Humanoid.
				if Client.PlayerGui:FindFirstChild("PairPlayer") then
					task.synchronize() -- We also missed this edge case, this would've errored when it tried to Destroy the Gui.
					Client.PlayerGui:FindFirstChild("PairPlayer"):Destroy()
					break
				end
				character:WaitForChild("HumanoidRootPart")
				for index, otherPlayer in ipairs(Players:GetPlayers()) do
					if otherPlayer ~= Client then
						if otherPlayer.Character:FindFirstChild("HumanoidRootPart") then
							local distance = (character.HumanoidRootPart.Position - otherPlayer.Character.HumanoidRootPart.Position).Magnitude
							if distance < closestDistance then
								local otherPlayerPaired = otherPlayer:GetAttribute("Paired")
								if not otherPlayerPaired then
									closestCharacter = otherPlayer.Character
									closestDistance = distance
								end
							end
						end
					end
				end
				-- We don't want to get back on the serial scheduler until we've confirmed we have to do something serial and out of parallelity (you're using a while loop in this callback, not ideal, but I'm not here to code everything for you yk!)
				print("closest character = "..tostring(closestCharacter))
				print("Closest distance = "..tostring(closestDistance))
				print("previous character = "..tostring(previousCharacter))
				if closestDistance <= 15 then
					if closestCharacter ~= previousCharacter then
						print("Closest character isnt old closest character")
						previousCharacter = closestCharacter
						task.synchronize()
						if Client.PlayerGui:FindFirstChild("PairPlayer") then
							Client.PlayerGui:FindFirstChild("PairPlayer"):Destroy()
							break
						end
						local clonedUI = ReplicatedStorage.UI.PairPlayer:Clone()
						clonedUI.Parent = Client.PlayerGui
						clonedUI.Adornee = closestCharacter:FindFirstChild("UpperTorso")
					end
				else
					previousCharacter = nil
					if Client.PlayerGui:FindFirstChild("PairPlayer") then
						task.synchronize()
						Client.PlayerGui:FindFirstChild("PairPlayer"):Destroy()
						break
					end
				end
			else
				break
			end
		end
		
		task.wait(1) -- Don't use while call() do, use while true do and add timeouts later on. Otherwise every time the while loop restarts it must wait on the return of task.wait and check if it's truthy
		-- Which adds yet another overhead because this now introduces a non-constant conditional to check.
	end
end


Client.CharacterAdded:ConnectParallel(checkPlayersToPair)]]--

local function GetClosest() --> Checks through players and sets previousCharacter to closestCharacter before changing closestCharacter, also sets "isClosestWithinDistance" to true when the closestCharacter is within DISTANCE_TO_PAIR and false when otherwise.
	
end

-- We're optimizing this so we're no longer waiting on an event triggered loop (which could leak memory, as well.)
game:GetService("RunService").PreRender:ConnectParallel(function(DeltaTime: number)
	if CLIENT.Character == nil then return end;
	
	local isClientPaired = CLIENT:GetAttribute(STATE_ATTRIBUTE)
	
	if clientCharacter ~= CLIENT.Character then
		
		clientCharacter = CLIENT.Character;
		
		clientHasRespawned = true;
		
		
	end
	
	if clientGUI:FindFirstChild(TARGET_GUI_NAME) and not isClientPaired then
		task.synchronize()
		task.spawn(function() clientGUI:FindFirstChild(TARGET_GUI_NAME):Destroy() end)
		return; -- Honestly it's better to just wait for the next call after moving back into serial phase
	end
	
	if not isClosestWithinDistance or clientHasRespawned or not isClientPaired then
		GetClosest() --> Changed up GetClosest to make it a generic call that doesn't need to rely on returning or saving its own values (closestCharacter and previousCharacter as well as isClosestWithinDistance should be set globally)
	end
	
	if not isClientPaired and closestCharacter ~= previousCharacter and closestCharacter ~= nil and isClosestWithinDistance then
		
		task.synchronize()
		
		local clonedUI = ReplicatedStorage.UI.PairPlayer:Clone()
		clonedUI.Parent = clientGUI
		clonedUI.Adornee = closestCharacter:FindFirstChild("UpperTorso")
		-- Fire your events to change your Paired attribute, etc...
	end
	
end)
2 Likes

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.