Function occuring cumulatively per call

Edit: grammar and code copy/pasteability, code snippet works when applied in workspace without duplicating. Origin code is too large to paste and describe quickly, will update upon request

Hey there, still semi-new to scripting and I appear to be the victim of bad writing, though I’m not sure how to fix the practice.

When I call a function when criteria is met, the function fires an additional time every time it’s called. Unsure if this qualifies as a data leak as I’m not well versed in the coding language yet but it’s causing me issues, here’s an abstract example of the code:

function Function()
	if criteria == true then
		print("Printed")
	else 
		return
	end
end

ActivationRequirementMet:Connect(Function)

Now every time I meet the activation requirement, the function fires multiple times, for example:

Printed
Printed(x2)
Printed(x3)
etc...

To move from the abstract to my specific issue in the matter, here is the least complex example I have:

local Enabled = true
local Combo = 1
local Mouse =  game.Players.LocalPlayer:GetMouse() -- its a localscript

function canHit() 
	if Enabled == true and
		1 == 1 --other conditions are here, removed for reading ease
		return true
	else
		return false
	end
end

function basicAttack()
	if Tool.Parent ~= Character then return end --delete if testing, though it may be the cause of the issue
	if canHit() == true then
		if Combo == 1 then
			Enabled = false
			anim1:Play() -- anim is specified in locals above, delete if you test in your workspace
			print("hitstart")
			anim1:GetMarkerReachedSignal("hitEnds"):Connect(function() -- replace with wait() if you test in workspace
				Enabled = true
			end)
		end
	else 
		print("Attack Denied.")
		return
	end
end

Mouse.Button1Down:Connect(basicAttack)

The output; note the “block” is used to disrupt the output so that the repeating effect is shown. Every instance of “hitstart” is a click.

  23:10:30.111  hitstart  -  Client - Main:102
  23:10:32.927  block  -  Client - Main:160
  23:10:33.862   ▶ hitstart (x2)  -  Client - Main:102
  23:10:35.494  block  -  Client - Main:160
  23:10:36.529   ▶ hitstart (x3)  -  Client - Main:102
  23:10:38.211  block  -  Client - Main:160
  23:10:38.944   ▶ hitstart (x4)  -  Client - Main:102

I feel that this is a fundamental topic, somehow I’ve missed it throughout my readings within lua, any references to reading material would be appreciated. I’ve searched the devhub and the (gasp) third page of google and havent found anything similar to this issue, though I may be searching wrong.

Thanks in advance. If you’d like to reply with a specific code, keeping it abstract would be preferred.

It seems like you are reconnecting events without disconnecting them. One way this could be caused is if you are running the line:

Mouse.Button1Down:Connect(basicAttack)

each time you click, causing the event to fire one more time for each subsequent click. This is a type of leak. You also have the same issue here:

anim1:GetMarkerReachedSignal("hitEnds"):Connect(function()
	Enabled = true
end)

If you are not destroying anim1 after use, this event will connect multiple times and fire once more each time the function is run.

Be sure to only connect your events only once, or to disconnect when you are finished with them if the object they are connected to is not destroyed.

I’m not too sure what you mean by disconnecting the event, I may be grossly uneducated in this topic; I thought events were disconnected whenever they were ended/returned?

anim1 is stored and loaded within a seperate part of the script, only called upon there to play the animation and listen for animation markers. Would storing that snippet outside of the function fix the issue?

AFAIK, events are only automatically disconnected when their Instance is destroyed. You have to manually disconnect an event in any other case:

local connection

connection = Instance.Signal:Connect(function() -- connection is defined with the event's RBXScriptSignal
    print("Signal detected")
    connection:Disconnect() -- disconnects the signal
end)

Seems like it might

That fix unfortunately causes the function to only be able to fire once; the context is that the code I’d like to execute is within a tool, with the ability to be executed multiple times (without duplicating), I have other instances where I’m doing the same thing however it doesn’t have a leak, I can’t quite tell what’s causing the difference however as both are structured largely the same, spanning ~30-80 lines of code. I’m redesigning my old system to be more efficient and easily modified for variations, however I’ve encountered leaking this time.

For your case, I’ll suggest something such as this:

local connection

if connection then -- checks if the connection variable has a signal attached
    connection:Disconnect() -- disconnects the old signal
end

-- creates and attaches a new signal to the connection variable
connection = Instance.Signal:Connect(function()
   -- this creates a new signal without having to worry about memory leaks
   -- since the if statement prior to this, checks for any old events and disconnects those
   -- but still allows for new connections to be created
end)

Make sure your connection variable is pre-defined outside of the function

1 Like

Please post the code surrounding the event connections if you still need help. Events do not automatically get disconnected after they return, they continue to fire until they are either manually disconnected, or the object they are attached to, if they are attached to an object, is destroyed.

I fixed it upon realizing the code I’d uploaded once reduced to not include the animation events within the code was not firing duplicates. Re-wrote the function from

function basicAttack()
	if Tool.Parent ~= Character then return end
	if canHit() == true then --canHit
		if Combo == 1 then
			Enabled = false
			Combo = 2
			anim2:Stop()
			anim1:Play()
			anim1:GetMarkerReachedSignal("hitStarts"):Connect(function()
				newHitbox:HitStart()
				print("hitstart")
			end)
			anim1:GetMarkerReachedSignal("hitEnds"):Connect(function()
				newHitbox:HitStop()
				Enabled = true
			end)
			anim1:GetMarkerReachedSignal("endCombo"):Connect(function()
				if Combo == 2 then Combo = 1 end
			end)
			return
		elseif Combo == 2 then
			Enabled = false
			Combo = 1
			anim1:Stop()
			anim2:Play()
			
			anim2:GetMarkerReachedSignal("hitStart"):Connect(function()
				newHitbox:HitStart()
				print("hitstart")
			end)
			anim2:GetMarkerReachedSignal("hitEnds"):Connect(function()
				newHitbox:HitStop()
				Enabled = true
			end)
			anim2:GetMarkerReachedSignal("endCombo"):Connect(function()
				if Combo == 1 then Combo = 1 end
			end)
			return
		end
	else 
		print("Attack Denied.")
		return
	end
end

to

function basicAttack()
	if Tool.Parent ~= Character then return end
	if canHit() == true then --canHit
		if Combo == 1 then
			Enabled = false
			Combo = 2
			anim2:Stop()
			anim1:Play()
			print("FIRE1")
		elseif Combo == 2 then
			Enabled = false
			Combo = 1
			anim1:Stop()
			anim2:Play()
			print("FIRE2")
		end
	else 
		print("Attack Denied.")
		return
	end
end

anim1:GetMarkerReachedSignal("hitStarts"):Connect(function()
	newHitbox:HitStart()
	print("hitstart")
end)
anim1:GetMarkerReachedSignal("hitEnds"):Connect(function()
	newHitbox:HitStop()
	Enabled = true
end)
anim1:GetMarkerReachedSignal("endCombo"):Connect(function()
	if Combo == 2 then Combo = 1 end
end)
anim2:GetMarkerReachedSignal("hitStart"):Connect(function()
	newHitbox:HitStart()
	print("hitstart")
end)
anim2:GetMarkerReachedSignal("hitEnds"):Connect(function()
	newHitbox:HitStop()
	Enabled = true
end)
anim2:GetMarkerReachedSignal("endCombo"):Connect(function()
	if Combo == 1 then Combo = 1 end
end)

and left everything else alone, no repeating outputs within the print window. I cant find out exactly what was causing it except for the listeners being inside of the event. output for revised script:

  00:10:00.105  FIRE1  -  Client - Main:98
  00:10:00.339  hitstart  -  Client - Main:145
  00:10:00.804  FIRE2  -  Client - Main:104
  00:10:01.105  hitstart  -  Client - Main:156
  00:10:01.520  FIRE1  -  Client - Main:98
  00:10:01.755  hitstart  -  Client - Main:145
  00:10:02.154  FIRE2  -  Client - Main:104
  00:10:02.454  hitstart  -  Client - Main:156
  00:10:02.803  FIRE1  -  Client - Main:98
  00:10:03.040  hitstart  -  Client - Main:145
  00:10:03.437  FIRE2  -  Client - Main:104
  00:10:03.722  hitstart  -  Client - Main:156
  00:10:04.086  FIRE1  -  Client - Main:98
  00:10:04.321  hitstart  -  Client - Main:145

Edit: apologies for the large code blocks, this was directly copy/pasted from my script without reducing for readability.
Edit2: I should note as well that some code/variables arent included because they were not modified, only the function itself was