This is impossible to fix as it's either a client bug or we don't have the correct packet data needed.

You see the problem is not the length of the hitlock itself but the problem is when the hitlock is applied.
The server uses aMotion to determine the delay between the attack and the actual hitlock (and substraction of HP) and the server sends the aMotion value to the client as well. However the client just completely ignores this value (I tried to send really high or really low numbers, it doesn't make any difference) and instead just uses the length of the attack animation frame instead. As the attack animation length is completely off for some monsters, it leads to hitlock being very desync server-sided and client-sided.

Without any means to get the client to use the actual aMotion value we send to it, it is impossible to fix. If you want to fix it you need to modify the client so that it does not ignore the aMotion value we send to it.

And where does that attack animation length come from that the client uses?

Probably hardcoded into the client, or it simply depends on the number of attack animation frames in the GRF.

I added two extra fix position codes to the source and it, most of the time, fixes this issue. The client will update where you are when you take damage and thus showing you where you stopped moving. You will not really see the flinch animation but you will see where you are standing.

if ((type&0x02 && !ud->walkpath.path_pos) // Force moving at least one cell.
|| (type&0x04 && td && DIFF_TICK(td->tick, tick) <= td->data/2) // Enough time has passed to cover half-cell
) {
ud->walkpath.path_len = ud->walkpath.path_pos+1;
unit_walktoxy_timer(INVALID_TIMER, tick, bl->id, ud->walkpath.path_pos);
clif_fixpos(bl); //New Add
}

and

if (ud->walktimer != INVALID_TIMER) { // Stop walking, if chasing, readjust timers.
    if (delay == 1) // Minimal delay (walk-delay) disabled. Just stop walking.
        unit_stop_walking(bl,4);
    else {
        // Resume running after can move again [Kevin]
        if(ud->state.running)
            add_timer(ud->canmove_tick, unit_resume_running, bl->id, (intptr_t)ud);
        else {
            unit_stop_walking(bl,4);

            if(ud->target)
                add_timer(ud->canmove_tick+1, unit_walktobl_sub, bl->id, ud->target);
        }
    }
            clif_fixpos(bl); //new add
}
return 1;

}
For the most part, it keeps the players and mobs moving accurately. It doesn't look that smooth when you're first getting used to it but it's a godsend for players.

Client: 2012-04-10

Revision: 14739
Last Changed Author: cydh.ramdh
Last Changed Rev: 14739
Last Changed Date: 2015-10-29 06:16:10 +0100 (Thu, 29 Oct 2015)

Mode: RENEWAL

Is there hope to fix this or can we close this issue as unable to fix?

It is basically a client bug. The client ignores the values we send it for aMotion and just uses whatever it has hardcoded for a specific monster. It's impossible to fix unless someone creates his own client. If projects like RObrowser get more stable they might have implemented it much better.

If you only care about positioning and not about animations, though, you could send fixpos packets before every attack and before being stopped every time. I tried that and it works almost perfectly fine but looks really strange.

I think I finally found a way to make the impossible possible.

I now understand how the client works here.

Basically when you send a clif_damage packet to the client, you can send "sdelay" (where we put AttackMotion) as value. The client will ignore all values above 432 and 432 stands for 1.0x animation speed. 216 for example means play the animation at double the speed.

Each monster has an attack animation and can define the frame in the attack animation on which it displays the damage and makes the target flinch / stop. If the damage frame is undefined, it instead displays the damage / flinch / stop at the very end of the animation.

You can extract the information after how many milliseconds the damage frame will be shown at 1x animation speed from the spr/act files of the client in the sprite\몬스터 folder. Let's call this value "ClientAttackMotion".

Knowing this, we can do the math of what value to send as sdelay to the client to make it show the damage frame at the exact moment we apply the damage and stop the target on the server.

Basically if "AttackMotion" is smaller than "ClientAttackMotion", we need to send the following as sdelay:
sdelay = AttackMotion * 432 / ClientAttackMotion

For cases where "AttackMotion" is greater than "ClientAttackMotion", we need to reduce the time we wait until we apply the damage to "ClientAttackMotion" (currently this is hardcoded to 1000ms instead).

This should fix all position lag for normal attacks at least.