Is my code optimizing?

Is my code optimizing?

For a code I made I thought that disconnect would be a good idea (its not) so I directly use booleans values I wanted to know if it was optimizing to do like its?

local RunService = game:GetService("RunService")
local TweenService = game:GetService("TweenService")

local player = game.Players.LocalPlayer
local character = player.Character
local rig = game.Workspace.Enemies.Rig
local rangePartClone = game.ReplicatedStorage.ManageClone.RangePart:Clone()
local range = 50
local Activate = true or false

local function onRenderStepped()
	if (character.HumanoidRootPart.Position - rig.HumanoidRootPart.Position).Magnitude <= range then 
		if Activate then
			Activate = false
			rangePartClone.Parent = workspace
			rangePartClone.Position = Vector3.new(rig.HumanoidRootPart.Position.X, rig.HumanoidRootPart.Position.Y - 2.85, rig.HumanoidRootPart.Position.Z)
			TweenService:Create(rangePartClone, TweenInfo.new(1), {Size = Vector3.new(0.15, range * 2, range * 2)}):Play()
		else
			return
		end
	elseif game.Workspace:FindFirstChild(rangePartClone.Name) ~= nil then
			local tweenClose = TweenService:Create(rangePartClone, TweenInfo.new(1), {Size = Vector3.new(0.15, 1, 1)})
			tweenClose:Play()
			tweenClose.Completed:Connect(function()
				rangePartClone.Parent = nil
				Activate = true
			end)
		else
			return
		end
	end
RunService.RenderStepped:Connect(onRenderStepped)
1 Like

Use local character = player.Character or player.CharacterAdded:Wait().


Activate will automatically evaluate to true, So remove “or false”.


Those else return is unnecessary.


Criticisms aside, your code looks great.

2 Likes

Thank you very much, for the “return” it’s normal, because I have to end this function which is therefore voluntary if I had not put return, then I would add code!

2 Likes

There are a number of things that could be cleaned up here. Now, I’m usually against premature optimization, but since your title talks about optimization, let’s start with the things that are likely to be the biggest performance hits.

Probably the biggest one is that in the case where the character is out of range, you’re calling findfirstchild on workspace every frame. This isn’t likely to cause unfathomable lag if it’s only this script thats doing that every frame, but depending on how many things you have in your workspace, it might hog up a little bit of extra frame time that could be better spent doing something else. As such, I suggest that you replace

elseif game.Workspace:FindFirstChild(rangePartClone.Name) ~= nil then

with

elseif rangePartClone.Parent == workspace then

Not only is it faster, it’s also much simpler. Usually you use FindFirstChild for things you don’t already have access to. In this case, you already have access to rangePartClone, so you don’t need to re-find it with FindFirstChild. You can instead use a simpler check.

The second biggest performance thing (which, again, isn’t likely to be huge if this is the only script thats doing this) is that you’re creating an entirely new tween every frame for values that don’t really change. You can factor that out of the onRenderStepped function, like so:

local RunService = game:GetService("RunService")
local TweenService = game:GetService("TweenService")

local player = game.Players.LocalPlayer
local character = player.Character
local rig = game.Workspace.Enemies.Rig
local rangePartClone = game.ReplicatedStorage.ManageClone.RangePart:Clone()
local range = 50
local Activate = true or false

-- We create our tweens here
local tweenOpen = TweenService:Create(rangePartClone, TweenInfo.new(1), {Size = Vector3.new(0.15, range * 2, range * 2)})
local tweenClose = TweenService:Create(rangePartClone, TweenInfo.new(1), {Size = Vector3.new(0.15, 1, 1)})

local function onRenderStepped()
	if (character.HumanoidRootPart.Position - rig.HumanoidRootPart.Position).Magnitude <= range then 
		if Activate then
			Activate = false
			rangePartClone.Parent = workspace
			rangePartClone.Position = Vector3.new(rig.HumanoidRootPart.Position.X, rig.HumanoidRootPart.Position.Y - 2.85, rig.HumanoidRootPart.Position.Z)
			tweenOpen:Play() -- We can play our tweens that we stored in the variable
		else
			return
		end
	elseif rangePartClone.Parent == workspace then
			tweenClose:Play()
		else
			return
		end
	end

-- We only have one tween, so we DON'T want to connect this
-- every frame; that will cause a memory leak! We only want to
-- connect this once. So, we pulll it out of onRenderStepped.
tweenClose.Completed:Connect(function() 
	rangePartClone.Parent = nil
	Activate = true
end)

RunService.RenderStepped:Connect(onRenderStepped)

Those are the biggest performance hits out of the way. They weren’t likely to become a problem anyway unless you duplicated this script multiple times, but I figured I’d knock em out bc that’s what your title is asking for. All that’s left is a little bit of cleaning up. I’ll place comments where I change things to explain why I do them:

local RunService = game:GetService("RunService")
local TweenService = game:GetService("TweenService")

local player = game.Players.LocalPlayer
local character = player.Character
local rig = game.Workspace.Enemies.Rig
local rangePartClone = game.ReplicatedStorage.ManageClone.RangePart:Clone()
local range = 50
local Activate = true or false

local tweenOpen = TweenService:Create(rangePartClone, TweenInfo.new(1), {Size = Vector3.new(0.15, range * 2, range * 2)})
local tweenClose = TweenService:Create(rangePartClone, TweenInfo.new(1), {Size = Vector3.new(0.15, 1, 1)})

local function onRenderStepped()
	if (character.HumanoidRootPart.Position - rig.HumanoidRootPart.Position).Magnitude <= range then 
		if Activate then
			Activate = false
			rangePartClone.Parent = workspace
			rangePartClone.Position = rig.HumanoidRootPart.Position - Vector3.new(0, 2.85, 0) -- You can do arithmetic on vectors! Much neater.
			tweenOpen:Play()
		end -- We don't need the else return block here.
	elseif rangePartClone.Parent == workspace then
		tweenClose:Play()
	end
end

tweenClose.Completed:Connect(function() 
	rangePartClone.Parent = nil
	Activate = true
end)

RunService.RenderStepped:Connect(onRenderStepped)

If you’re looking for next steps, then consider not running this every frame, but maybe every 0.1 seconds or so. This doesn’t seem like it necessarily needs to be done every frame.

1 Like

You can do this instead:

rangePartClone.Position = rig.HumanoidRootPart.Position - Vector3.new(0, 2.85, 0)

This method is about 2.75x faster.

Benchmarked using Boatbomber’s benchmarking module.
Script:

local part : Part = workspace.Part

return {
	ParameterGenerator = function()
		return
	end,

	Functions = {
		["Vector Subtraction"] = function(Profiler) 
			Profiler.Begin("Vector Subtraction")
			for i = 1, 10000 do
				local v = part.Position - Vector3.new(0, .25, 0)
			end
			Profiler.End()
		end,

		["Non Vector Subtraction"] = function(Profiler) 
			Profiler.Begin("Non Vector Subtraction")
			for i = 1, 10000 do
				local v = Vector3.new(part.Position.X, part.Position.Y - .25, part.Position.Z)
			end
			Profiler.End()
		end,
	},
}