Performance and robust code

Help and Feedback Scripting Support
#1

Recently, I tried to create skills for my game. But my code felt inefficient and weak. Here is the script:

--// Services
local TweenService = game:GetService("TweenService")
local UserInputService = game:GetService("UserInputService")
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local Debris = game:GetService("Debris")
local Players = game:GetService("Players")

--// Modules
local keySettings = require(script.UserInputs)

--// Variables
local character = script.Parent
local humanoid = character:WaitForChild("Humanoid")
local rootPart = character:WaitForChild("HumanoidRootPart")

local partTemplate = ReplicatedStorage.SKILL.Spawner
local remote = ReplicatedStorage.REMOTES.TP
local Ragdoll = ReplicatedStorage.REMOTES.Ragdoll
local spawnParts = {}
local debounce = false
local teleportDebounce = false

local function cleanupPart(part)
	if table.find(spawnParts, part) then
		table.remove(spawnParts, table.find(spawnParts, part))
	end
	Debris:AddItem(part, keySettings.E.DecayTime)
end

local function spawnPart()
	local clone = partTemplate:Clone()
	clone.Parent = workspace
	clone.Name = "Teleporter" .. tostring(#spawnParts + 1)
	clone.Material = Enum.Material.Neon
	clone.Color = Color3.fromRGB(77, 255, 0)
	clone.Position = rootPart.Position + Vector3.new(0, 3, 0)
	clone.Anchored = true
	table.insert(spawnParts, clone)

	-- Touched Event
	clone.Touched:Connect(function(hit)
		if teleportDebounce or #spawnParts < 2 then return end
		local touchingCharacter = hit.Parent
		if touchingCharacter == character then
			humanoid.WalkSpeed = 0
			teleportDebounce = true

			-- Teleport to the first other part
			for _, otherPart in pairs(spawnParts) do
				if otherPart ~= clone then
					remote:FireServer(otherPart.Position)
					break
				end
			end

			task.wait(keySettings.E.Cooldown)
			humanoid.WalkSpeed = 16
			teleportDebounce = false
		end
	end)

	-- Triggered Event
	clone.TRY.Triggered:Connect(function()
		cleanupPart(clone)
	end)
end

local function highlightPlayers(key)
	for _, player in pairs(Players:GetPlayers()) do
		if player.Character and player.Character:FindFirstChild("HumanoidRootPart") and player ~= Players.LocalPlayer then
			local highlight = Instance.new("Highlight")
			highlight.OutlineColor = Color3.fromRGB(0, 255, 17)
			highlight.Parent = player.Character
			Debris:AddItem(highlight, keySettings[key].DecayTime)
		end
	end
end

local function applyDash()
	if not rootPart or not rootPart:FindFirstChild("RootAttachment") then
		warn("HumanoidRootPart or RootAttachment missing!")
		return
	end

	local velocity = Instance.new("LinearVelocity", rootPart)
	velocity.VectorVelocity = rootPart.CFrame.LookVector * 100
	velocity.Attachment0 = rootPart.RootAttachment

	task.delay(3, function()
		velocity:Destroy()
	end)
end

local function onInput(input, gameProcessed)
	if gameProcessed or debounce then return end

	local key = input.KeyCode
	debounce = true

	if key == Enum.KeyCode.E then
		if #spawnParts < 2 then
			spawnPart()
		end
	elseif key == Enum.KeyCode.R then
		highlightPlayers("R")
		task.wait(keySettings.R.Cooldown)
	elseif key == Enum.KeyCode.Q then
		applyDash()
	elseif key == Enum.KeyCode.Z then
		Ragdoll:FireServer()
	end

	debounce = false
end

--// Connections
UserInputService.InputBegan:Connect(onInput)
#3

This code looks fairly good actually. There’s only a few things I would do different.

  1. Always set parent last, reason why: Instance | Documentation - Roblox Creator Hub (Object Replication)
  2. Use more variables for code clarity

Edited code:

--// Services
local Players = game:GetService("Players")
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local UserInputService = game:GetService("UserInputService")
local Debris = game:GetService("Debris")
local TweenService = game:GetService("TweenService")

--// Modules
local keySettings = require(script.UserInputs)

--// Variables
local LocalPlayer = Players.LocalPlayer

local character = script.Parent
local humanoid = character:WaitForChild("Humanoid")
local rootPart = character:WaitForChild("HumanoidRootPart")

local partTemplate = ReplicatedStorage.SKILL.Spawner
local remote = ReplicatedStorage.REMOTES.TP
local Ragdoll = ReplicatedStorage.REMOTES.Ragdoll

-- Controls
local debounce = false
local teleportDebounce = false

-- Tables
local spawnParts = {}

-- Functions
local function cleanupPart(part)
	local partIndex = table.find(spawnParts, part)
	if partIndex then
		table.remove(spawnParts, partIndex)
	end
	
	Debris:AddItem(part, keySettings.E.DecayTime)
end

local function spawnPart()
	local clone = partTemplate:Clone()
	clone.Name = "Teleporter" .. tostring(#spawnParts + 1)
	clone.Material = Enum.Material.Neon
	clone.Color = Color3.fromRGB(77, 255, 0)
	clone.Position = rootPart.Position + Vector3.new(0, 3, 0)
	clone.Anchored = true
	clone.Parent = workspace

	-- Touched Event
	clone.Touched:Connect(function(hit)
		if teleportDebounce or #spawnParts < 2 then return end
		
		local touchingCharacter = hit.Parent
		if touchingCharacter == character then
			humanoid.WalkSpeed = 0
			teleportDebounce = true

			-- Teleport to the first other part
			for _, otherPart in spawnParts do
				if otherPart ~= clone then
					remote:FireServer(otherPart.Position)
					break
				end
			end

			task.wait(keySettings.E.Cooldown)
			humanoid.WalkSpeed = 16
			teleportDebounce = false
		end
	end)

	-- Triggered Event
	clone.TRY.Triggered:Connect(function()
		cleanupPart(clone)
	end)
	
	table.insert(spawnParts, clone)
end

local function highlightPlayers(key)
	for _, player in Players:GetPlayers() do
		if player == LocalPlayer then
			continue
		end
		
		local character = player.Character
		
		if character and character:FindFirstChild("HumanoidRootPart") then
			local highlight = Instance.new("Highlight")
			highlight.OutlineColor = Color3.fromRGB(0, 255, 17)
			highlight.Parent = character
			
			Debris:AddItem(highlight, keySettings[key].DecayTime)
		end
	end
end

local function applyDash()
	if not rootPart or not rootPart:FindFirstChild("RootAttachment") then
		warn("HumanoidRootPart or RootAttachment missing!")
		return
	end

	local velocity = Instance.new("LinearVelocity", rootPart)
	velocity.VectorVelocity = rootPart.CFrame.LookVector * 100
	velocity.Attachment0 = rootPart.RootAttachment
	velocity.Parent = rootPart
	
	Debris:AddItem(velocity, 3)
end

local function onInput(input, gameProcessed)
	if gameProcessed or debounce then return end

	debounce = true
	
	local key = input.KeyCode

	if key == Enum.KeyCode.E then
		if #spawnParts < 2 then
			spawnPart()
		end
	elseif key == Enum.KeyCode.R then
		highlightPlayers("R")
		task.wait(keySettings.R.Cooldown)
	elseif key == Enum.KeyCode.Q then
		applyDash()
	elseif key == Enum.KeyCode.Z then
		Ragdoll:FireServer()
	end

	debounce = false
end

--// Connections
UserInputService.InputBegan:Connect(onInput)

You also can use a dictionary to store all the functions for your keycodes, making it easier to customize functions later.

Alternate code:

local keyFunctions = {
	E = function()
		if #spawnParts < 2 then
			spawnPart()
		end
	end,
	
	R = function()
		highlightPlayers("R")
		task.wait(keySettings.R.Cooldown)
	end,
	
	Q = applyDash,
	
	Z = function()
		Ragdoll:FireServer()
	end,
}

local function onInput(input, gameProcessed)
	if gameProcessed or debounce then return end

	debounce = true
	
	local key = input.KeyCode
	local keyFunction = keyFunctions[key]
	
	if keyFunction then
		keyFunction()
	end

	debounce = false
end

--// Connections
UserInputService.InputBegan:Connect(onInput)
#4

too much functionality in one script even if its all input triggered. applyDash along with highlightPlayers and spawnParts is just weird. look into SRP

#5

I personally wouldn’t say it is too much, because as you said, it’s only for input triggered. Once they add more functions with more complexity sure you could move to a different architecture but right now I believe it’s good.

#6

better to structure it now than leave it for later

#7

Thank you for your reply. I will take these into account

#8

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