How do I write clean code?

You can write your topic however you want, but you need to answer these questions:

  1. What do you want to achieve? Keep it simple and clear!

Just have clean code, that’s it.

  1. What is the issue? Include screenshots / videos if possible!

Alot of my code isn’t the worst and alot of the time it actually works and it’s optimized, but it’s quite dirty. I think it’s because I rush too much and just want to get the code working first, then I tell myself “I’ll clean it up later” but never bother.

  1. What solutions have you tried so far? Did you look for solutions on the Developer Hub?

I’ve tried being more patient and writing clean code first but that didn’t work as I got too impatient.

Tried also using more functions and module scripts to make my cleaner. But it’s still SOOO messy compared to code I see from other people.

2 Likes

Please provide some examples of your recent code such that we can give detailed feedback.

Outside of that, I’d recommend these two videos https://youtu.be/Bf7vDBBOBUA?si=hn3HoODVYEUxp-Hk https://youtu.be/-J3wNP6u5YU?si=DhBu9Fs1XvX9RIkF

2 Likes

send us your code so we can help more

Reducing nesting is a great way, instead of using

local function CoolFunction()
    if condition then
        --do this cool little thing
    end
end

you can try doing this:

local function CoolFunction()
    if not condition then return end
    --do this cool little thing
end

And also just generally avoid repeating code that looks/functions similarly

1 Like

Here’s some example code of an NPC I made that patrols the area, goes to a player if its in its range. And can also deal with staircases and what not through pathfinding.

local pfs = game:GetService("PathfindingService")

local path = pfs:CreatePath({
	AgentRadius = 2,
	AgentHeight = 5,
	AgentCanJump = true,
	AgentCanClimb = false,
	WaypointSpacing = 4
})

local RunService = game:GetService("RunService")

local righuman = workspace.Rig.Humanoid

local rigtorso = workspace.Rig.HumanoidRootPart

local maxdistance = 40

local db = false

local function ReturnRaycast(returnedtarget)
	if returnedtarget then
		return returnedtarget
	end
end

local function NewRaycast(target)
if target then
		local params = RaycastParams.new()
		params.FilterType = Enum.RaycastFilterType.Exclude
		params.FilterDescendantsInstances = {workspace.Rig}
		local direction = (target.PrimaryPart.Position - workspace.Rig.PrimaryPart.Position)
		local raycast = workspace:Raycast(rigtorso.Position, direction, params)
		if raycast then
			return raycast
		end
end
end

local function FindVictim()
	local victims = workspace:GetChildren()
	for i, victim in victims do
		if victim:FindFirstChildOfClass("Humanoid") and victim ~= workspace.Rig then
			local target = victim
			local distance = (rigtorso.Position - target.HumanoidRootPart.Position).Magnitude
			if distance <= maxdistance then
				return target
			end
		end
	end
end

while true do
	local newtarget = FindVictim()
	local returnedtarget = NewRaycast(newtarget)
	if returnedtarget and not returnedtarget.Instance.Parent:FindFirstChildOfClass("Humanoid") and db == true then
		path:ComputeAsync(rigtorso.Position, newtarget.PrimaryPart.Position)
	else
		path:ComputeAsync(rigtorso.Position, workspace.Part.Position)
	end
	if path.Status == Enum.PathStatus.Success then
		for i, wp in path:GetWaypoints() do
			local target = FindVictim()
			local raycast = NewRaycast(target)
			if target and raycast and raycast.Instance.Parent:FindFirstChildOfClass("Humanoid") then
				righuman:MoveTo(raycast.Position)
				db = false
				break
			elseif target and raycast and not raycast.Instance.Parent:FindFirstChildOfClass("Humanoid") and db == false then
				db = true
				break
			elseif target and raycast and not raycast.Instance.Parent:FindFirstChildOfClass("Humanoid") and db == true then
				local part = Instance.new("Part", workspace)
				part.Size = Vector3.new(.5,.5,.5)
				part.Material = "Neon"
				part.Shape = "Ball"
				part.Anchored = true
				part.CanCollide = false
				part.CanQuery = false
				part.Position = wp.Position + Vector3.new(0,2,0)
				if wp.Action == Enum.PathWaypointAction.Jump then
					righuman.Jump = true
				end
				righuman:MoveTo(wp.Position)
				righuman.MoveToFinished:Wait()
			else
				local part = Instance.new("Part", workspace)
				part.Size = Vector3.new(.5,.5,.5)
				part.Material = "Neon"
				part.Shape = "Ball"
				part.Anchored = true
				part.CanCollide = false
				part.CanQuery = false
				part.Position = wp.Position + Vector3.new(0,2,0)
				if wp.Action == Enum.PathWaypointAction.Jump then
					righuman.Jump = true
				end
				righuman:MoveTo(wp.Position)
				righuman.MoveToFinished:Wait()
			end
		end
	end
	task.wait()
end

Personally this is just how I would make it look easier to read

local pfs = game:GetService("PathfindingService")
local RunService = game:GetService("RunService") -- PUT SERVICES AT THE TOP

local path = pfs:CreatePath({
	AgentRadius = 2,
	AgentHeight = 5,
	AgentCanJump = true,
	AgentCanClimb = false,
	WaypointSpacing = 4
})

local righuman = workspace.Rig.Humanoid -- VARIABLES THAT ARE SIMILAR STAY TOGETHER
local rigtorso = workspace.Rig.HumanoidRootPart

local maxdistance = 40

local db = false

local function ReturnRaycast(returnedtarget)
	if returnedtarget then
		return returnedtarget
	end
end

local function NewRaycast(target)
	if target then
		local params = RaycastParams.new() --- AGAIN, SIMILAR VARIABLES STAY TOGETHER
		params.FilterType = Enum.RaycastFilterType.Exclude
		params.FilterDescendantsInstances = {workspace.Rig}
	
		local direction = (target.PrimaryPart.Position - workspace.Rig.PrimaryPart.Position) -- these ones arent like the above ones so they get their own space :)
		local raycast = workspace:Raycast(rigtorso.Position, direction, params)
		
		if raycast then
			return raycast
		end
	end
end

local function FindVictim()
	local victims = workspace:GetChildren()
	
	for i, victim in victims do --- IMO FOR LOOPS SHOULD BE SEPARATED FROM ABOVE VARIABLES
		if victim:FindFirstChild("Humanoid") and victim ~= workspace.Rig then
			local target = victim
			local distance = (rigtorso.Position - target.HumanoidRootPart.Position).Magnitude
			
			if distance <= maxdistance then --- IF STATMENTS **USUALLY* LOOK BETTER SEPARATED FROM VARIABLES
				return target
			end
		end
	end
end

local function createBall(wp) --- ADDED BECAUSE YOU DO THIS TWICE LOWER IN THE SCRIPT
	local part = Instance.new("Part", workspace)
	part.Size = Vector3.new(.5,.5,.5)
	part.Material = "Neon"
	part.Shape = "Ball"
	part.Anchored = true
	part.CanCollide = false
	part.CanQuery = false
	part.Position = wp.Position + Vector3.new(0,2,0)
	
	if wp.Action == Enum.PathWaypointAction.Jump then
		righuman.Jump = true
	end
	righuman:MoveTo(wp.Position)
	righuman.MoveToFinished:Wait()
end


while true do
	local newtarget = FindVictim()
	local returnedtarget = NewRaycast(newtarget)
	
	if returnedtarget and not returnedtarget.Instance.Parent:FindFirstChild("Humanoid") and db == true then
		path:ComputeAsync(rigtorso.Position, newtarget.PrimaryPart.Position)
	else
		path:ComputeAsync(rigtorso.Position, workspace.Part.Position)
	end
	
	if path.Status == Enum.PathStatus.Success then -- unrelated if statements get separated from other if statements too.. imo
		for i, wp in path:GetWaypoints() do
			local target = FindVictim()
			local raycast = NewRaycast(target)
			
			if target and raycast and raycast.Instance.Parent:FindFirstChild("Humanoid") then
				righuman:MoveTo(raycast.Position)
				db = false
				break
			elseif target and raycast and not raycast.Instance.Parent:FindFirstChild("Humanoid") and db == false then
				db = true
				break
			elseif target and raycast and not raycast.Instance.Parent:FindFirstChild("Humanoid") and db == true then
				createBall(wp)
			else
				createBall(wp)
			end
		end
	end
	
	task.wait()
end

Thanks for the tips.

30char