Help cleaning up / improving module

[simplyjustbased]

Basically first time working with humanoids and stuff, and wanna know if theres ways i can improve / clean up code on this module

Code

-- The NextBot Handler Module --
local Players = game.Players
local DisableAllBots = false

local PathFindingService = game:GetService("PathfindingService")
local RunService = game:GetService("RunService")
local debris = game:GetService("Debris")

local Touched = {}

-- Debugs, For each # of debugs enabled, the more information given.
local Debug = false -- Holds alot of the looped prints
local Debug2 = false -- Extra information on most of the looped prints
local Debug3 = false -- Mainly the non-looped prints for information, also used for viewing the path if using pathfinding

-- Mainly for pathfinding, uses runservices instead of while task.wait() do
local UseRunService = true

-- Functions

local function FindNearest(NBC)
	local NearestPlayer
	local NearestDistance
	
	for _, Player in (Players:GetPlayers()) do
		if Player.Character and Player.Character:FindFirstChild("Humanoid").Health > 0 then
			local Mag = (Player.Character.HumanoidRootPart.Position - NBC.Position).Magnitude
			
			if not Player.Character.IsInSafeZone.Value then
				if NearestDistance then
					if Mag < NearestDistance then NearestDistance = Mag; NearestPlayer = Player.Character end
				else
					NearestDistance = Mag; NearestPlayer = Player.Character
				end		
			end
		end
	end
	
	return NearestPlayer, NearestDistance
end

local function MoveToMovement(NextBot, ValFol)
	local Stepped
	local NearestPlayer, NearestDistance
	
	Stepped = RunService.Heartbeat:Connect(function(DTime)
		if NextBot.Humanoid.Active.Value and NextBot.Humanoid.Health > 0 then
			NearestPlayer, NearestDistance = FindNearest(NextBot.Character)

			if NearestPlayer and NearestDistance then
				if NearestDistance <= ValFol.DetectionDistance.Value then
					NextBot.Humanoid:MoveTo(NearestPlayer.HumanoidRootPart.Position)
				end
			end
		else
			Stepped:Disconnect()
		end
	end)
end


local function pfm(NextBot)
	if NextBot.Humanoid.Health <= 0 then warn("NextBot Is Dead / Health is equal to or below 0") return end

	local Waypoints
	local NextWaypointIndex
	local reachedConnection
	local blockedConnection 

	local NearestPlr, NearestDistance = FindNearest(NextBot.Character)
	
	local Path = PathFindingService:CreatePath({
		
		['AgentRadius'] = 5,
		['AgentHeight'] = 5,
		['AgentCanJump'] = true,
		
		Costs = {
			Water = 20,
			SmoothPlastic = 1,
			ForceField = math.huge,
			Untouchable = math.huge
		}	
	})
		
	
	if not NearestPlr or NearestDistance > NextBot.ChangeableValues.DetectionDistance.Value then
		return "No Near Players"
	else
		local Yes, No = pcall(function()
			Path:ComputeAsync(NextBot.HumanoidRootPart.Position, NearestPlr.HumanoidRootPart.Position)
		end)


		if Yes and Path.Status == Enum.PathStatus.Success then
			Waypoints = Path:GetWaypoints()

			blockedConnection = Path.Blocked:Connect(function(BwI)
				if BwI >= NextWaypointIndex then
					blockedConnection:Disconnect()
					pfm(NextBot)
				end
			end)


			for	i, v in ipairs(Waypoints) do

				if v.Action == Enum.PathWaypointAction.Jump then
					NextBot.Humanoid.Jump = true
				end

				if Debug3 then
					local Part = Instance.new("Part")
					Part.Shape = Enum.PartType.Ball
					Part.Size = Vector3.new(1,1,1)
					Part.Anchored = true
					Part.Position = v.Position
					Part.Material = Enum.Material.Neon
					Part.Color = Color3.new(0,4,0)
					Part.CanCollide = false
					Part.Transparency = 0.5
					Part.Parent = workspace.Extras
				end

				NextBot.Humanoid:MoveTo(v.Position)
				local Check = NextBot.Humanoid.MoveToFinished:Wait()
				
				if not Check then
					pfm(NextBot)
					print("Uncheck")
					break
				end
				
				if (Waypoints[#Waypoints].Position - NearestPlr.HumanoidRootPart.Position).Magnitude > NextBot.ChangeableValues.DetectionDistance.Value then
					pfm(NextBot)
					print(NearestPlr.Name .. " is to far")
					break
				end
			end
			if Debug then print("Succesfully Moved To waypoint!  [Debug]: " .. NearestPlr.Name .. " Which's Distance was " .. NearestDistance .. if Debug2 then " # Of Waypoints: " .. #Waypoints else " [Set Debug2 to true for more information]") end
		else
			 if Debug then warn("Failed To Compute A Valid Path, Did you make sure that the NextBot Isnt Trapped / Stuck?: " .. if No then No else "Nil") end
		end
	end
end

local function PathFindingMovement(NextBot)
	local Run
	
	if UseRunService then
		Run = RunService.Heartbeat:Connect(function()
			if NextBot.Humanoid.Active.Value then
				if not DisableAllBots then
					local D = pfm(NextBot)
					
					if D and Debug then
						print(D)
					end
				end
			else
				Run:Disconnect()
			end
		end)
	else
		while task.wait() do
			if NextBot.Humanoid.Active.Value then
				if not DisableAllBots then
					local D = pfm(NextBot)

					if D and Debug then
						print(D)
					end						
				end		
			else
				break
			end
		end		
	end
end

local function GotTouched(NextBot, WhatIsTouched)
	local Player
	local IsPlayer
	local ValFol = NextBot.ChangeableValues

	if WhatIsTouched and Players:GetPlayerFromCharacter(WhatIsTouched.Parent) then
		Player = WhatIsTouched.Parent
		IsPlayer = true			
	else
		IsPlayer = false
	end

	if IsPlayer and Player then
		if Player.Humanoid.Health > 0 then
			if not table.find(Touched, Player) then
				table.insert(Touched, Player)
				Player.Humanoid:TakeDamage(ValFol.Damage.Value)
				task.wait(ValFol.DmgCooldown.Value)
				table.remove(Touched, table.find(Touched, Player))
			end
		end
	end
end

local function HealthSetup(NextBot)
	if not NextBot.ChangeableValues.CanBeKilled.Value then if Debug3 then NextBot.Character.HealthGui.Enabled = false; warn("You have re-setup the nextbot, and changed the CanBeKilled Value, To nolonger see this warn disable Debug3") end return end
	local HealthGui = NextBot.Character.HealthGui
	local HealthChangedFunc
	local Died
	
	HealthGui.Enabled = true
	HealthGui.TextLabel.Text = NextBot.Humanoid.MaxHealth .. " / " .. NextBot.Humanoid.Health
	
	HealthChangedFunc = NextBot.Humanoid.HealthChanged:Connect(function()
		HealthGui.TextLabel.Text = NextBot.Humanoid.MaxHealth .. " / " .. NextBot.Humanoid.Health
		HealthGui.Frame.Frame.Size = UDim2.new(NextBot.Humanoid.Health / NextBot.Humanoid.MaxHealth, 0,1)
	end)
	
	Died = NextBot.Humanoid.Died:Connect(function()
		HealthChangedFunc:Disconnect()
		Died:Disconnect()
	end)
end

-- Module
local Nbot = {}

Nbot.Setup = function(NextBot)
	NextBot.PrimaryPart:SetNetworkOwner(nil)
	local ValFol = NextBot.ChangeableValues

	local NextBotSpeed = ValFol.Speed.Value
	local NextBotJumpPower = ValFol.JumpPower.Value
	local NextBotHealth = ValFol.Health.Value
	local CanBeKilled = ValFol.CanBeKilled.Value
	local Pic = ValFol.Image.Value
	local TouchedFunc
	local Died
	
	local NBotHumanoid = NextBot.Humanoid
	
	NBotHumanoid.WalkSpeed = NextBotSpeed
	NBotHumanoid.JumpPower = NextBotJumpPower
	
	if CanBeKilled then
		NBotHumanoid.MaxHealth = NextBotHealth; NBotHumanoid.Health = NextBotHealth
		HealthSetup(NextBot)
	else
		local ff = Instance.new("ForceField", NextBot)
		ff.Visible = false
		NBotHumanoid.MaxHealth = math.huge; NBotHumanoid.Health = math.huge
	end
	
	NextBot.Character.Front.ImageLabel.Image = "rbxassetid://" .. Pic
	NextBot.Character.Back.ImageLabel.Image = "rbxassetid://" .. Pic
	
	if not NextBot.Humanoid:GetAttribute("Active") then
		NextBot.Humanoid:SetAttribute("Active", false)
	end
	
	if NextBot:FindFirstChild("Hitbox") then
		TouchedFunc = NextBot.Hitbox.Touched:Connect(function(Thing)
			if DisableAllBots == false then
				GotTouched(NextBot, Thing)				
			end
		end)
	end
	
	Died = NextBot.Humanoid.Died:Connect(function()
		NextBot.Character.CanCollide = true
		debris:AddItem(NextBot, 3)
		
		TouchedFunc:Disconnect()
		Died:Disconnect()
	end)
	
	return "Setup Complete"
end

Nbot.Start = function(NextBot)
	local ValFol = NextBot.ChangeableValues
	NextBot.Humanoid.Active.Value = true

	if ValFol.SmartAi.Value then
		PathFindingMovement(NextBot)
		if Debug3 then print("Smart Movement Activated") end
	else
		MoveToMovement(NextBot, ValFol)
		if Debug3 then print("Basic Movement Activated") end
	end	
	
end

Nbot.Stop = function(NextBot)
	NextBot.Humanoid.Active.Value = false
end

Nbot.DisableAllBots = function()
	if DisableAllBots then
		if Debug3 then warn("Bots are already disabled") end
		return
	end
	DisableAllBots = true
	print("Bots Disabled")
	return 
end

Nbot.EnableAllBots = function()
 
	if not DisableAllBots then
		if Debug3 then warn("Bots are already enabled") end
		return 
	end
	DisableAllBots = false
	print("Bots Enabled")
	return
end

return Nbot

People might not be sure what exactly some stuff does so heres a rblx file of the basics and of the bot and stuff
NBotTest.rbxl (52.1 KB)

Check out the readme in the NextBot

Notes

• For some reason Image is broken, not sure why
• Sometimes pushes player, even though everything has CanCollide off

Most Recent Edit: Changed code to newer code with help from @kingerman88 , and attached a newer version of the place with the updated code
(small edit)
Updated a few bits of the code again, not to big where ill update recent edit

[kingerman88]

Some general coding practices feedback

---

local PathFindingService = game:GetService("PathfindingService")
local RunService = game:GetService("RunService")
local debris = game:GetService("Debris") -- AHHHHH why pascal case for only that one

---

Within your code you do a lot of this

if UseRunService == true then
-- or
if Debug == true then

You can actually shorten this to

if UseRunService then
-- / 
if Debug then
-- since it's the same as:
if true then / if false then

---

If the humanoid doesn’t exist this will error as you’ll be indexing the Humanoid, I’d switch to Player.Character:FindFirstChild("Humanoid")

if Player.Character and Player.Character.Humanoid

---

@simplyjustbased

• For some reason Image is broken, not sure why

Probably because the website Id does not equal the rbxassetid
Ex:
https://www.roblox.com/library/7038660754/Images-UIelement - Decal Website link
http://www.roblox.com/asset/?id=7038660718 - Actual id

You’ll probably want to use InsertService

[simplyjustbased]

@kingerman88

You can actually shorten this to

if UseRunService then
-- / 
if Debug then
-- since it's the same as:
if true then / if false then

Alright, will definitely clean up some areas with this

@kingerman88

If the humanoid doesn’t exist this will error as you’ll be indexing the Humanoid, I’d switch to Player.Character:FindFirstChild("Humanoid")

Oh, i mainly would just do if this and this.whateveriwanted, and it worked, althought i think the method you requested is better so ill change those aswell.

@kingerman88

Probably because the website Id does not equal the rbxassetid
Ex:
https://www.roblox.com/library/7038660754/Images-UIelement - Decal Website link
http://www.roblox.com/asset/?id=7038660718 - Actual id

You’ll probably want to use InsertService

Never knew this, thought i could just add ihe id and it would work

Thanks for the help, ill update the code in the post once im done changing it
(edit not sure how to use insertservice, but i got the other two done, updating script and place right now)

[Katrist]

Along with @kingerman88’s suggestions, you should use:

task.delay([number], function()
	instance:Destroy()
end)

instead of Debris:AddItem as Debris uses the old wait(n) instead of task.wait(n).

Another thing is when declaring unknown variables, instead of this:

local Value

do,

local Value = nil

It’s easier to read and also removes some warnings if you’re using --!strict.

The last improvement I can think of is to not use the second parameter of Instance.new(). You should always set parent last for performance reasons:

PSA: Don't use Instance.new() with parent argument

I’ve discovered a pretty bad performance issue in one of top games that has to do with Instance.new and wanted to write about this, since this is not obvious unless you know the system inside out. Tthere are several ways to create a ROBLOX object in Lua: local obj = Instance.new(‘type’); fill obj fields local obj = Instance.new(‘type’, parent); fill obj fields local obj = util.Create(‘type’, { field1 = value1, … }) If you care at all about performance, please only use the first option - I wi…

[simplyjustbased]

@Katrist

task.delay([number], function()
	instance:Destroy()
end)

Wouldn’t this pause the pathfinding to delete the part?

@Katrist

Another thing is when declaring unknown variables, instead of this:

local Value

do,

local Value = nil

It’s easier to read and also removes some warnings if you’re using --!strict.

Alright, may do this as i dont even know what

@Katrist

–!strict

is

@Katrist

The last improvement I can think of is to not use the second parameter of Instance.new(). You should always set parent last for performance reasons:

Alright, completely forgot about this will update the code to have this!

[Katrist]

@simplyjustbased

Wouldn’t this pause the pathfinding to delete the part?

I’m talking about the parts where you use Debris:AddItem(item, number). Debris:AddItem() deletes the item after the specified time but it uses wait(n) instead of task.wait(n) for scheduling.

Just replace those parts with:

task.delay(number, function()
	item:Destroy()
end)

[Katrist]

@simplyjustbased

Alright, may do this as i dont even know what

It’s a luau typechecking method:

Luau Type Checking Release

10 months ago, we’ve started upon the journey of helping Roblox scripters write robust code by introducing an early beta of type checking. We’ve received a lot of enthusiastic feedback and worked with the community on trying to make sure critical issues are addressed, usability is improved and the type system is ready for prime time. Today I’m incredibly excited to announce that the first release of Luau type checking is officially released! Thanks a lot to @Apakovtac, @EthicalRobot, @fun_enthu…

[simplyjustbased]

@Katrist

I’m talking about the parts where you use Debris:AddItem(item, number). Debris:AddItem() deletes the item after the specified time but it uses wait(n) instead of task.wait(n) for scheduling.

Just replace those parts with:

oh, will do, thought i had my cleaning script in the module but nah will use this 100%

@Katrist

It’s a luau typechecking method:

Hm never heard of this, ill check it out

[OptimizedFunction]

@simplyjustbased

Wouldn’t this pause the pathfinding to delete the part?

No, since task.delay(n, callback) calls the callback passed to it after n seconds have elapsed while not yielding the thread.