Trinitycore: Core/Spells: Range validation is too strict on moving players

[chaodhib]

Description:
The range validation in TC has been reworked to mimic the range validation done in the client (using a decompiled code of wow client 6.2.3). This has been done in https://github.com/TrinityCore/TrinityCore/commit/e6a52d4aae9e8facc46c3b05e0560d540a928d73. Range validation on the server side is needed (since it prevents cheating) and this rework works greatly for the most part. On moving players however (either the target is a player or the caster is a player or both), it can be too strict and produces false negatives.

The issue was first discused here (and below): https://github.com/TrinityCore/TrinityCore/issues/17539#issuecomment-242581530

Current behaviour: When a spell is casted by a moving player OR on a moving player, close to max range, the spell cast might fail because of range validation failure.

Expected behaviour: If an unmodified client allows the use of a spell, the spell should not fail on the server side because of range validation.

Steps to reproduce the problem:
This can be reproduced by yourself using 2 wow instances and a server running in debug mode. On the server running on debug mode, put a break point on this line.

1. Have 2 players go to the Gurubashi arena (for example).
2. Have one player stand still.
3. Have the other player get in and out of max spell range, while spamming a spell on the other player. The easiest way to do that is to strafe left and right, getting in and out of the "in range" circular zone. cf:
   
4. The break-point will activate, indicating that the spell validation failed - even though, from the client point of view, the player was in range.

Branch(es): 3.3.5. probably master too.

TC rev. hash/commit: eca3645d8bb655b88e39f05e36b435171f738f70

TDB version: Not linked to DB.

Operating system: Any

The cause is:
Each client only send the player's position by discrete steps and the other clients use extrapolation between each discrete step. The server does not use extrapolation so it does not know the precise position of a player at all times.

A first try for a solution can be found here: https://github.com/TrinityCore/TrinityCore/pull/18128

[Krudor]

I think I've seen this behavior on live as well.

[Shauren]

Definitely seen this out of range case on retail
It MIGHT be the case that people playing on TC for so long got used to the broken state

[Raydor]

@chaodhib Any update to this issue? Are you working on something related to this? Any shitty, ugly or whatever hackfix for this in the meantime? As you might guess range in TC for pvp players is pretty fucked up (what we commented at https://github.com/TrinityCore/TrinityCore/issues/17539#issuecomment-242581530)

[chaodhib]

@Shauren is saying that this behavior also exists (or at least existed) on retail. If he is right, then there is no issue.

I'm not really doing any active development at this moment on this part. Nonetheless, I think implementing a simple extrapolation of the player movement (without taking into account collisions ; like i did, see above) would help alleviate the problem, without fixing it entirely. However, this (not perfect but not too complex) solution could be an acceptable solution.

[Killyana]

Couldn't be related to: https://github.com/TrinityCore/TrinityCore/issues/19271 ?

[chaodhib]

No. These are 2 different issues imo.

[Langerz82]

I will see what I can do with this.

[Langerz82]

Does this make it a bit better?
https://gist.github.com/Langerz82/d47cfba2a176ed4bd52f00d1c845c57c

Note - displacementWalking & displacementRunning values need tweaking or possibly take into account client latency.

[chaodhib]

Using the sniff parsing tool that I've wrote, I manage to find this packet exchange which proves that the current behavior is blizz-like:

Map: Warsong Gulch (489)

ServerToClient: MSG_MOVE_SET_FACING (0x00DA) Length: 36 ConnIdx: 0 Time: 05/15/2010 00:31:18.000 Number: 122344
Guid: Full: PLAYER_1 Type: Player Low: 00000 Name: PLAYER_1
Movement Flags: None (0)
Extra Movement Flags: None (0)
Time: 2573157110
Position: X: 1269.031 Y: 1478.594 Z: 313.6568
Orientation: 2.645158
Fall Time: 940

ClientToServer: MSG_MOVE_HEARTBEAT (0x00EE) Length: 37 ConnIdx: 0 Time: 05/15/2010 00:31:20.000 Number: 122655
Guid: Full: PLAYER_2 Type: Player Low: 00000 Name: PLAYER_2
Movement Flags: Forward, StrafeLeft (5)
Extra Movement Flags: None (0)
Time: 369305941
Position: X: 1255.925 Y: 1481.629 Z: 310.8066
Orientation: 5.471869
Fall Time: 1043

ClientToServer: CMSG_CAST_SPELL (0x012E) Length: 16 ConnIdx: 0 Time: 05/15/2010 00:31:20.000 Number: 122736
Cast Count: 47
Spell ID: Hammer of Justice (10308)
Cast Flags: None (0)
Target Flags: Unit (2)
Target GUID: Full: PLAYER_1 Type: Player Low: 00000 Name: PLAYER_1

ClientToServer: MSG_MOVE_HEARTBEAT (0x00EE) Length: 37 ConnIdx: 0 Time: 05/15/2010 00:31:21.000 Number: 122748
Guid: Full: PLAYER_2 Type: Player Low: 00000 Name: PLAYER_2
Movement Flags: Forward, StrafeLeft (5)
Extra Movement Flags: None (0)
Time: 369306441
Position: X: 1259.424 Y: 1481.538 Z: 311.7462
Orientation: 5.471869
Fall Time: 1043

ServerToClient: SMSG_CAST_FAILED (0x0130) Length: 6 ConnIdx: 0 Time: 05/15/2010 00:31:21.000 Number: 122765
Cast count: 47
Spell ID: Hammer of Justice (10308)
Reason: OutOfRange (97)

Context: One player (PLAYER_1 aka target) is standing still. Another player (PLAYER_2 aka caster, the player currently controlled by the client from which this sniff has been captured) is moving toward the target in order to cast a spell on him.

Notes:

• Speed of target (PLAYER_1): 0y/s (not moving)
• Speed of the caster (PLAYER_2): 7y/s
• Target stands on point A: X: 1269.031 Y: 1478.594 Z: 313.6568
• Caster moves from point B (X: 1255.925 Y: 1481.629 Z: 310.8066) to point C (X: 1259.424 Y: 1481.538 Z: 311.7462)
• distance between point B and point C: 3,624y in 3D. 3,500y in 2D.

Interpretation
From this sniff extract, we can see that, as soon as the caster is in range of spell (from the point of view of its client), somewhere between point B and point C, he tries to launch a spell against the target. But from the point of view of the server, the caster is still out of range. This can be explained by only 2 reasons:

1. the server hasn't received the next heartbeat yet (in the next hearbeat, on point C, the caster is indeed in range).
2. Nor does the server know that the player is actually in between point B and point C.
3. This packet exchange proves that a) even an unmodified client can encounter "our of range" cast result, even if, from its point of view, the player was in spell range. b) that the server does NOT extrapolate player positions for spell range checking.

This issue can therefore be closed. TC's behavior is blizz-like on this. For those that would like to be fixed nonetheless, the PR mentioned above can be a starting point.

[HelloKitty]

@chaodhib Very good experimental research. I had always assumed Blizzard predicted/extrapolated position. Obviously would have been expensive and technically difficult to do! Very surprising findings.

I do personally wonder why they didn't solve this in a similar way that they solved CMSG_AREATRIGGER positions. The 3.3.5 client at least sends an untimely MSG_MOVE_HEARTBEAT right before the CMSG_AREATRIGGER. Maybe they had a good reason not to implement this kind of client-help or extrapolation of position state.

[chaodhib]

Blizzard ¯ _(ツ) _/¯

Thanks for the info regarding CMSG_AREA_TRIGGER. I didn't know there are situations when MSG_MOVE_HEARTBEAT is not sent 500ms after the last move message.