Function inside of ModuleScript runs one extra time every time I call it?

[Bankrovers]

Hello, my Humanoid:MoveTo() function keeps repeating itself one more time every time I call attackModule.attackNPC, anyone know why?

local attackModule = {}

function attackModule.attackNPC(player: Player, nonPlayerCharacter: Model)
	local character: Model? = player.Character
	local humanoid: Humanoid? = if character and character:FindFirstChildOfClass("Humanoid") then character:FindFirstChildOfClass("Humanoid") else nil

	if not character or not humanoid then return end

	local primaryPart0: BasePart? = character.PrimaryPart
	local primaryPart1: BasePart? = nonPlayerCharacter.PrimaryPart

	if not primaryPart0 or not primaryPart1 then return end

	local playerSlots: Model = nonPlayerCharacter:FindFirstChild("PlayerSlots") :: Model
	if not playerSlots then warn("PlayerSlots not found!") return end

	local nearestPlayerSlotDistance: number = 100
	local nearestPlayerSlot: Part

	for _, playerSlot in playerSlots:GetChildren() do
		local magnitude: number = (playerSlot.Position - primaryPart0.Position).Magnitude
		if nearestPlayerSlotDistance >= magnitude then
			nearestPlayerSlotDistance = magnitude
			nearestPlayerSlot = playerSlot
		end
	end

	local function moveToNearestPlayerSlot()
		humanoid:MoveTo(nearestPlayerSlot.Position, nearestPlayerSlot)
	end

	moveToNearestPlayerSlot()

	local retries: number = 0

	local function onMoveToFinished(reached: boolean)
		if not reached and retries < 5 then
			retries += 1
			moveToNearestPlayerSlot()
		return end

		primaryPart0.CFrame = CFrame.lookAt(Vector3.new(primaryPart0.Position.X, primaryPart0.Position.Y, primaryPart0.Position.Z), primaryPart1.Position)

		player:SetAttribute("IsAttackLocked", true)

		print("finish")
	end

	humanoid.MoveToFinished:Connect(onMoveToFinished)

	humanoid:GetPropertyChangedSignal("MoveDirection"):Connect(function()
		player:SetAttribute("IsAttackLocked", false)
	end)
end

return attackModule

Output after calling the Module one time:

Output after calling the Module a second time:

[7z99 (cody)]

I don’t think it’s an issue with module that’s causing it to run multiple times, it’s likely an issue with your script. Could you show that?

[Bankrovers]

Alright,

local attackModule = require(game:GetService("ServerScriptService"):WaitForChild("AttackModule"))

local nonPlayerCharacter: Model = script.Parent
local nonPlayerCharacterPrimaryPart: BasePart? = nonPlayerCharacter.PrimaryPart

if not nonPlayerCharacterPrimaryPart then return end

local hitbox: Part = nonPlayerCharacter:FindFirstChild("Hitbox") :: Part
local clickDetector: ClickDetector = hitbox:FindFirstChildOfClass("ClickDetector") :: ClickDetector

local function onMouseClick(player: Player)
	attackModule.attackNPC(player, nonPlayerCharacter)
end

clickDetector.MouseClick:Connect(onMouseClick)

[7z99 (cody)]

Oh I see, you have a couple memory leaks in your original post. You’re creating a new connection every time the function is called but you aren’t disconnecting the old ones, so that’s why you’re seeing more prints than expected. You need to disconnect your connections when you don’t need them anymore:

@Bankrovers

humanoid.MoveToFinished:Connect(onMoveToFinished)

@Bankrovers

    humanoid:GetPropertyChangedSignal("MoveDirection"):Connect(function()
		player:SetAttribute("IsAttackLocked", false)
	end)

[Bankrovers]

Something like this?

	local connection
	local retries: number = 0

	local function onMoveToFinished(reached: boolean)
		if not reached and retries < 5 then
			retries += 1
			moveToNearestPlayerSlot()
		return end

		primaryPart0.CFrame = CFrame.lookAt(Vector3.new(primaryPart0.Position.X, primaryPart0.Position.Y, primaryPart0.Position.Z), primaryPart1.Position)

		player:SetAttribute("IsAttackLocked", true)

		print("finish")
		
		connection:Disconnect()
	end

	connection = humanoid.MoveToFinished:Connect(onMoveToFinished)

	humanoid:GetPropertyChangedSignal("MoveDirection"):Connect(function()
		player:SetAttribute("IsAttackLocked", false)
	end)

[7z99 (cody)]

Yeah that looks like it would work but you need to disconnect your humanoid:GetPropertyChangedSignal connection as well (though this wouldn’t change anything at first glance but if you keep calling the function you’ll notice memory will go up over time unless the humanoid is destroyed, but in which case you’re still creating more connections than intended).

[Bankrovers]

I’m not sure how to disconnect non-local functions, how’d I go about doing this?

[7z99 (cody)]

You’d just do it how you’d disconnect any other connection, so something like:

local moveConnection, propertyConnection

-- ...

local function onMoveToFinished(reached: boolean)
    -- ...
    moveConnection:Disconnect()
    propertyConnection:Disconnect()
end

moveConnection = humanoid.MoveToFinished:Connect(onMoveToFinished)

propertyConnection = humanoid:GetPropertyChangedSignal('MoveDirection'):Connect(...)