[Targeting] Fix bug when using /tar on invalid target #3407
Conversation
|
Looks good to me after some testing. 🥇 |
|
Looks good to me. Would probably be good to let some others chime in. |
Akkadius
changed the title
|
In general, as long as testing and function works as expected - I approve of this PR. For folks trying to pull this PR in to merge with their fork, this is going to be an absolute nightmare to understand what has changed for merge conflicts. This should be updated with only the changes and not the formatting changes. |
My suggestions for a merge if auto merge doesnt work is to just copy these two functions verbatim over the top of existing ones. |
Using /tar on an invalid target should not negate a current target. (Tested on live)
My previous attempt to fix this was invalid, as Handle_OP_Target was getting called for multiple packets, so my fix could not reside there alone.
In my digging I found that hitting escape (to remove target), mobs dying, clicking with the mouse, all issued direct OP_TargetMouse commands which should bahave as we have always done them. However, our code also called the same code for OP_Target, which actually only seems to be meant to server as a validation of a /tar command. If we accept the target, the client immediately sends the corresponding OP_TargetMouse. If we reject it, the client sends nothing further.
This fix seems to solve the original issue while not breaking the rest, which is much better than the last shitty PR I submitted.
The diff is hard to read because of indentation changes caused by code removal. Handle_TargetMouse is exactly whaty Handle_Target used to be, except it mo longer handes accept/reject. HandleTarget has been refactored for clarity, and only handles accept/reject.