Instance is not nil after being destroyed

Help and Feedback Scripting Support
#1

I have many LocalScripts in StarterPlayerScripts and ScreenGui’s that have the ResetOnSpawn property unchecked (basically scripts that run from top to bottom when the player joins the game). Each script has a CharacterAdded event that refreshes the player’s character every time the player respawns, so that the code in the scripts can use the correct character instance.

local currentCharacter = player.Character or player.CharacterAdded:Wait()

player.CharacterAdded:Connect(function(character)
	currentCharacter:Destroy()
    print(currentCharacter) -- prints out instance name
    print(currentCharacter == nil) -- prints false
	currentCharacter = nil -- does this remove the reference from memory (allow for GC)?
	currentCharacter = character
	
end)

The problem is that, for some reason, currentCharacter is not nil. I’m very confused because isn’t the player’s character supposed to be destroyed after they die? I tried manually destroying it, but it’s not turning up as nil. I also tried setting currentCharacter to nil (which of course sets it to nil in the script), but I’m not sure if that removes it from reference.

Does currentCharacter = nil remove the previous character instance from memory, or do I need to do something else?

Any help would be appreciated!

#2

Instance:Destroy

"Sets the Instance.Parent property to nil, locks the Instance.Parent property, disconnects all connections, and calls Destroy on all children. This function is the correct way to dispose of objects that are no longer required. Disposing of unneeded objects is important, since unnecessary objects and connections in a place use up memory (this is called a memory leak) which can lead to serious performance issues over time.

Tip: After calling Destroy on an object, set any variables referencing the object (or its descendants) to nil. This prevents your code from accessing anything to do with the object.

Instance:Destroy does not set your reference to the instance to nil. The instance and its descendants were destroyed, but that does not eliminate from memory entirely. You will have to erase your references to destroyed instances manually in order for them to be completely garbage-collected

On another note,

local currentCharacter = player.Character or player.CharacterAdded:Wait()

The above code is flawed. There is a fatal edge case (see last two paragraphs) with this code snippet’s usage in the context of LocalScripts under StarterGui. The edge case will cause you to develop a reference to a destroyed instance. I have some additional thoughts on the snippet, which you can hear more about from a pin I made in the HiddenDevs Discord server:

Unfold to learn more...

Forget about using

local character = player.Character or player.CharacterAdded:Wait()

In every case where this is necessary it’s got a fatal edge case (see last two paragraphs), and in virtually every other situation, aborting continuation if the character doesn’t exist is best. My argument stems from, say, activating an ability mid-death, or teleporting characters to a map. It wouldn’t be practical to propagate the ability activation into the next life, or to interrupt all characters from being teleported when one isn’t available. But say you wanted to catch the outliers anyway, well, you could either spawn a new thread for every player to use the snippet:

task.spawn(function()
    local character = player.Character or player.CharacterAdded:Wait()

    teleport(character)
end)

Or be more efficient in your design:

local character = player.Character

if character then
    teleport(character)
else
    player.CharacterAdded:Once(teleport)
end

Not every situation is practically resolved using that snippet, and in many cases, a resolution isn’t really needed.

If your script is in StarterCharacterScripts, you can access the character as the script’s parent with guarantee; waiting for the character in this context would yield indefinitely. If your script is in StarterPlayerScripts (which it is), you cannot survive off that snippet as scripts under that container are only executed once, which means your reference to the character will eventually become outdated. If you need the character more than once, you follow a modified workflow:

local character = player.Character

if character then
    onCharacterAdded(character)
end

player.CharacterAdded:Connect(onCharacterAdded)
3 Likes
#3

So, for each script, I’ll have to set currentCharacter to nil every time the player respawns?

Also, for the fatal edge case, what I’m getting at is that line of code would lead to memory leaks every time the player respawns? I use that line of code in a lot of my localscripts.

I also read on the documentation that LocalScripts under StarterGui typically run before player.Character is changed to the new character (after respawning). So, from what I understand (I may have misread) is that I need to put all the code that accesses the player’s character inside an onCharacterAdded function?

I also use this a lot:

local character = player.Character

while not character do
   character = player.Character
   task.wait()
end

Is this code also flawed?

Anyways, thanks a lot for the help!

#4

“I need to put all the code that accesses the player’s character inside an onCharacterAdded function”

Not necessarily. The documentation shows what you should do:

local character = player.Character

if not (character or character.Parent) then
	character = player.CharacterAdded:Wait()
end

-- ...

The edge case can lead to a memory leak as it prevents the previous character model from being garbage-collected, but the fact that it’s destroyed will most definitely cause issues when code attempts to work with that character, ultimately causing an error.

Your “currentCharacter” tactic is what should be written as:

local function onCharacterAdded(character: Model & any)
    -- ...
end

local character = player.Character

if character then
    onCharacterAdded(character)
end

player.CharacterAdded:Connect(onCharacterAdded)
2 Likes
#5

currentCharacter simply refers to the player character. Even if you delete a character by Instance:Destroy() method, player.Character will still point to that character. However, if you set the character to nil, the character will be deleted and the player character will become empty.

#6

And also, delete player character in Client is not a good way. How about run code in Server?

#7

Yes, but right after I set currentCharacter to nil/ destroy it, it’s set to the new character. Does this lead to some sort of race condition?

Thanks for clearing things up for me. I really appreciate it!

#8

The character is referenced in the client script which means it’s taking up client memory. Right now, I have a pretty bad memory leak that happens every time the player respawns, and I was just wondering if this is a cause / contributor to it.

#9

No race condition. The lot of your code is just redundant. You need only one line:

currentCharacter = character

If Player.CharacterAdded is firing, the player’s previous character was already destroyed. There’s no need to set the previous reference to nil as overriding it will have the same effect. So long as no references to a destroyed instance is found, the garbage collector can do its thing

#10

I think there may be other reasons, but usually all physics calculations, such as player animations and player coordinates, are done on the client. All character-related coding is calculated on the client and accepted by the server. So, among the people who use hacks in the game, those who use things like flying or clip through walls are difficult to prevent. So I think deleting a character is also character-related and can take up enough memory to be calculated on the client.

#11

If you want to check if a destroyed instance is nil, then I suggest doing something like this:

if instance == nil or (instance and instance.Parent == nil) then
    print("instance is destroyed")
end

The destroyed instance is either nil, or its parent is nil.

It is that simple.

#12

You cannot know if a destroyed instance is nil. The instance itself will never be as it is eradicated from memory entirely. Your reference to that instance is what becomes nil, and that change in value is always caused by the scripter—you set it to nil, so you should know your reference is nil. Despite that, other scripts or even the engine could still have a reference to that instance. On a side-note, you can simplify your expression to:

if not (instance or instance.Parent) then

The only way to know if an instance is currently destroyed is to make an attempt to set its parent, as Instance:Destroy locks that property:

local function isDestroyed(instance: Instance): boolean
    return not pcall(function()
        instance.Parent = instance.Parent
    end)
end

But this isn’t practical in production code

#13

If you want an event to check if an instance is destroyed, then just do this:

instance.Destroying:Once(function()
    --do stuff
end)

The Destroying event fires prior to an instance being destroyed.
This is the simplest way, and therefore the best way because it is good practice to keep your systems and code simple.
Using :Once instead of :Connect ensures that there won’t be a connection in the memory after it has been fired, this saves memory and prevents memory-leaks from happening.

1 Like
#14

Yes. This is what’s done in production. This doesn’t help you know if an instance is currently destroyed, but it helps you run additional cleanup code when the instance gets destroyed. This is the only practical time to set up code for handling a destroyed instance.

On another note, though RBXScriptSignal:Once is ideal for this scenario, do know that event listeners bound to an event with RBXScriptSignal:Connect will be disconnected automatically if the instance the event belongs to is destroyed

1 Like