Update Session Dict Before invoking PlayerStatusChanged

[benev0]

Tested with SS14 master

Stops session existing in InternalSessions while PlayerStatusChanged is invoked

fix space-wizards/space-station-14#32926

[benev0]

• readyCount has delay after changes: original behavior was to alter on original player reconnect (assuming also incorrect)
• no clue if original behavior of the function is actuality correct
• PlayerStatusChanged has 56 refs in ss14 content repo; no clue what dire consequences may be unleashed regardless of if new behavior is correct

[benev0]

I am going to assume that the test failure is unrelated

[ElectroJr]

no clue what dire consequences may be unleashed regardless of if new behavior is correct

IMO it makes more sense for the session to still exist when the event gets raised, in case some event handlers end up calling other methods that need to retrieve the session via the userId. Its up to event handlers to check the status of sessions. I.e., its probably better to just fix this by having the lobby only count players with either SessionStatus.Connected or SessionStatus.InGame, instead of introducing breaking changes that might cause bugs elsewhere else.

Actually, given that the description of ISharedPlayerManager.PlayerCount is "Number of players currently connected to this server", either the description is inaccurate and need updating, or there is a bug and it should already be filtering by the session status. I guess it depends on what PlayerCount is mainly used for.

[benev0]

IMO it makes more sense for the session to still exist when the event gets raised ...

My head cannon is that the PlayerStatusChanged event should either have data passed or be fired after changes occur. Firing before with no data indicates that status will change for a player, but that data is not available, nor does it indicate which player will have this change; this behavior is also inconsistent with the PlayerStatusChanged fired on connect which waits for the player to be added to the data. Regardless, it seems that a full examination of downstream subscribers may be in order.