Performance question

Help and Feedback Scripting Support
#1

so I wanted to ask that when we have something like buttons in the guide that I put that:

	for i, skin in pairs(script.Parent.main.Skin.ScrollingFrame:GetChildren()) do

the script will start waiting for:

skin.TextButton.MouseButton1Click:Connect(function()

so the script looks something like this:

script.Parent.Event.Event:Connect(function()
	for i, skin in pairs(script.Parent.main.Skin.ScrollingFrame:GetChildren()) do
		skin.TextButton.MouseButton1Click:Connect(function()
		print("Clicked")
		end)
	end
end)

every timeif the script receives a message from the event, the function starts again, the question is whether it is not bad for performance since I did not use Disconnect().
In the game, when a change is made to the buttons (for example, one is deleted or added), then an event is sent.
To make it easier to imagine, the player chooses a skin from a category, and if the player changes the category, other skins (buttons) are loaded.
Thanks for help

#2

Yes. Not only is it bad for performance, but having multiple events being fired for the same action can lead to unpredictability. Subtle bugs have the potential to appear because each button ends up running the same code multiple times.

You want to save a reference to the connections and then disconnect all them before reconnecting.

local connections = {}
script.Parent.Event.Event:Connect(function()
    -- disconnect previous connections
    for _,connection in connections do
          connection:Disconnect()
    end
    connections = {} 

    -- reconnect connections
	for i, skin in pairs(script.Parent.main.Skin.ScrollingFrame:GetChildren()) do
		table.insert(connections, skin.TextButton.MouseButton1Click:Connect(function()
		     print("Clicked")
		end))
	end
end)
4 Likes
#3

Ok, I needed to check this, I guess I need to study the disconnect function.
One more thing, if I use a similar loop that I run only once, doesn’t it cause a problem? example:

local tile = script:GetChildren()
for i = 1, #tile do
	tile[i].Touched:Connect(function(hit)
		if hit.Parent:FindFirstChild("Humanoid") then
			hit.Parent.Humanoid.Health = 0
		end
	end)
end```
#4

If it’s only running once, that’s fine. Although, if that Touched event is not needed after killing the Humanoid, it’s better to disconnect it. Usually, performance/design issues like these arise when you take a look at the scale of things rather than a few connections that don’t grow in size. But that isn’t to say you should just leave the connections running. If you don’t need that event anymore, get into the habit of disconnecting it.

If you ask yourself “Is the same event on the same object being connected more than once?” and say yes, you should disconnect the unnecessary connections. You really don’t need the same event running more than once on the same object.

1 Like