Is using many functions for an if statement bad performance wise? If so how should I do it in the future

This is a support category for asking questions about how to get something done on the Roblox websites or how to do something on Roblox applications such as Roblox Studio.

You can write your topic however you want, but you need to answer these questions:
I want to know if there’s a better way to run checks that look for character accessory and if this is bad performance wise.

I’ve tried looking for help on the devforum but nobody seems to have the same question.


local function EndlagCheck()
	for _, accessory in ipairs(character:GetChildren()) do
		if accessory:IsA("Accessory") and accessory.Name == "Endlag" then
			return true
		end
	end
	return false
end

local function StunCheck()
	for _, accessory in ipairs(character:GetChildren()) do
		if accessory:IsA("Accessory") and accessory.Name == "Stun" then
			return true
		end
	end
	return false
end

local function StaggerCheck()
	for _, accessory in ipairs(character:GetChildren()) do
		if accessory:IsA("Accessory") and accessory.Name == "StaggerStun" then
			return true
		end
	end
	return false
end

local function ParryCheck()
	for _, accessory in ipairs(character:GetChildren()) do
		if accessory:IsA("Accessory") and accessory.Name == "ParriedStun" then
			return true
		end
	end
	return false
end

local function AttackMoveCheck()
	for _, LV in ipairs(HumanoidRootPart:GetChildren()) do
		if LV:IsA("LinearVelocity") then
			return true
		end
	end
	return false
end

local function HeavyCheck()
	for _, accessory in ipairs(character:GetChildren()) do
		if accessory:IsA("Accessory") and accessory.Name == "HeavyAttack" then
			return true
		end
	end
	return false
end

local function M1Check()
	for _, accessory in ipairs(character:GetChildren()) do
		if accessory:IsA("Accessory") and accessory.Name == "LightAttack" then
			return true
		end
	end
	return false
end

local function AttackMove(NumberedAttack)
if equipped and not StunCheck() and not StaggerCheck() and not ParryCheck() and isBlocking == false and Endlag.Value == false and IsDashing.Value == false and Humanoid.Health > 0 then
	
	Humanoid.JumpPower = 0
	
	if NumberedAttack == 1 then
	
		local linearVelocity = Instance.new("LinearVelocity")
		linearVelocity.Attachment0 = HumanoidRootPart.RootAttachment
		linearVelocity.MaxForce = 25000
		linearVelocity.RelativeTo = Enum.ActuatorRelativeTo.Attachment0
		linearVelocity.VectorVelocity = Vector3.new(0, 0, -7.5)
		linearVelocity.Parent = HumanoidRootPart

		game.Debris:AddItem(linearVelocity, 0.3)

	elseif NumberedAttack == 2 then
		
		local linearVelocity = Instance.new("LinearVelocity")
		linearVelocity.Attachment0 = HumanoidRootPart.RootAttachment
		linearVelocity.MaxForce = 25000
		linearVelocity.RelativeTo = Enum.ActuatorRelativeTo.Attachment0
		linearVelocity.VectorVelocity = Vector3.new(0, 0, -3)
		linearVelocity.Parent = HumanoidRootPart

		game.Debris:AddItem(linearVelocity, 0.3)
		end
	end
	if AttackMoveCheck() == false then
	task.wait(0.25)
	end
	Humanoid.JumpPower = 50
end

Here’s a tiny bit of my code is the over usage of functions bad?

I don’t think it would do any harm to be honest. If it would do any performance issues it would have to be in multiple scripts to notice a tiny drop of performance, but very little. As long as it works it should be fine.

There’s probably no performance problems with the code above - it’s fairly standard practice to split code into functions. However, since you do have a lot of repeated code for checking if accessories exist, it may make sense for you to make a single function which takes the name of an accessory to find as an input, as below:

local function hasAccessory(accessoryName: string): boolean
    for _, accessory in pairs(character:GetChildren()) do
        if accessory:IsA("Accessory") and accessory.Name == accessoryName then
            return true
        end
    end
    return false
end

Then instead of having to call the different functions to look for individual accessories, you would just pass the accessory names you’re looking for:

if hasAccessory("Stun") then ... -- same as "if StunCheck() then ..."
if hasAccessory("StaggerStun") then ... -- same as "if StaggerCheck() then..."
if hasAccessory("ParriedStun") then ... -- same as "if ParryCheck() then ..."

This may make your code easier to work with and have less repetition. Regardless though, your code shouldn’t have any problems regarding performance, it can just be hard to read if it contains a lot of repetition and the solution above may be more elegant.

it wouldn’t matter much. you can profile it to see if this is a bottle neck of the game. I would guess it probably not.

In its current stage, it is easy to read and extend.

because most of the involves enumerating all children, an alternative would be:

local function AccessoryNameCheck(accessory, name)
	return accessory.Name == name
end

local function EndlagCheck(accessory)
	return AccessoryNameCheck(accessory, "Endlag")
end

local function StunCheck(accessory)
	return AccessoryNameCheck(accessory, "Stun")
end

local function StaggerCheck(accessory)
	return AccessoryNameCheck(accessory, "StaggerStun")
end

local function ParryCheck(accessory)
	return AccessoryNameCheck(accessory, "ParriedStun")
end

local function HeavyCheck(accessory)
	return AccessoryNameCheck(accessory, "HeavyAttack")
end

local function M1Check(accessory)
	return AccessoryNameCheck(accessory, "LightAttack")
end

local function AttackMoveCheck()
	for _, LV in ipairs(HumanoidRootPart:GetChildren()) do
		if LV:IsA("LinearVelocity") then
			return true
		end
	end
	return false
end

local function CanDash() -- wrap the original checks into a function
	-- so we can use early returns, these are relatively lighter than the looping name checks
	if not equipped then return false end
	if isBlocking then return false end
	if Endlag.Value then return false end
	if IsDashing.Value then return false end
	if Humanoid.Health <= 0 then return false end

	--  and group up the looping for all accessories
	for _, accessory in ipairs(character:GetChildren()) do
		if accessory:IsA("Accessory") then
			if StunCheck(accessory) then return false end
			if StaggerCheck(accessory) then return false end
			if ParryCheck(accessory) then return false end
		end
	end
	return true
end

local function AttackMove(NumberedAttack)
	if CanDash() then
	
		Humanoid.JumpPower = 0
	
		if NumberedAttack == 1 then
	
			local linearVelocity = Instance.new("LinearVelocity")
			linearVelocity.Attachment0 = HumanoidRootPart.RootAttachment
			linearVelocity.MaxForce = 25000
			linearVelocity.RelativeTo = Enum.ActuatorRelativeTo.Attachment0
			linearVelocity.VectorVelocity = Vector3.new(0, 0, -7.5)
			linearVelocity.Parent = HumanoidRootPart

			game.Debris:AddItem(linearVelocity, 0.3)

		elseif NumberedAttack == 2 then
		
			local linearVelocity = Instance.new("LinearVelocity")
			linearVelocity.Attachment0 = HumanoidRootPart.RootAttachment
			linearVelocity.MaxForce = 25000
			linearVelocity.RelativeTo = Enum.ActuatorRelativeTo.Attachment0
			linearVelocity.VectorVelocity = Vector3.new(0, 0, -3)
			linearVelocity.Parent = HumanoidRootPart

			game.Debris:AddItem(linearVelocity, 0.3)
		end
	end
	if AttackMoveCheck() == false then
		task.wait(0.25)
	end
	Humanoid.JumpPower = 50
end