Fix issues with Torch God event#3279
Fix issues with Torch God event#3279lost-werewolf wants to merge 5 commits intoPryaxis:general-develfrom
Conversation
Greptile SummaryThis PR addresses two Torch God event regressions in TShock: hostile Key changes:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant C as Client
participant B as Bouncer.cs
participant STR as SendTileRectHandler.cs
participant S as Server/World
Note over C,S: Torch God Event Flow
C->>B: NewProjectile (TorchGod)
B->>B: Check type == TorchGod && !unlockedBiomeTorches
alt player has NOT unlocked biome torches
B->>S: Allow projectile (args.Handled = false)
S-->>C: Projectile visible to all clients ✓
else player HAS unlocked biome torches
B->>B: Main.projHostile[type] == true
B->>C: Reject + RemoveProjectile ✗
end
C->>STR: SendTileRect (torch tile)
STR->>STR: IsRectATorch(player, rect)
alt rect.Width==1 && rect.Height==1 && both tiles are TileID.Torches
STR->>STR: range = 104
else
STR->>STR: range = 32 (default)
end
STR->>STR: IsInRange(realX, realY, range)
alt within range
STR->>S: Apply tile change ✓
else out of range
STR-->>C: Reject tile rect ✗
end
Reviews (1): Last reviewed commit: "add missing torch check" | Re-trigger Greptile |
| if (type == ProjectileID.TorchGod && !args.Player.TPlayer.unlockedBiomeTorches) | ||
| { | ||
| TShock.Log.ConsoleDebug(GetString("Bouncer / OnNewProjectile super accepted from (torch god) {0}", args.Player.Name)); | ||
| args.Handled = false; | ||
| return; | ||
| } |
There was a problem hiding this comment.
Potential exploit: hostile projectile bypass for non-event players
The condition !args.Player.TPlayer.unlockedBiomeTorches allows any player who has not yet unlocked biome torches to send TorchGod projectiles, regardless of whether they are currently in the event. Because this check is placed before the Main.projHostile[type] gate, it effectively lets a malicious client bypass the hostile-projectile filter simply by never completing the torch unlocking mechanic.
A griefer on a server with multiple users who haven't unlocked biome torches could spam TorchGod projectile packets at will. Even if the projectile itself does not deal direct player damage, it can still trigger server-side effects (tile modifications, audio/visual spam to other clients, etc.) that were intentionally gated behind the hostile-projectile check.
Consider adding an additional server-side guard — for example, a cooldown, a per-player projectile count limit, or logging/rate-limiting suspicious creation spikes — to mitigate abuse until a more authoritative event-state sync is available from the client.
There was a problem hiding this comment.
Partially addressed in a95e88c.
It isn't possible to check if the player is in the torch god event, since the client doesn't send PlayerInfo when setting the flag to true, only when setting it to false.
| // Is the rect actually modifying a torch? | ||
| return rect[0, 0].Type == TileID.Torches && Main.tile[rect.X, rect.Y].type == TileID.Torches; | ||
| } |
There was a problem hiding this comment.
Extended range may not apply when relighting extinguished torches
The condition Main.tile[rect.X, rect.Y].type == TileID.Torches requires the current server-side tile to already be a torch. However, the PR description mentions that the player may need to "relight torches extinguished by the event."
If the Torch God event removes or replaces a torch tile (rather than only modifying its frame/paint), the server tile at that position would no longer have type TileID.Torches at the time the client sends the rect to relight it. In that case, IsRectATorch would return false, the range would stay at 32, and the relighting packet would be rejected — exactly the problem this PR aims to fix.
It's worth clarifying whether the vanilla Player.TorchAttack path only repaints/converts existing torch tiles (in which case the check is fine) or whether it can also replace/remove and later recreate them. If the latter, the server-side type guard should be relaxed or replaced with a check solely on rect[0, 0].Type.
There was a problem hiding this comment.
If the torch is removed before the client relights it, it's not a problem. Player.RelightTorches doesn't send a TileSquare on a tile that's either NOT a torch or is NOT an unlit torch, so this shouldn't occur. Lit/unlit torches are the same tile, just with different framing.
Fixes two issues relating to the Torch God event:
Player.TorchAttackchecks 100 blocks, but just in case.Unsure if the range check should be completely removed for torches. The player can move extremely far away and then they won't be able to relight the torches extinguished by the event, as they'll get rejected by the range check.
Due to a bug or oversight, the client doesn't sync their
happyFunTorchTimestate when it's set to true, only when it's set to false, so there's no way to know for sure if the player is in the event to apply proper range checking (and projectile creation). Feedback appreciated.