Sentry Review! Any Suggestions to improve the code?

[dzhann757 (Apollo)]

local TweenService = game:GetService("TweenService")
local SoundService = game:GetService("SoundService")
local Players = game:GetService("Players")
local Debris = game:GetService("Debris")

local Sentry = script.Parent
local Sentry_Head = Sentry:WaitForChild("Sentry_Head")

local Sentry_SFX = Sentry:WaitForChild("Sentry_SFX")
local Sentry_Fire = Sentry_SFX:WaitForChild("Sentry_Fire")
local Sentry_Idle = Sentry_SFX:WaitForChild("Sentry_Idle")
local Sentry_Target_Spotted = Sentry_SFX:WaitForChild("Sentry_Target_Spotted")

local Sentry_Guns = Sentry:WaitForChild("Sentry_Guns")
local Sentry_Gun_Holder = Sentry_Guns:WaitForChild("Sentry_Gun_Holder")
local Ray_Origin = Sentry_Gun_Holder:WaitForChild("Ray_Origin")
local Point_Light = Ray_Origin:WaitForChild("PointLight")

local Sentry_Module = require(Sentry:WaitForChild("Sentry_Module"))

local Idle = true
local Target_Detected = false

local MAX_RANGE = 100
local COOLDOWN = 0.1
local ROTATE_TIME = 0.2
local v101 = Vector3.new(1, 0, 1)

local function Fire_Flash()
	Point_Light.Enabled = true
	task.wait(0.1)
	Point_Light.Enabled = false
end

local function Get_Nearest_Player()
	local Magnitudes = {}
	local Target_Player = nil
	
	for i, player in Players:GetPlayers() do
		if not player.Character or not player.Character:FindFirstChild("HumanoidRootPart") then continue end
		
		local character = player.Character or player.CharacterAdded:Wait()
		local HRP = character:FindFirstChild("HumanoidRootPart")
		
		local Distance = player:DistanceFromCharacter(Sentry_Head.Position)
		if Distance > MAX_RANGE then continue end
		
		table.insert(Magnitudes, {Distance, player})
	end
	
	table.sort(Magnitudes, function(a, b)
		return a[1] < b[2]
	end)
	
	if #Magnitudes <= 0 then return nil end
	Target_Player = Magnitudes[1][2]
	
	return Target_Player
end

local Idle_Rotation = task.spawn(function()
	while true do
		if Idle == false then task.wait() continue end
		task.spawn(Sentry_Module.Play_Sentry_SFX, Sentry_Idle)
		
		for i = 0, 4, 1 do
			if Idle == false then break end
			
			local Random_Rotation = math.random(-45, 45)
			local Current_Sentry_Head_CFrame = Sentry_Head.CFrame
			local New_Sentry_Head_CFrame = CFrame.fromMatrix(Current_Sentry_Head_CFrame.Position, (Current_Sentry_Head_CFrame.RightVector*v101).Unit, (-(Current_Sentry_Head_CFrame.LookVector*v101).Unit:Cross((Current_Sentry_Head_CFrame.RightVector*v101).Unit)).Unit, -(Current_Sentry_Head_CFrame.LookVector*v101).Unit)
			-- New_Sentry_Head_CFrame wasn't written by me, I asked someone for help on it.

			local Rotate_Sentry_Head = TweenService:Create(Sentry_Head, TweenInfo.new(1, Enum.EasingStyle.Linear), {CFrame = New_Sentry_Head_CFrame * CFrame.Angles(0, math.rad(Random_Rotation), 0)})
			Rotate_Sentry_Head:Play()
			Rotate_Sentry_Head.Completed:Wait()

			task.wait(ROTATE_TIME)
		end
	end
end)

while true do
	local Target_Player = Get_Nearest_Player()

	if Target_Player == nil then
		if Idle == false then
			Idle = true
		end
		
		if Target_Detected == true then
			Target_Detected = false
		end
	else
		repeat task.wait() until Target_Player.Character or Target_Player.CharacterAdded:Wait()

		if not Target_Player.Character or not Target_Player.Character:FindFirstChild("HumanoidRootPart") then return end

		local Target_Character = Target_Player.Character or Target_Player.CharacterAdded:Wait()
		local Target_HRP = Target_Character:FindFirstChild("HumanoidRootPart")

		if not Target_Character or not Target_HRP then return end
		
		local Origin = Ray_Origin.WorldCFrame.Position
		local Direction = Target_HRP.Position - Origin

		local RaycastParameters = RaycastParams.new()
		RaycastParameters.FilterType = Enum.RaycastFilterType.Exclude
		RaycastParameters.FilterDescendantsInstances = {Sentry}
		RaycastParameters.IgnoreWater = true

		local RayResults = workspace:Raycast(Origin, Direction, RaycastParameters)

		if not RayResults then
			print("Hit Nothing!")
		else
			local Hit_Instance = RayResults.Instance
			local Hit_Position = RayResults.Position
			local Hit_Distance = RayResults.Distance

			if Hit_Instance.Name == "Handle" and Hit_Instance.Parent:IsA("Accessory") and Players:GetPlayerFromCharacter(Hit_Instance.Parent.Parent) or Players:GetPlayerFromCharacter(Hit_Instance.Parent) then
				task.spawn(function()
					if Idle == false then return end
					Idle = false
				end)
				
				task.spawn(function()
					if Target_Detected == true then return end
					Target_Detected = true
					
					if Sentry:FindFirstChild(Sentry_Idle.Name) then
						Sentry:FindFirstChild(Sentry_Idle.Name):Destroy()
					end
					
					Sentry_Module.Play_Sentry_SFX(Sentry_Target_Spotted)
				end)

				local LookAt_CFrame = CFrame.lookAt(Sentry_Head.Position, Hit_Position)

				task.spawn(Sentry_Module.Play_Sentry_SFX, Sentry_Fire)
				task.spawn(Fire_Flash)
				Sentry_Module.Create_Visualizer(Origin, Hit_Position, Hit_Distance, Color3.fromRGB(0, 255, 0))
				TweenService:Create(Sentry_Head, TweenInfo.new(0.25, Enum.EasingStyle.Linear), {CFrame = LookAt_CFrame}):Play()
			else
				if Target_Detected == true then
					Target_Detected = false
				end
				
				if Idle == false then
					Idle = true
				end
				
				Sentry_Module.Create_Visualizer(Origin, Hit_Position, Hit_Distance, Color3.fromRGB(255, 0, 0))
			end
		end
	end

	task.wait(COOLDOWN)
end

I finished this scripting this sentry with a little bit of help and I wanted to see if there is anything that I could improve!

[x4q_b (oliver)]

the characteradded yield isnt necessary because just above you return if there isn’t a character. there’s no need for a second check, your code already stops if there isn’t a character.

if not Target_Player.Character or not Target_Player.Character:FindFirstChild("HumanoidRootPart") then return end

local Target_Character = Target_Player.Character or Target_Player.CharacterAdded:Wait()

also at

table.sort(Magnitudes, function(a, b)
		return a[1] < b[2]
	end)

you’re comparing distance which is a number to a player. that comparison will not work. the player is an object.

what i saw is this while true loop that will always run regardless of whether or not the sentry is still there. you should break the loop if the sentry gets deleted in any way so it doesnt error.

local Idle_Rotation = task.spawn(function()
	while true do

and lastly i saw some need of grouping in your operator usage

if Hit_Instance.Name == "Handle" and Hit_Instance.Parent:IsA("Accessory") and Players:GetPlayerFromCharacter(Hit_Instance.Parent.Parent) or Players:GetPlayerFromCharacter(Hit_Instance.Parent) then

so this will cause any ray that hits a player’s character regardless if its an accessory.
you can fix it by doing this

if ((Hit_Instance.Name == "Handle" 
    and Hit_Instance.Parent:IsA("Accessory") 
    and Players:GetPlayerFromCharacter(Hit_Instance.Parent.Parent)))
    or Players:GetPlayerFromCharacter(Hit_Instance.Parent) then

proper grouping is important so yeah

other than that your code is very well structured and should work better with these changes

[dzhann757 (Apollo)]

Thanks for some suggestions! I accidentally made a mistake on the table.sort() part and I didn’t know you can do that with if statements!

[x4q_b (oliver)]

the grouping looks complicated sometimes but most of the time it’s really just checking all your operators and the organization of them

[dzhann757 (Apollo)]

I didn’t know you can group it like that though

[dzhann757 (Apollo)]

Wait, I have a question though. I added print statements to the while true do loop and when I destroyed the sentry, it stopped printing. Why do I have to add a check for the while true do loop if it didn’t error?

[x4q_b (oliver)]

its precautions.
if your sentry gets deleted in a while true loop it will spam a bunch of errors in your console which can hinder your debugging

[dzhann757 (Apollo)]

Oh, alright but it didn’t error though. It was supposed to right..?

[x4q_b (oliver)]

if your script can’t find the sentry and you don’t use a findfirstchild check, it will spam errors.
if you check if it exists first you will be fine

[dzhann757 (Apollo)]

Oh alright! Good to know! >:D (New thing learned)