What are your thoughts of my Zombie-AI Script?

Hello my fellow scripters! I have written a small script that allows a zombie to follow the closest player that is within 100 studs. I wan’t to know what your thoughts are about the scripts, and ways I can improve it.

Disclaimer: I already know that I should have used PathfindingService instead of the :MoveTo() method. I written the sample just as a test run. In future programs I will use PathfindingService as my method for making a NPC follow a player. I also like to address the pcall() at the end is to ensure the code does not break if something bad goes wrong. Every 2 seconds a new target would be found if the current target is not the closest anymore, or if he left the server/crashed. Anyways, here is my script:

local zombie = script.Parent
local zomRoot = zombie:WaitForChild("HumanoidRootPart")
local zomHum = zombie:WaitForChild("Humanoid")

local function findTarget()
   local distance = 100
   local target
   
   for _, obj in pairs (workspace:GetChildren()) do
      if obj ~= zombie and obj:FindFirstChild("Humanoid") then
          local root = obj:FindFirstChild("HumanoidRootPart")
          local hum = obj:FindFirstChild("Humanoid")
          local magnitude = (zomRoot.Position - root.Position).magnitude
         
          if magnitude < distance then
              distance = magnitude
              target = hum
         end
      end
   end
   return target 
end

while true do
    wait(2)
    local success, errorMsg = pcall(function()
        local target = findTarget()
        if target then
            zomHum:MoveTo(target)
        else
            zomHum:MoveTo(zomRoot.Position + Vector3.new(math.random(50,50), 0, math.random(50,50)))
        end
    end)

    if errorMsg then
        warn(errorMsg)
    end

end
2 Likes

Instead of going through all of your workspace, why not use

for _, obj in pairs (game.Players:GetChildren()) do
    local Character = obj.Character
    local root = Character:WaitForChild("HumanoidRootPart")
    --And other stuff in here
end
5 Likes

Originally I was writing it this script for both players and NPCs, which is why I did not use the GetChildren() method inside the Players service. If I had wanted it only for real users, I should have used:

for _, obj in ipairs (game.Players:GetPlayers()) do
    --Remaining of code goes here
end

Additionally, you are right about the entire “Searching the entire workspace thing”. I should have used CollectionService, placed all NPCs in a folder (if I was including them), or better yet put them all in a table and iterate through them. I was being stupid. Thanks for the reply. I’ll be sure to not put in work that is not needed, and not mass search entire objects :smile:

2 Likes

Then you should store all of the npc and player positions in a table in some module script and iterate through that, …I just don’t like the thought of going through workspace…

2 Likes

I know. You replied too quickly. After my post I edited some areas of it to include more details. Here is my completed segment:

Additionally, you are right about the entire “Searching the entire workspace thing”. I should have used CollectionService, placed all NPCs in a folder (if I was including them), or better yet put them all in a table and iterate through them. I was being stupid. Thanks for the reply. I’ll be sure to not put in work that is not needed, and not mass search entire objects :smile:

2 Likes

You should replace the “script.Parent” in your if statement with the variable zombie and instead of defining hum literally even though you call the same method in the same way in the if condition, you can just place hum behind the if statement and have the if statement check hum instead.

No need to have the function create the variable for distance if it stays constant if it can go at the top but I suppose it doesn’t really matter.

I personally think your indentation gets a little weird in your function findTarget but if you like it that way it doesn’t matter.

Another thing you could do if you want to is to replace success with _ since you’re not using it as you did with the pairs iterator.

Other than code improvements, you could also do the moving only when the zombie is alive so it won’t try to chase things while dead.

You should replace the “script.Parent” in your if statement with the variable zombie and instead of defining hum literally even though you call the same method in the same way in the if condition, you can just place hum behind the if statement and have the if statement check hum instead.

Already done that in my script.

No need to have the function create the variable for distance if it stays constant if it can go at the top but I suppose it doesn’t really matter.

the distance does not stay constant. As I stated at the top of the post, “Every 2 seconds a new target would be found if the current target is not the closest anymore, or if he left the server/crashed.”. The distance is a constantly changing variable. The function also needs to create the variable distance so that a new distance is chosen each function run. There is no problem with having a variable keep track of the closest player (or if a player is within 100 studs) via the distance variable. There is a need to put the distance variable. Additionally, even if it stayed constant (which it does not) having a variable lets the script change values/properties from one line, without having to go on all occurances.

Other than code improvements, you could also do the moving only when the zombie is alive so it won’t try to chase things while dead.

My NPC would remove whenever he die. He will not try to chase the user after he is dead. This is the code to make him follow the player.

1 Like

Not sure what you mean by already done since I didn’t see that change before I replied but the distance variable part is my fault for not noticing.

I should warn you with this line:
zomHum:MoveTo(zomRoot.Position + Vector3.new(math.random(50,50), 0, math.random(50,50)))

I suggest you to combine it with raycasting and stuff to check if there is a wall or not. Because i did that mistake in my game, when there was more then 20-25 npcs that moves around, when they hit to a wall or something while moving, It was lagging the game alot and players’ ping were increasing alot. PhysicsService is kinda weird. Just remove it or like i said, combine it with raycasting or make your own custom humanoid.

Like I stated in my post, I know that I should have use PathFindingService instead of the :MoveTo method. This was just a test run. Thanks for the concern though