Npc attack pattern only works with one npc

So i made a script where if you are close enough to a npc it starts attacking you with pattern of attacks but for some reason it only works for one single npc

heres the code

server script

local Folder = game.Workspace.EnemyAiFolder
local AttackModule = require(script.AttackModuleScript)
local NpcHolder
game["Run Service"].Heartbeat:Connect(function()
	for _,Npc in pairs(Folder:GetChildren()) do
		NpcHolder = Npc
		return
	end
end)


while true do
	task.wait()
	if NpcHolder then
		AttackModule.Attack(NpcHolder.Attack,NpcHolder)
	end
end

Module #1

local Attackmodule = {}
local module = Attackmodule
local module2 = require(script.ModuleScript)
local AmountOfAttacksUntilCooldown = 4
local CurentAmountOfAttacksUntilCooldown = 0
local Frequency = 0.4
local Cooldown = 1

module.Attack = function(AttackVal,Npc)
	if AttackVal.Value == true then
		local deb = false
		if deb == false then
			CurentAmountOfAttacksUntilCooldown += 1
			deb = true
			module2.Hitbox(Npc)
			task.wait(Frequency)
			deb = false
			if CurentAmountOfAttacksUntilCooldown > AmountOfAttacksUntilCooldown then
				wait(Cooldown)
				CurentAmountOfAttacksUntilCooldown = 0 
			end
		end
	end
end


return Attackmodule

Module #2

local module = {}
module.Hitbox = function(Npc)
	local deb = false

	local Hitbox = Instance.new("Part")
	Hitbox.Parent = workspace
	Hitbox.Anchored = true
	Hitbox.CanCollide = false
	Hitbox.Size = Vector3.new(5,5,5)
	Hitbox.BrickColor = BrickColor.new("Really red")
	Hitbox.Transparency = 0
	Hitbox.Position = Npc.HumanoidRootPart.Position + Npc.HumanoidRootPart.CFrame.LookVector * 5
	game.Debris:AddItem(Hitbox,0.4)
	Hitbox.Touched:Connect(function(hit)
		if hit.Parent:FindFirstChild("Humanoid") and not hit.Parent:FindFirstChild("Attack") then
			if deb == false then
				deb = true
				hit.Parent:FindFirstChild("Humanoid"):TakeDamage(10)
				wait(0.5)
				deb = false
			end
		end
	end)
end
return module

Any help is appreciated

1 Like

The reason is simple. You are returning the function after it finds an NPC.
Your script is based on who the NpcHolder is and since it only takes 1 NPC and returns, it will only work with 1.

so you mean i need to get rid of the return in the runservice function?

i mean i did some changes in the script but it still works not in i way i wanted to

It would be best to have a script for each NPC.
Some may have different attacks, skills, etc.

i dont really know about that to be honest here

i just want it to be optimised

Example: You have NPC1 (this would be the ID).
You have 10x NPC1 in the game.
Program NPC1 in ServerStorage and clone it to the workspace at the spawn coordinate. You can modify 1 single model and all will be modified.

The model would be sent to the workspace and your scripts to the ServerScriptService.

Optimization does not mean less code, it means more organization, especially thinking on a large scale.

I think instead of making scripts for every npc you should make the function run everytime an npc spawns.

And for already existing npcs (so the ones in the workspace before u join a server) you run that function for each of them through a loop (which you did), and the reason why it worked only for one npc before is because you had a return in the loop which stops the function entirely, if you put continue it would skip the current iteration and go to the next one (I had issues like these too so dont worry :D).

Also i think a better way to do the “starting” loop is this

for _,Npc in pairs(Folder:GetChildren()) do
	AttackModule.Attack(Npc.Attack, Npc)
end

And I feel like you could just put that loop in the while loop instead
edit note: or you could put in the heartbeat but i feel that would be laggy, or you could make a detection system for every npc and then you run that for loop only once at the start of the game and when more npcs get added you run the function. Just a suggestion though, you can stick with whatever u find better

Also nothing important but why would you pass Npc.Attack and Npc as arguements? You could just do a variable for the attack val or not make one at all in the module1.Attack function

module.Attack = function(Npc)
     -- example 1
     if Npc.Attack.Value then
        --code
     end

    --example 2
    local AttackVal = Npc.Attack
    if AttackVal.Value then
        --code
    end
end

This is just my suggestion but feel free to ignore it, it still would work fine the way you made it

Optimization does mean more organized but also means less code. If you have multiple instances of a script doing the same thing it would ruin performance, the impact depends on how many scripts and how big the script is. Even if the impact isnt noticable its still slower than making one script work for every npc

hmmmmmmmmmm ill try using this