--This leaks
while(true) do
for key,player in game.Players:GetPlayers() do
--Spawn!
player:LoadCharacter()
end
wait(0.2)
end
--This doesnt
while(true) do
for key,player in game.Players:GetPlayers() do
if (player.Character) then
player.Character:Destroy()
player.Character = nil
end
player:LoadCharacter()
end
wait(0.2)
end
Expected behavior
I would expect LoadCharacter() to correctly clean up the old character. It does not.
while(true) do
for key,player in game.Players:GetPlayers() do
if (player.Character) then
local humanoid = player.Character:FindFirstChild("Humanoid")
if (humanoid) then
humanoid.Health = 0
end
end
end
wait(0.2)
end
This leaks.
This is a big deal, because this is what happens when you just let characters get handled by roblox normally.
I don’t even know how to stop this case from leaking, none of my experiments worked.
local weakCharacters = setmetatable({}, { __mode = "v" }) -- Create a weak table
for _, player in pairs(game.Players:GetPlayers()) do
player.CharacterAdded:Connect(function()
-- Keep a weak reference to the old character since it's weak, shouldnt prevent the garbage collector from cleaning it up?
table.insert(weakCharacters, character)
end)
end
game.Players.PlayerAdded:Connect(function(player)
player.CharacterAdded:Connect(function()
table.insert(weakCharacters, character)
end)
end)
while true do
for _, player in pairs(game.Players:GetPlayers()) do
player:LoadCharacter()
end
wait(0.2)
end
It probably doesnt work, because I’m assuming that player:LoadCharacter() doesn’t hold onto other strong references of the old character internally. Worth a shot.
local Players = game:GetService("Players")
local function cleanupCharacter(player)
if player.Character then
-- Cleanup logic; we know it works
player.Character:Destroy()
player.Character = nil
end
end
local function overrideLoadCharacter(player)
-- Ensure we're not doing this more than once per player
if player:IsA('Player') and not player:FindFirstChild("LoadCharacterOverridden") then
local originalLoadCharacter = player.LoadCharacter
player.LoadCharacter = function(...)
cleanupCharacter(player)
return originalLoadCharacter(player, ...)
end
-- Add a tag
local tag = Instance.new("BoolValue")
tag.Name = "LoadCharacterOverridden"
tag.Parent = player
end
end
-- Immediately override for existing players
for _, player in pairs(Players:GetPlayers()) do
overrideLoadCharacter(player)
end
-- Setup to override for any players that join in the future
Players.PlayerAdded:Connect(overrideLoadCharacter)
Make sure no other script in your game is trying to call LoadCharacter before this script, no idea what determines the order in which scripts run in serverscriptservice, assuming it’s sorted by name prefix the script with $
Understandable Pointed out that it’s an outdated comment.
Good to know Roblox instances do not get garbage collected, but I heavily doubted from the start that weak tables would work, considering I don’t know the internal code haha
Bonus bug!
I think there might be something broken with the way player.Character:Destroy() works.
player.Character:Destroy()
player.Character = nil
does NOT behave the same way as just calling
player.Character:Destroy()
Full example:
CharacterAutoLoads = false
while(true) do
for key,player in game.Players:GetPlayers() do
if (player.Character) then
player.Character:Destroy()
--player.Character = nil --Comment this line out
end
player:LoadCharacter()
end
wait(0.2)
end
The following code causes the following console error:
I assume that’s the core movement scripts likely thinking that the character still exists, but it doesn’t. I don’t particularly think that’s a bug, as it’s only throwing a warning. There should be some sort of method though to destroy the character without that workaround.
It is weird that this is still happening after being reported and “fixed” many times. This is a major issue for games that make the player die a lot and do not use custom respawning systems
This isn’t an issue as long as scripts aren’t holding references to the character after it respawns.
local Character = Player.Character
Player:LoadCharacter()
-- ...Leak!
local Character = Player.Character
Player.CharacterRemoving:Connect(function()
Character = nil
end)
Player.CharacterAdded:Connect(function(newCharacter)
Character = newCharacter
end)
Player:LoadCharacter()
-- ...No leak!
Instance:Destroy() is a handy way to remove existing connections and references so that garbage collection can occur, but garbage collection still won’t occur if care isn’t taken to manually discard of old references held in scripts.
References to Instances are never weak, even in weak tables!
That’s what the bug report is about. Opening @MrChickenRocket 's repro file, this is the only script in the baseplate:
It does not hold any references to the old character; merely calling ::LoadCharacter.
Oh no! Sorry, I misunderstood the bug report. I thought the OP was reporting that Instance:Destroy() isn’t implicitly called on old characters. If there’s still a leak when there’s no scripts holding references to the old character, that’s a whole other kettle of fish - and it’s quite disappointing that some people mention this happening months ago yet we still don’t have a fix.
Pointed out that it’s an outdated comment.
