How can I optimize this tower placement code?

Help and Feedback Code Review
#1

I’ve been working on a TD game, but testers has been reporting of numerous FPS drops when placing too many towers. This script, handling the tower placement, was the cause of the FPS drops, taking up 1% to 66% of the script performance as more towers are being placed by that player. How can I optimize this? Do I need to change it entirely?

local UIS = game:GetService("UserInputService")
local Unit = nil
local player = game.Players.LocalPlayer
local _Config = script.Parent.Parent.Parent.Configuration
local Remote = game.ReplicatedStorage.ReplicatedStorage_CLOUD.UnitControls.PlaceUnit
local Mouse = player:GetMouse()

for i, v in pairs(script.Parent:GetChildren())do
	if v:IsA("Frame") and v:FindFirstChild("Place") then
		v:FindFirstChild("Place").MouseButton1Up:Connect(function()
			_Config.UnitChoosed.Value = v:WaitForChild("Place"):WaitForChild("Tower").Value
			_Config.UnitEditor.Value = true
			Unit = _Config.UnitChoosed.Value:Clone()
			Unit.Parent = game.Workspace
			Mouse.TargetFilter = Unit
			local AnimationHandler = Unit:WaitForChild("Visual"):WaitForChild("AnimationController"):LoadAnimation(Unit:WaitForChild("Visual"):WaitForChild("Animations"):WaitForChild("Hold"))
			AnimationHandler:Play()
			game:GetService("RunService").RenderStepped:Connect(function()
				if Mouse.Target ~= nil then
					Unit.PrimaryPart.CFrame = CFrame.new(Mouse.Hit.Position) * CFrame.new(0, Unit.Hitbox.Size.Y/2, 0) -- Offset
				end
			end)
			Mouse.Button1Down:Connect(function()
				Remote:FireServer(_Config.UnitChoosed.Value, Mouse.hit, _Config.UnitOrientation.Value, _Config.UnitPlaceable.Value)
				Unit:Destroy()
			end)
		end)
end
	end
2 Likes
#2

You should minimize WaitForChild use, especially on instances you expect to already be there. The entire hierarchy is already constructed when :Clone() returns so you can avoid using WaitForChild on that entirely.

Additionally, you’re using FindFirstChild as v:FindFirstChild("Place").MouseButton1Up, which could just be replaced with dot indexing if you’re not also doing a nil check.

As for the actual performance issues, you have a RunService.RenderStepped event that you connect on every unit but never disconnect. You should store the event somewhere and dispose of it via connection:Disconnect() at the same time that your unit is.

7 Likes
#3

On top of the above advice, I would strongly recommend you use Heartbeat instead of RenderStepped because you aren’t performing any actions that would reasonably need to occur before a frame renders. Simply changing this could also save you a little bit, even if CFrame is the least expensive property to update in the engine.

When you need a RunService event you should generally always be using Heartbeat; Stepped and RenderStepped/BindToRenderStep are for more specific use cases, the former being for something that should occur before physics are simulated and the latter for camera/character updates.

3 Likes