New Player and Character Destroy Behavior

Updates Announcements
#39

About time this was addressed. This is something people have been complaining about for awhile now. I’ve had to resort to manually calling destroy on the player’s character when they died, and when they left the server to make sure it was cleaned up properly.

3 Likes
#40

That’s actually a good point. Probably introduce a time setting to where after this many seconds, the character is destroyed. Probably two separate settings:

  1. Player died and respawn, must be less than the respawn time.
  2. When the player leaves the server.
5 Likes
#41

Will WaitForChild work after a PlayerRemoving function? Or does this count as a deferred operation?

Players.PlayerRemoving:Connect(function(Player)
	datastore:SetAsync(Player.Name, Player:WaitForChild("DataFolder").Level.Value)
end) --- does wait for child count as deferred operation?

Players.PlayerRemoving:Connect(function(Player)
	datastore:SetAsync(Player.Name, Player.DataFolder.Level.Value)
end) --- or is this preferred without wait for child?
1 Like
#42

WaitForChild will return immediately if the child exists, though it would be ill-advised to use it in that context: The object is being destroyed immediately thereafter so there’s nothing to wait for, you would just want to use FindFirstChild in that context.

3 Likes
#43

I like this idea, because I tried enabling the setting, and it broke a lot of my player-removing functions, as the player was destroyed too early. I’d love if I could add a buffer time, it would prevent the breakage of these functions.

1 Like
#44

Hi!

Once you mentioned this, I looked a bit deeper into the replication problem and did some tests. The character on client’s side won’t be completely removed automatically.

With the last year’s Destroy() news (Destroy() Can Now Replicate to Clients), one would expect local character to be cleaned up too, but the automatic removal apparently differs from manual.

The behaviour puzzled me for a while until I saw this at the very beginning of the announcement haha:

Which means,

  1. unaltered automatic behavior will not destroy the character locally;

  2. setting player.Character to nil will not destroy the character locally;

  3. calling player:LoadCharacter() will not destroy the character locally;

  4. calling player.Character:Destroy() will destroy the character locally (but don’t do this*);

  5. calling player.Character:Destroy() after player.Character has been set to nil will not destroy the character locally.

*Destroying immediately causes a bunch of “Player:Move called, but player currently has no humanoid. (x953)” warnings.

I guess we’ll be using a good old remote event for local removal (along with option 5).

3 Likes
#45

Just wanted to chip in and second this, I recently worked on a house system with “local” interiors which involved parenting other characters to nil to make them “invisible”.

I soon realized that the characters were never getting destroyed even when I do so from the server. I ended up using a remote to tell everyone that the character should be destroyed locally after the fact. A little weird, but I didn’t think too much about it since PSA: Connections Can Memory Leak Instances was already reported.

1 Like
#46

It would be situational and specific to the game, but would it really matter on the client? It’s the server that has the problem.

1 Like
#47

Will an existing character Model be destroyed when a player leaves? I’m asking because that case isn’t covered by either of these two behaviors.

1 Like
#48

Performance impact is somewhat difficult to gauge considering memory usage accumulates over time.

I imagine the server is a lot more powerful than players’ computers, and the leak could slowly impair performance on older/low-end devices.

The leak could slowly impair performance on older/low-end devices. For comparison, in terms of available memory, each server is allocated roughly 7 Gb of RAM (although it crashes after exceeding 6 Gb).

Destroying old characters is fairly straightforward and doesn’t have to happen immediately.

And disconnecting player’s own character is probably a lot more crucial if a local script doesn’t get refreshed with each respawn (outside the character, backpack …).

1 Like
#49

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.

2 Likes
#50

This is an amazing feature but wouldn’t this break a lot of old games that no longer get updates?

1 Like
#51

I’ve got a feeling that this will break something.
But i am not sure what.

Cool update though.

2 Likes
#52

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.

1 Like
#53

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?

#54

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.

2 Likes
#55

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.

1 Like
#56

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
image

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.

4 Likes
#57

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?)

#58

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?

1 Like