Fix pvp mode bugs

[lost-werewolf]

This is a redo of #3237 without any conventional commits this time.

• fix players bypassing 'pvpwithnoteam' from spawn packet
• fix team fastswitch from player spawn packet
• fix initial team change message not being broadcast on player spawning in
• check player team and pvp status in OnSecondUpdate to enforce current pvp mode

[greptile-apps]

Greptile Summary

This PR fixes several PvP-mode bypass vectors by taking full control of the non-SSC spawn packet path and wiring in an InitialTeamChangePending guard for the first post-spawn team-change packet, then adding a per-second enforcement sweep in OnSecondUpdate as a backstop.

Key changes:

• HandleSpawn (GetDataHandlers.cs): The non-SSC path now always returns true (TShock handles the packet entirely instead of delegating to the Terraria engine). Team zeroing for pvpwithnoteam and fast-switch detection are applied before FinishedHandshake is set; the connection-completion logic previously handled by Terraria's MessageBuffer is replicated manually here, including hard-coded packet IDs 129 and 139.
• HandlePlayerTeam (GetDataHandlers.cs): The InitialTeamChangePending flag bypasses the same-team early-return and the 5-second cooldown for the very first team-change packet after spawning, allowing the initial join broadcast to propagate. Unsets the flag and resets LastPvPTeamChange to DateTime.MinValue so subsequent changes are not incorrectly throttled.
• OnSecondUpdate (TShock.cs): Clears a stale InitialTeamChangePending flag after 5 seconds and actively corrects a player's hostile flag and team every tick if they diverge from the configured PvP mode. PvPMode.ToLowerInvariant() is recomputed on every iteration of the per-player loop, creating one allocation per player per second.
• TSPlayer.cs: Adds the InitialTeamChangePending boolean field; minor doc-comment triple-slash fix.

Confidence Score: 4/5

• This PR is safe to merge with minor style issues; core logic is well-reasoned and the bypass vectors it targets are correctly closed.
• The overall approach is sound: the InitialTeamChangePending flag correctly models the "one free initial team-change" window, pvpwithnoteam enforcement is applied at spawn, in the team-change handler, and in the periodic sweep. No regressions to SSC spawning were introduced. The three style-level concerns (no-op dead assignment, magic packet IDs, per-player string allocation) do not affect correctness.
• Pay attention to GetDataHandlers.cs — the manual replication of Terraria's MessageBuffer connection-completion block (magic numbers 129/139) should be verified against the current Terraria source to ensure it remains in sync.

Important Files Changed

Filename	Overview
TShockAPI/GetDataHandlers.cs	Core of the fix: HandleSpawn now fully takes over the non-SSC spawn path (returning true instead of false), enforcing pvpwithnoteam team zeroing and rate-limit checks at spawn time; HandlePlayerTeam gains InitialTeamChangePending awareness to allow the first post-spawn team-change packet through without triggering the 5-second cooldown. Logic is sound but includes a no-op TPlayer.dead assignment and hard-coded packet numbers 129/139.
TShockAPI/TShock.cs	Adds per-player PvP-mode enforcement in OnSecondUpdate: corrects hostile flag for disabled/always/pvpwithnoteam modes and resets team to 0 for pvpwithnoteam. Also expires InitialTeamChangePending after 5 seconds. Minor concern: PvPMode.ToLowerInvariant() is allocated once per player per second inside the loop.
TShockAPI/TSPlayer.cs	Adds the InitialTeamChangePending boolean field with a clear XML doc comment; also fixes a stray triple-slash in a doc comment. Straightforward and clean.

Sequence Diagram

sequenceDiagram
    participant C as Client
    participant HS as HandleSpawn
    participant HT as HandlePlayerTeam
    participant OSU as OnSecondUpdate

    C->>HS: PlayerSpawn (team=N, context=SpawningIntoWorld)
    Note over HS: pvpwithnoteam? → force team=0, teamCorrectNeeded=true
    Note over HS: Fast-switch? (FinishedHandshake && <5s) → teamCorrectNeeded=true
    HS->>HS: FinishedHandshake=true<br/>InitialTeamChangePending=true (non-SSC, team≠0, !teamCorrectNeeded)<br/>LastPvPTeamChange=UtcNow
    HS->>C: PlayerSpawn broadcast (team corrected if needed)
    HS-->>C: PlayerTeam correction (if teamCorrectNeeded)

    C->>HT: PlayerTeam (team=N)
    Note over HT: InitialTeamChangePending=true → skip early-return
    alt pvpwithnoteam
        HT-->>C: PlayerTeam correction (reject)
    else InitialTeamChangePending
        HT->>HT: InitialTeamChangePending=false<br/>LastPvPTeamChange=DateTime.MinValue
        HT->>C: Broadcast team change (return false → engine handles)
    else rate limit (<5s)
        HT-->>C: PlayerTeam correction (reject)
    else allowed
        HT->>HT: LastPvPTeamChange=UtcNow
        HT->>C: Broadcast team change
    end

    loop Every second
        OSU->>OSU: Expire InitialTeamChangePending if ≥5s elapsed
        OSU->>OSU: Enforce pvp hostile flag (disabled/always/pvpwithnoteam)
        OSU->>OSU: Enforce team=0 for pvpwithnoteam
        OSU-->>C: TogglePvp / PlayerTeam corrections if needed
    end

Loading

_{Last reviewed commit: d31bde4}

[greptile-apps]

ToLowerInvariant() called per player per tick

Config.Settings.PvPMode.ToLowerInvariant() allocates a new string on every call. Since this runs inside the OnSecondUpdate loop (once per connected player, every second), it creates unnecessary allocations proportional to the player count. Consider computing it once before the loop, or caching a lowercased copy of the setting when it changes.

Suggested change

	string pvpMode = Config.Settings.PvPMode.ToLowerInvariant();
	string pvpMode = Config.Settings.PvPMode.ToLowerInvariant();

Move this line outside the per-player foreach loop to compute it only once per tick.

[greptile-apps]

TPlayer.dead assignment overwritten by Spawn()

Terraria.Player.Spawn() resets this.dead = false internally, so the assignment on line 3068 is immediately overwritten by the Spawn(context) call on line 3071. The authoritative death-state for TShock is tracked via TSPlayer.Dead (already set earlier at line 2986 for the non-SSC path), so this doesn't break anything — but the assignment here is a no-op that may confuse future readers.

Consider either removing the dead = true assignment, or placing it after the Spawn() call if the intent is to override Terraria's reset:

Suggested change

	if (respawnTimer > 0)
	args.Player.TPlayer.dead = true;
	args.Player.TPlayer.Spawn(context);
	args.Player.TPlayer.Spawn(context);
	if (respawnTimer > 0)
	args.Player.TPlayer.dead = true;

[greptile-apps]

Magic packet-type numbers

The raw numeric literals 139 and 129 are used in calls to NetMessage.TrySendData. These are opaque to anyone not familiar with the Terraria protocol and will silently break if Terraria ever renumbers its packet types. The rest of TShock casts PacketTypes enum members (e.g. (int)PacketTypes.PlayerSpawn) instead of hard-coding integers.

If these packet types are missing from the PacketTypes enum, consider adding them; otherwise, reference the existing named constants to keep the code consistent and self-documenting.