Help cleaning up / improving module

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 :+1:

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

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

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

1 Like

Alright, will definitely clean up some areas with this

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.

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 :smiley: :+1:
(edit not sure how to use insertservice, but i got the other two done, updating script and place right now)

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:

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

Alright, may do this as i dont even know what

is

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

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)

It’s a luau typechecking method:

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

Hm never heard of this, ill check it out

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

1 Like