I have been using PlayerRemoving all this time, but found out only now that when it is being fired, it is being fired before Player was actually removed from Players.
It means that:
– I can’t use PlayerRemoving with any data-related scripts such as ProfileStore, ProfileService, just Roblox DataStore and any other (any scripts where you store some values for each player), because usually you store data of in-game players and when they leave you clear their profile. With doing it on PlayerRemoving you can still get a call later to get a profile of this player and get an error.
Should I use Players.ChildRemoved everywhere and does it mean that PlayerRemoving is just useless?
1 Like
PlayerRemoving, as the name implies with the -ing suffix and not -ed, is fired while they are BEING removed. It is not useless, its purpose is to let you do some cleanup in your game before the player’s Player object is gone.
3 Likes
Okey, I get it. But does it mean that I need to use ChildRemoved in 95% of cases? Like if I need to clear player data, I should do it on ChildRemoved. I just have never seen anyone doing it, only PlayerRemoving.
For me it’s a little bit strange that there is no signal PlayerRemoved then.
I’ve never had to use ChildRemoved like that. PlayerRemoving has always worked for me.
From what I can tell, it depends on what you want.
Do you want cleanup where you need the Player instance? Use PlayerRemoving, because that fires BEFORE they are deleted
Do you want something to happen when they are completely no longer in your game? Use ChildRemoved, I guess, but PlayerRemoving should still work for 99% of scenarios. And so, you should use PlayerRemoving unless you have a specific reason NOT to.
My take is that in fact PlayerRemoving shouldn’t work in 99% of scenarios and will lead to bugs.
I have a simple script that stores boolean “Active” (not in AFK-mode or any other conditions) and other data for each player. I have an intermission timer that every second checks that amount of active players is still enough to start the game. What I was doing is that I was removing player data table on PlayerRemoving and it was exactly what have lead to me getting an error, because data was already removed, intermission timer tried to get all active players, he was going through all players (let’s remember that Player left, but it is still being returned from Players:GetChildren()), then tried to index profiles[player].Active which was already cleared and got an error. And I believe in most of the cases you would like to have this behaviour, why would you want to get in situation where you removed some data but can be in trouble if you find this player through Players:GetChildren() or some other way?
I have a feeling the library you’re using might be doing some cleanup of its own, which is why PlayerRemoving is erroring like that for you.
I don’t use any library, I just have my own table.
1 Like
Okay, this is what made me think you were using a library.
I just understood that if my logic is right, I need to also rewrite part which stores data for players in-game such as active weapon, active pistol and so on – that’s why I added this (everything that is related to info that will be saved later in data store located in other place)
if you’re doing something like this:
Players.PlayerRemoving:Connect(function(player)
profiles[player] = nil
end)
then maybe later if you do something like this:
for _, player in Players:GetChildren() do
if profiles[player].Active then -- error
-- ...
end
end
it errors because the player’s data was deleted, but they’re still in Players?
I’m not too good with datastores, but I’ve never heard of someone having to use ChildRemoved like that. It might be a problem with your logic somewhere.
Yes, you pointed the problem right, but just forget about datastores for now. It’s just additional info, it’s not directly related to the question.
As I described before, I have a line:
activePlayers = PlayerStateService:GetLoadedPlayers()
Which does:
function PlayerStateService.GetLoadedPlayers(self: PlayerStateService): {Player}
local result = {} :: {Players}
for _, player in Players:GetChildren() do
if self:GetAttribute(player, "LoadedModules") then
table.insert(result, player)
end
end
return result
end
Which was calling:
function PlayerStateService.GetAttribute(self: PlayerStateService, player: Player, attribute: string): any
return self._playerStates[player][attribute]
end
But because of the fact that I was doing:
Players.PlayerRemoving:Connect(function(player: Player)
self._playerStates[player] = nil
end)
I was in situation when
- PlayerRemoving is fired, profile is cleared.
- Players:GetChildren() still returns player.
- Tries to get attribute, there is no profile, so indexing it with “attribute” fails.
i think i see the problem, maybe?
Replace that method with something like this:
function PlayerStateService.GetAttribute(self: PlayerStateService, player: Player, attribute: string): any
local state = self._playerStates[player]
if not state then
warn(`No state found for player {player.Name}`)
return nil
end
return state[attribute]
end
This checks to make sure the player’s state exists before indexing it.
edit: updated method, should be better now
1 Like
Yes, I understand that it works, but why I would want to do this checks if I can just switch from PlayerRemoving to ChildRemoved? I would not need to write some checks, do some warns and so on.
I honestly don’t like idea of if profile exists do something. I believe it’s always better to call things believing that your call is correct and if it’s not, you directly get an error – you did something wrong, you called it with invalid data / in invalid time. With your way you are just hiding a problem and making sure error is not appearing on the screen.
And I believe in most of the cases when you call PlayerRemoving, you are doing it in fact to clear some data and you get exactly in this type situation, not in situation where you want handler to be called RIGHT before Player was removed from Players.
Sure. That solution was with the mindset of using PlayerRemoved. If ChildRemoved works just fine for you, then use it. There’s no reason not to, it works for how you have it set up. People might not agree with you using it, but if you don’t think there will be any problems straying from the norm, and it works, then go for it. It’s not like you’ll suffer insane performance hits for changing how you detect a player leaving the game.
But if people don’t agree with me using it then what do they suggest? Use PlayerRemoving? Then you need to do checks, checks and checks again. We did a check to see if profile exist. There is no profile, we decided to return nil. Then the caller that wanted to get a profile has to check for nil and make his own logic for that. I believe you can make this sequence even longer if you really want to do it.
I just don’t know why would you want to do that. I am just trying to get idea behind everybody using PlayerRemoving.
What you want is waiting for the player to be parented to something else (e.g. nil), and that’s what ChildRemoved does. What PlayerRemoving does is fire when the player leaves the game but their Player instance is still a child of Players. It’s not bad at all to use it. Some people might be confused, but I really don’t see a problem here. If you don’t want to do all those checks, then ChildRemoved makes the most sense. If you ever have any problems with it, try PlayerRemoving with those checks. But for now, you’ve already said ChildRemoved works, haven’t you?
Yes, it’s totally fine, but I also wanted to not just find the fix for the problem, but also understand the reasoning behind the solution. Now, when I understood that I need to use ChildRemoved, I understood that I need to use in other places also and started to question why people even use PlayerRemoving at all. Maybe it’s just because it’s hard to get this error? Is it just a bad practice?
For example, my data that is related to data storage works this way: player joins, data from Roblox storage is extracted and put into some table because we want to extract it one time from storage and then just have it somewhere. It’s not really related to datastores, so don’t give too much attention to the fact that I have mentioned datastores. I was watching some really popular guide and there was explained that of course we need on PlayerRemoving remove this data of player.
But now I understand that I don’t know, let’s imagine, some function wanted to get player points or some other data. Again, we are still in the same situation. Profile would be already returned. It’s really easy to make this type of error not knowing why it happens.
I’ve already explained it. Different use cases. Sometimes you want the Player object before it’s deleted. Other times you don’t. Most of the time you want to just use PlayerRemoving, but your very specific scenario and desire to not use runtime checks leads you to using the other.
I have never had this error from PlayerRemoved.