Fixed Server not tracks deaths, Now players can be viewed with the /death, /pvpdeath, /alldeath and /allpvpdeath commands.

[hufang360]

Fixed Server not tracks deaths, Now players can be viewed with the /death, /pvpdeath, /alldeath and /allpvpdeath commands.

[hakusaro]

Thank you so much for your contributions! This looks good to me, but another person needs to review this besides me before merging!

[drunderscore]

It seems like logging into a brand-new user (never before logged into) will allow the player to retain the death counter they previously had (be it from our SSC of another user after /logout, or the character they are joining with), instead of defaulting to 0.

Reproduction steps:

• Execute /user add bruh bruh default in console
• Join server
• Execute /login bruh bruh
• Observe that the death counts from the joining character are preserved.

Otherwise, once the user has existed, it seems it counts properly, and logging in and out to different users properly updates the count.

[drunderscore]

This looks suspicious... why are only death counts that are greater than 0 allowed to override the player's data?

[hufang360]

It seems like logging into a brand-new user (never before logged into) will allow the player to retain the death counter they previously had (be it from our SSC of another user after /logout, or the character they are joining with), instead of defaulting to 0.

Reproduction steps:

• Execute /user add bruh bruh default in console
• Join server
• Execute /login bruh bruh
• Observe that the death counts from the joining character are preserved.

Otherwise, once the user has existed, it seems it counts properly, and logging in and out to different users properly updates the count.

Thank you so much. I'll fixed it.

[hufang360]

This looks suspicious... why are only death counts that are greater than 0 allowed to override the player's data?

Because, the this.numberOfDeathsPVE variable may be null, When "HandleSpawn" event can read the value is 9985.
It isn't always good, i need make change.

Refer: #2758 (comment)

[hufang360]

I change this, It seems work. Can you review it agrain. @drunderscore

[hakusaro]

@hufang360 I'm happy with this but it probably won't make 5.1, likely 5.2. This is because of the newest terraria release. Sorry.

[drunderscore]

The issue of newly-created users inheriting the previous death count is fixed now, but something is still wrong unfortunately...

Dying will properly increment the death counter (executing /deaths whilst dead shows the correct number), but once the player respawns, their local death count will override the server's.

[drunderscore]

Dangerous change here -- when SpawningIntoWorld with non-matching death counts (which is likely), we will no longer call the OnPlayerSpawn handler, which is very much unexpected and will cause issue with plugins.

This entire check should be moved below the OnPlayerSpawn handler call.

[hakusaro]

@drunderscore can you re-review this one?

[drunderscore]

Something still isn't correct -- the same issue as above persists, where in death the counter holds the expected value, but once the player respawns, their local player's death count is respected. I've attached a video to show exactly what I'm talking about.

• Show that before login that local player has x deaths
• Login to new user
• Show new user has 0 deaths
• Die
• Before respawn, show user has 1 death
• After respawn, show user does not still have 1 death as expected.

Screencast.from.11-28-2022.09.13.16.AM.webm

[sgkoishi]

From the video, it seems that the player starts with 12544 times and ended up with 12545, which probably indicates that the client is maintaining their local count and send to the server and overwrite it. Does the server side 0-1 count ever reach the client?

[sgkoishi]

Something still isn't correct -- the same issue as above persists, where in death the counter holds the expected value, but once the player respawns, their local player's death count is respected. I've attached a video to show exactly what I'm talking about.

* Show that before login that local player has x deaths

* Login to new user

* Show new user has 0 deaths

* Die

* Before respawn, show user has 1 death

* After respawn, show user does not still have 1 death as expected.

The player initiates the Spawn process, and sends the local count to the server; to overwrite we need to let the server send Spawn again (double spawn!). We could initiate an extra server Spawn once per (after) login, or once after death. This might look awkward but probably the only way 🤔