Well, my client code uses both cases where I have to explicitly disconnect events, but I do that anyways for everything unless it’s meant to be persistent on a persistent object. Perhaps that’s why I haven’t had the issue too much.
This is an amazing feature but wouldn’t this break a lot of old games that no longer get updates?
I’ve got a feeling that this will break something.
But i am not sure what.
Cool update though.
Hi
From what I’m seeing, you should be able to get a clone of the character in the CharacterRemoving event still.
Just make sure you’re not yielding in any way.
What if I want to detect if the player is leaving via BindToClose? Before, developers could simply add a task.wait(0.1) and check for a state that would be applied in your BindToClose (eg. IS_SHUTTING_DOWN == true), but now I cannot do task.wait within a PlayerRemoving function. How would we know when the player is leaving through a shutdown?
Well, they were still freed if you do a manual cleanup of connections (or call Destroy yourself) and other references (which is always a good idea to do).
But it was a common source of server memory going up with many bug reports being sent to us.
Yes, because Player:LoadCharacter
causes CharacterRemoving
to trigger, it also causes Destroy
to be called automatically on the old one.
Player death doesn’t remove the character instantly, that only happens on respawn.
Leaving players already unparent character from the world and like you said, there is a chance to clone in CharacterRemoving
.
We do see that not being able to reparent removed characters back might be the biggest problem with this change. We would like to hear more about the problematic cases so we can adjust the new behavior if possible.
Yes. We have updated the wording in the original post to be more clear on that - any events that cause CharacterRemoving
to be called will be followed by a Destroy
(on the server).
There is a risk of that, as evidenced in some replies here.
So far we didn’t get reports of game breaking bugs, but we will still evaluate what adjustments might be needed to reduce the impact of this change.
There should be some way to see if the Player is removing due to BindToClose, since BindToClose currently runs after all the Players are removed in Roblox
Most developers would set a Shutdown check variable to true in BindToClose, and in PlayerRemoving would check something like task.wait(0.1) and IS_SHUTDOWN
. But since this isn’t allowed anymore in the new update, there is pretty much no way to tell if the player is leaving due to a shutdown.
Due to this, games which have combat-logging can no longer tell whether the reason of combat-logging is shutdown or actual logging.
What is the event order of characters despawning after this update?
I would assume its like this:
- Player.Character gets set to nil
- CharacterRemoving fires
- Character gets destroyed
This is important because there is no function to manually despawn a character, so we have to make one ourselves. (Or can we just set Player.Character to nil and everything else is done automatically?)
This is a very nasty issue. I have for sure created many many memory leaks this way. What is the reason behind the player and character not being destroyed in the first place?
Also are there other instances that don’t get destroyed?
The fact that nothing was destroyed “in the first place”! Destroying an Instance was only introduced as a concept many years after Roblox was first created.
Things have gradually been migrated to destruction piece by piece over the years since then. It probably would have been better to buckle down and migrate everything all at once back then but hindsight is 20/20.
Back when I was starting out, I believed that this was always the case, and didn’t disconnect anything connected to players thinking it will get disconnected when they leave anyway, and then I published my first major game and boy the headaches and the stress and frustration searching for this memory leak, until some1 finally told me about player instances not being destroyed
so that others don’t have to go through this in the future, i’m really glad we got this update!
None of your examples throw any errors with the new behavior enabled.
Your BadgeService
call is already broken today, badges cannot be awarded to players that are not in the game.
Please, we ask you to test your code and experiences with new behavior enabled and report real issues, no need to speculate.
Hi, thanks for sharing these code snippets. First though, I want to clear up a few things about what ‘Destroying’ means and does not mean.
Destroying an Instance, exclusively does the following:
- Sets Instance Parent to nil
- Disconnects all Connections to the Instance’s Events
- Recursively destroys all children
Destroying an instance does not do the following:
- Make the instance ‘vanish’ into the garbage collector while it still has active references
- Throws an error when accessing a reference to the Instance
- Remove Instance properties or make Instance properties throw an error when accessed
Looking at your code samples:
someTable[plr] = true -- throws error
This would not error, the player Instance reference is still active. someTable[plr] would be nil because you set it to nil in the PlayerRemoving
callback.
local character = plr.Character -- throws error
This would not throw an error, it would return a reference to the player’s last Character model (which has been destroyed).
RemoteEvent:FireClient(plr, "DataLoaded") -- throws error
This would error, but this is expected behavior and not changed by the new player / character destroy behavior. The player is no longer in the experience so it is conceptually impossible to send a networked message to their client.
BadgeService:AwardBadge(plr.UserId, badgeId) -- throws error
Once again, this would error, but also is unrelated to the new behavior. You cannot award a badge to a player that is not in rthe experience. Accessing the UserId itself would not error (it would give you the user Id)
someTable[plr] = nil -- throws (but used to be necessary)
This would not error as the player Instance reference is still valid.
Thank you for for the kind response. I apologize for not testing the code before posting here, as it is clear now that I was incorrect, and especially because half of my “examples” would error for a perfectly logical and completely unrelated reason . Thank you as well for clarifying the ‘Destroying’ behavior, as I had previously misunderstood this to force garbage collection of an instance, which was the root cause of all my misconceptions. I feel blind for having not realized this before…
This is quite an interesting thing to see! Makes me wonder what else in my experiences may be causing potential memory leaks…
I just recoded my ragdoll body system from using the same thing, to recloning to make it compatible with this change. I don’t mind, but it would probably be better to have a way to keep the model from being destroyed. And keep the model from parenting to nil when removed from the player as it’s character as well.
how do i enable this
30CHARBYPASS
In the Properties of the Workspace.
Enabling this solved a huge memory leak we had on the server relating to animations and signals.