implement Elapsed Time compensation to Attack Timer adjustements and DW adjustment

[balakethelock]

🍰 Pullrequest

• Starting attack state on a mob right now resets your swing timer to full. This PR makes it delay by 200ms instead.
• Right now any attack while DW locks the unit from making an attack with the other weapon for 200ms. This PR removes that delay.
• Right now swing timers take longer than they should based on server tick rate. The longer the elapsed time between updates, the more lost swings will accumulate over time. This PR makes elapsed time "flow" into the upcoming swing reducing its remaining timer by as much as the swing before it was delayed.

Proof

https://discord.com/channels/484741332850573317/1155578838353449141/1190423674512216194
The thread includes:

• sheet showing mainhand and offhand swings happen within as little as 017ms between each other. That proves that hits of another weapon extending the timer of the other weapon is incorrect.

• vanilla video showing a rogue using his opener attacks instantly with the MH, the OH attack follows in very short time, NOT a full reset which would have been 1.7 secs considering Warblade of the Hakkari swing speed. Restarting autoattacks after gouge works the same as opening from stealth: If a DW unit starts attacking a mob while both its weapons are ready, it will always swing with the MH immediately, and with the OH a short while later.

• these fight club posts discussion1 and discussion2 and discussion3 assert the existence of a delay between first MH and first OH swing, its variation from 200ms to 600ms can most likely be explained by batching.

• The non existence of some system to compensate a unit's swing timer for long update times seems highly unlikely because that would make swing speed in practice slower than it should based on the server's tick rate and that would be most apparent in lengthy fights by seeing units do less total swings than one predicts they would do by fight length (secs)/attack speed (swing per sec)= total swings

• Generally looking at combat logs and sniff parses I haven't seen that happen. Most likely what happens is an attack could be ready at t=0, but it's not until t=100ms that a game update happens and the unit attacks. In Vmangos the next swing happens at 100ms+attackspeed, but in classic it happens at 0+attackspeed

Issues

• None

How2Test

https://github.com/vmangos/core/assets/111737968/1a5fb8bd-73f1-422a-a86c-c8a0d694a3e7
First attack on a mob is always mainhand, followed by offhand in 200ms. After that, if I walk out of melee range and walk back in, the two will be synced (they still won't happen concurrently in one game update, but the offhand will get compensated on each swing for the delay that happened to it on the previous one)

https://github.com/vmangos/core/assets/111737968/0780fd01-c008-4183-b21a-4d956daf9d41
After the PR ranged autoattacks work fine. They also are affected by the compensation in lost time from long updates.

To debug the swing timers, I wrote this into the ResetAttackTimer function:

void Unit::ResetAttackTimer(WeaponAttackType type, bool compensateDiff/*= false*/)
{
    m_attackTimer[type] = uint32(GetAttackTime(type) * m_modAttackSpeedPct[type]) + ((compensateDiff) ? std::min(m_attackTimer[type], 0) : 0);
	Player* pPlayer = ToPlayer();
	if (pPlayer)
    {
        ChatHandler(pPlayer).PSendSysMessage("%i swing. MH %i / OH %i / Ranged %i", type, GetAttackTimer(BASE_ATTACK), GetAttackTimer(OFF_ATTACK), GetAttackTimer(RANGED_ATTACK));
    }
}

Todo / Checklist

• None

[ratkosrb]

You forgot to initialize it in the constructor.

[ratkosrb]

Also member variables need to start with m_ before the name.

[ratkosrb]

You forgot indentation.

[ratkosrb]

I suggest to add a separate if statement below instead of doing it like that.

[balakethelock]

If I try to do it like this,. m_attackTimer was already set to the attackspeed in the line above it, so this will always add 0

m_attackTimer[type] = uint32(GetAttackTime(type) * m_modAttackSpeedPct[type])
if  (compensateDiff) 
    m_attackTimer[type] += std::min(m_attackTimer[type], 0)
// m_attackTimer was already set to the attack speed above, so this will always add min(m_attackTimer, 0) = 0

So I can first save it then add the saved value after setting to 0 like this:

int32 savedAttackTimer = std::min(m_attackTimer[type], 0)
m_attackTimer[type] = uint32(GetAttackTime(type) * m_modAttackSpeedPct[type])
if  (compensateDiff) 
    m_attackTimer[type] += savedAttackTimer 

Is this method better?

[ratkosrb]

What about

if (compensateDiff && m_attackTimer[type] < 0)
    m_attackTimer[type] += int32(GetAttackTime(type) * m_modAttackSpeedPct[type])
else
    m_attackTimer[type] = int32(GetAttackTime(type) * m_modAttackSpeedPct[type]);

[ratkosrb]

Wouldn't this change cause off hand attack to never go off when server has high update time and main hand attack speed is lower than it?

[balakethelock]

Yes it would in the case where the server update time is longer than mainhand attack speed. It probably shouldn't happen, but I have no retail wow videos of a DW unit attacking when the server is that laggy.

Note though "DW never hits if the server update time is longer than MH attack speed" is already true for how vmangos is before this PR, because of this:

                if (HaveOffhandWeapon())
                {
                    if (GetAttackTimer(OFF_ATTACK) < ATTACK_DISPLAY_DELAY)
                        SetAttackTimer(OFF_ATTACK, ATTACK_DISPLAY_DELAY);
                }

Making every world update when its very high update time, the mainhand lands and delays the offhand by 200 ms so it's no longer ready within that same update, then the next update happens and, the MH is ready so it attacks with it and delays the offhand by 200ms again, and so on making the offhand never hit.

I'm not sure what the retail wow would do in this situation. I don't have enough data to conclude if an offhand can hit in the same world update as a mainhand. People in classic discords describe a situation where if offhand and mainhand hit very close then the warriors with the talent flurry only lose one charge of the buff. Is that a spellbatching thing or were melee attacks batched as well in classic era, I don't know.

[balakethelock]

This PR is wrong. New data has emerged, thanks to Guybrush who did this log for us from SOD (should be the same core as classic era afaik, just different world DB)

https://vanilla.warcraftlogs.com/reports/hLWxwgcDv1mHzBtT#fight=1&view=events&type=damage-done

In this log he had a 2.4 speed mainhand and 1.6 speed offhand. He commences the autoattack command at melee range (this is important) and he hits immediately with mainhand. Offhand follows 0.8 secs later (the offhand's speed divided by two). Then after 2.4 secs from combat start the two swings are perfectly aligned and happen in the same world update.

So that made me dig more, and I found this log:

https://vanilla.warcraftlogs.com/reports/t4P7rnymKgaFMQvW#fight=44&type=summary&source=32&view=events
Where a rogue has a 1.8 speed offhand. The rogue also commences the autoattack command at melee range (He appears to start his attacks with sinister strikes, and it only starts autoattacks at melee range) the offhand swing happens exactly 0.9 secs after mainhand, that's again offhand speed divided by two!

In the same raid and same boss fight is this other player
https://vanilla.warcraftlogs.com/reports/t4P7rnymKgaFMQvW#type=summary&view=events&source=43&fight=44
This player hits with his offhand only 0.09 sec after his mainhand, effectively these can be treated as happening on the same time. I suppose the difference between this case and the cases before it is this person queued his autoattacks before reaching the boss, while the cases before queued their autoattacks once they became at boss melee range.

Will use this new information to change the PR