Should you disconnect character connections when they die?

With my research, I noticed that “Destroyed” doesn’t fire when characters die and then respawn. Would this mean I should call a kind of disconnect function? For example:

local Players = game:GetService("Players")
local TempCon = nil

Players.PlayerAdded:Connect(function(Plr)
	Plr.CharacterAdded:Connect(function(Char)
		TempCon = Char.ChildAdded:Connect(function() --Create any kind of event
			
		end)
	end)
end)

task.wait(5) --Player leaves before this continues
print(TempCon.Connected) --Outputs true, which I think indicates that this connection is active.

Surely this would lead to some kind of memory leak but I could be completely wrong, in one of my recent games, there is a huge memory leak that slowly builds over time and I am looking at potential solutions.

When an Event that is connected to an Object that was recently destroyed, it would be removed, but the Variable Holding that Event is keeping it from being Garbage Collected. (Not entirely sure if thats the case however, would recommend looking into it however)

1 Like

I see, I will continue to do more research on this, thanks!

So you are aware for certain, this will cause a memory leak since for what ever reason when the character is removed it isn’t destroyed. There are a couple bug reports and misc support threads on this but none of the bug reports have been addressed I don’t think.

You can test this yourself by checking connection.Connected (a connection won’t be GC’d if its Connected property is true because it is still considered alive)

local character = game:GetService('Players').LocalPlayer.Character
local connections = {}

for _, child in character:GetDescendants() do 
    table.insert(connections, child.Changed:Connect(function() end)) 
end 

game:GetService('Players').LocalPlayer.CharacterRemoving:Connect(function(character) 
    task.delay(2, function() 
        local connected = 0 
        for _, connection in connections do
            if connection.Connected then 
                connected += 1 
            end 
        end
        print(connected) --> 162 for me, which means 162 connections exist in memory and won't be garbage collected
    end)
end)

Versus on a properly destroyed instance,

local part = Instance.new('Part') 
local c = part.Changed:Connect(function() end) 

c:Disconnect() 

print(c.Connected) --> false

Also note that players aren’t destroyed when they leave either, so it’s advisable to disconnect listeners connected to the player’s events as well.

If you want a surefire solution, hook into .CharacterRemoving, set the player’s Character property to nil (else you’ll get a bunch of warnings saying something like “Player:Move was called but Player has no humanoid”), and destroy the character there. The current active character is passed to the function passed to it so you can use this to retain a ref to the character but the details are up to you to decide.

Other topics on issue

Old Characters Aren't Destroyed Characters not destroyed when respawned
Destroying Characters

4 Likes

I think cody is right. This issue has remained dormant for quite some time and to date hasn’t been properly addressed. Since garbage collection is an internal process, we can’t solve the root of this behavior. The same applies both to connections bound to player and to character.

As Cairo also said, events do in fact get disconnected once the instance they are linked to is destroyed and all reference is lost or weak at least (in case of weak tables).

To add to the cody’s example, garbage collection may be illustrated with the following:

local tab = setmetatable({}, {__mode = "v"})
tab[1] = Instance.new("Part") -- default and not parented

repeat task.wait() until next(tab) == nil
print("Userdata got garbage collected.")

Why didn’t this work?

Because garbage collector runs in cycles and when there’s actual memory to be cleared. One hacky way is to get it busy.

local tab = setmetatable({}, {__mode = "v"})

tab[1] = Instance.new("Part") -- default and not parented

-- Pollute the place a little.
game:GetService("RunService").Stepped:Connect(function()
	local part = Instance.new("Part")
	part.Parent = workspace
	task.wait(1)
	part:Destroy()
end)

repeat task.wait() until next(tab) == nil
print("Userdata got garbage collected.")

The rest is handled internally as well. The userdata is gone and with it the part for us too. It was created with lua_newuserdata() and provided access to C++ code. Somewhere in the very near future the engine should recognize the state of the lost part and free up the memory.

I don’t know why described behaviour is not typical for characters. In some past reports I’ve read about character and player instances being finally destroyed minutes after becoming redundant, though I haven’t seen that myself.

All in all,
as cody said, I also personally destroy the characters upon respawning. Just to be concern free, even if we are not talking about a memory leak, the same logic applies, whether we rely on destroying, using maids/janitors and so on.

2 Likes

Fantastic insight, a lot of things learnt here.

So what you mean by destroying the character upon respawning, is by having a variable outside the scope of CharacterAdded that is assigned to the character argument that is passed to said event, and of course once it’s destroyed, you just reassign the new character argument to the variable.

But I’m still a little concerned about this as this means there is a variable that appears to me that won’t get garbage collected since it’s out of the CharacterAdded event. If it isn’t much of a hassle, could you perhaps show me a small snippet of code so I know that I’m doing this safely?

2 Likes

Thanks! It’s @7z99 solution though, I just tried adding to it. I’m also more of a hobbyist, so while I do research and say what I’m fairly certain happens, I sure can make mistakes.


Guess it depends on your framework and how you handle characters. One example way is definitely what cody mentioned:

local PlayerService = game:GetService("Players")

local tempConn: RBXScriptConnection

PlayerService.PlayerAdded:Connect(function(player)
	player.CharacterAdded:Connect(function(character)
		-- Running once for testing purposes, otherwise our print() would output
		-- 'true' because new connection would be assigned.
		if not tempConn then
			tempConn = character.ChildAdded:Connect(function(child) end)
		end
	end)

	player.CharacterRemoving:Connect(function(character)
		character:Destroy()
		task.wait(1) -- Checking immidiately might return 'true'.
		print(tempConn.Connected) --> false
	end)
end)

I usually have a character class that looks somewhat like this:

local Character = {}
Character.__index = Character

local Maid = require(...)

function Character.new(player: Player)
	local self = {}
	
	self.model = player.Character or player.CharacterAdded:Wait()
	-- other vars, states ...
	self._maid = Maid.new()
	self._maid.SampleConnection = something.Changed:Connect(function() end)
	
	return setmetatable(self, Character)
end

function Character:Discard()
	self._maid:DoCleaning()
	self.model:Destroy()
	self.model = nil
end

return Character

:Discard() should ask maid to clean connected events. Now that I’m thinking, we could alternatively send the maid the whole player.Character.

From this point, let’s say we have a Client module that provides access to everything regarding every player, from data to character movement.

local Client = {}

local Character = require(...)

local players: {[Player]: any} = {}

function Client.new(player: Player)
	players[player] = {}
	players[player].loaded = false
	
	-- Retrieving and handling data, maybe in a spearate thread.

	players[player].Character = Character.new(player)

	player.CharacterAdded:Connect(function(character)
		if players[player].Character then
			players[player].Character:Discard()
		end
		players[player].Character = Character.new(player)
	end)

	players[player].loaded = true
end

function Client.leaving(player: Player)
	-- Save the data.
	
	players[player].Character:Discard()
	players[player] = nil
end

setmetatable(Client, {
	__metatable = Client,
	__index = players, -- So that empty index leads to 'players'.
	__newindex = function(self, k, v)
		warn("Client: module is read-only.")
	end,
})

return Client

When a new player joins we call Client.new(), init a new character and connect CharacterAdded, which does the same, but beforehand clears the old character. When player is leaving the game, we call Client.leaving(), which also deletes the character, and finally, removes the reference.


Depends on your specific case. But the basic principle remains: when a player respawns, disconnect and destroy the old character and lose the reference (nil and/or working inside a scope - functions, do-blocks etc. and/or overwriting the variable with new character and so forth).

I hope this helps!

As for player instance, I’m frankly not sure. I haven’t tried to manually remove a player in my projects before, nor did I ever notice any conspicuous issue with that, probably because players come and go much less often then characters are respawned.

player:Destroy()? I’d leave this to the engine for now, to be on the safe side, especially while we can disconnect events manually or use Maid/Janitor/Trove/… Correction: There has been a very recent update.

Good luck!

(Edit: spelling and link.)

2 Likes

This is fantastic, thanks for your help! I will continue to do my research on this to really make sure everything is covered.

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.