Back2Basics - blizzlike+ WoW 2.4.3 / TBC Server

Zitat

1. Extrahit Delay
2. Melee Hitbox

• Meleehitrange (wer hätte es gewusst..?)
  
• Extra-hit-delay (überschreibung durch unterschiedliche extra-hits)

Zitat

Hi, the person in question is me.

- Vayrie
Hey, I fixed this issue before I looked at your code, and we have a fairly similar fix. The main thing I'd like to point out (to anyone interested in why this movement lag came about) is that the real problem lies not really in the timers, but rather the interval that is distributed to the client (from what I could gather).

What was actually broken (as far as I could tell, feel free to correct me if I'm wrong) is that Mangos sent the current server time rather than the client-provided timestamp with the movement forwards to the other client, in other words:

The packet the client sends on a movement update contains the updated state as well as a millisecond-precise timestamp to reflect when the change happened. That means if the first package looks like this (note that this is not actually how WoW packets look):

	Quellcode
1 2 3 4 5	MOVE_FORWARD opcode Timestamp: 3600000 and the second packet looked like: MOVE_LEFT opcode Timestamp: 3600500

All that's needed is the difference between packet 1 and 2 to make the movement look proper (in other words if we sent a packet with timestamp 0 and then timestamp 500, it'd look the same, because the WoW client, from what I could gather, uses the delta time and not the actual timestamp).

That value was already being forwarded as part of the packet updates in mangos, so all should really have been well, if it wasn't for one little line someoned had added. This line is located in the function HandleMoverRelocation(), which is called by the HandleMovementOpcodes() function (thusly called on each update). This function has the following code

	Quellcode
1 2 3 4	void WorldSession::HandleMoverRelocation(MovementInfo& movementInfo) { movementInfo.UpdateTime(WorldTimer::getMSTime()); // Line that caused all the damage // ...

The following line will overwrite the client sent value and instead add the millisecond time that has passed since the start of the server. Since it does this for all packages we're effectively rebasing it on server time; the main complication is that we're using the server's timestamp rather than the timestamp distributed along with the movement packet. This means that if packets do not arrive in the exact time difference that the movement timestamp in them implies, we'll get laggy movement (aka, in practice we'll always get laggy movement).

The following is an example of what could happen:

Client Timestamp 1: 4000 ms
Packet arrival timestamp: 1000 ms
Client Timestamp 2: 4500 ms
Packet arrival timestamp: 1800 ms
Correct Movement delta: 500 ms
Mangos Caused Delta: 800 ms

So, although we could essentially fix the issue by just removing that line above, there still remains one problem. That is that the movement timestamp is effectively user input, and as user input someone with access to a packet sniffer could very well do the following:

Movement 1: 3600 ms
Movement 2: 4100 ms (Malicious user sets this value to something else, like 45000 or 2000.)

Although I haven't specifically examined how the client uses the delta timestamp to calculate movement, I tried feeding malicious input and essentially I got my character teleporting around 5-10 yards at a time (it looked like a severe case of a rogue lagging with sprint), effectively becoming untargetable.

In that area I decided to, after having packet sniffed some of Retail's movement code and notice they changed the values too, implement a sort of callibration code, that would recalculate the value in server time and make sure the values did not sway too much. This is pretty much the first draft: (Note that this code is still in an unaccepted state @ our repo, since I still haven't looked over if there's other ways to exploit this, or even if my fix is solid enough to get added.)

Quellcode

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28

// Called as a part of each HandleMovementOpcodes that passes the Validity check void WorldSession::SynchronizeMovement(MovementInfo &movementInfo) { // We want the time to be based on server time uint32 currMsTime = WorldTimer::getMSTime(); // Invalidate synchronizations older than 750 ms (and start new) if (lastMoveTimeServer < currMsTime - 750) { lastMoveTimeServer = currMsTime; } else { // The time that is between last client's time and current client time gets applied to last server time uint32 clientMsDiff = movementInfo.GetTime() - lastMoveTimeClient; if (clientMsDiff > 750) // Do not accept too big diffs (also covers values that result in a negative diff) lastMoveTimeServer = currMsTime; else { uint32 serverDiffApplied = lastMoveTimeServer + clientMsDiff; lastMoveTimeServer = serverDiffApplied; } if (lastMoveTimeServer > currMsTime) lastMoveTimeServer = currMsTime; } lastMoveTimeClient = movementInfo.GetTime(); movementInfo.SetTime(lastMoveTimeServer); // Same function as UpdateTime(), just renamed }

I realize now this became a pretty long post, hopefully someone was wondering about why the problem came about and find this informative .

Zitat von »Tollw00t«

Wenn man jetzt von dem Problem absieht, dass Schaka jetzt erläutert hat sind für mich an nächster Stelle:

1. Extrahit Delay
2.Sachen wie Charge / Gifte gelten als spells und benötigen somit spellhit / sind reflektierbar / werden von resi beeinflusst (vgl Gifte vs Sl-Lock)

Zitat von »Tollw00t«

Wenn man jetzt von dem Problem absieht, dass Schaka jetzt erläutert hat sind für mich an nächster Stelle:



1. Extrahit Delay

2.Sachen wie Charge / Gifte gelten als spells und benötigen somit spellhit / sind reflektierbar / werden von resi beeinflusst (vgl Gifte vs Sl-Lock)

Zitat

Ich hab ne relativ lange PvP-Prioritätenliste vor ner Weile intern für die Team-Devs veröffentlicht, falls es da Fragen von den freien Devs gibt. Ich will die nur nicht veröffentlichen, weil dann wieder sofort ne riesen Erwartungshaltung hinterherkommt.