
It seems that the Auto Attack feature causes the left-click input to be duplicated
My4096 opened this issue · 7 comments
When the Auto Attack feature is enabled, if casting a spell quickly and then immediately input R-L, sometimes the L input is registered again, triggering the 3rd spell.
This doesn’t always happen, but I’ve confirmed that this issue occurs regardless of which spell is cast before R-L.
The issue does not occur when clicking very slowly or when the Auto Attack feature is disabled.
- Wynntils version: 2.4.7
- Mod loader: fabric-loader-0.16.2-1.21.1
- Operating system: Windows 10
Yeah this is the issue I thought was a thing I wanted to fix before reenabling the feature by default. Thanks for the repo steps I'll look into it later this week
It really bugs me that these edge-cases keep popping up. It is obviously a hard thing to get the spell casting and auto attacking working properly, in tandem, and in all kind of situations.
I think the current whack-a-mole approach is not fruitful. Even if we can manage to fix all bugs for now, any change on the Wynncraft side risks wrecking havoc again.
So we need to step away a bit, and tackle the problem from another angle. Do we need to write some kind of testing framework? Can we make some kind of formal description on the state of minecraft packages sent, what wynn has registered and replied to us, what we want to achieve, and what we plan to do next and when to achieve this?
I also seriously believe that we need to collect all "fake clicking" stuff in a single class. We cannot share that implementation across features on the current low level with direct minecraft interactions. We need to have a way to basically let auto attack say:
Handler.Clicker.addRepeatingActivity(LEFT_CLICK, BACKGROUND_PRIORITY, AutoAttackFeature::checkIfStopRequested)
and cast spell like Handler.Clicker.addSequence([LEFT_CLICK, LEFT_CLICK, RIGHT_CLICK], HIGHEST_PRIORITY)
or something like that. Or maybe not even saying LEFT or RIGHT, but just ATTACK or USE, or ATTACK_PRIMARY and ATTACK_SECONDARY (for spells) or whatever. Anyway, having a single class that is not a Feature that is responsible for making sure that the request from the Feature is carried out in a correct way, without any ill side effects or interactions with other requests etc.
Unfortunately I don't have as much time for Wynntils as I used to anymore and at the moment I'm interested in implementing some of the long requested lootrun features, but once I've finished that I could revisit this, might give me the chance to come at this problem with a fresh start.
It's not often I need to wear my Lead Developer's Design&Architecture hat, but this is one of them. We need a single class, which will be the only one allowed to send fake mouse clicks, and it has the responsibility to make sure they are sent correctly and recieved/interpreted as intended by Wynncraft. And we need to be able to test it, somehow, by dealing it a carefully controlled series of responses to the packets they send, such as it seems like packets getting lost or messed up, or that Feature clients request contradictory actions.
I apologize I do not have enough time myself to step in and write such a class. I don't know if @kristofbolyai has either. I do think that you are ready to rise to the challenge @ShadowCat117, if you want. I can provide feedback and some rough guidance on what I think needs to be done.
The issue stems from SpellEvent.Completed
being posted after the 4th input (1st of 2nd spell).
Tick count logged every tick
[16:38:27] [Render thread/INFO] (Minecraft) [STDOUT]: onTick 116470
Manual right click sent
[16:38:27] [Render thread/INFO] (Minecraft) [STDOUT]: Right click
preventWrongCast updated to current tick count + 60
[16:38:27] [Render thread/INFO] (Minecraft) [STDOUT]: preventWrongCast 116530
[16:38:27] [Render thread/INFO] (Minecraft) [STDOUT]: onTick 116471
[16:38:27] [Render thread/INFO] (Minecraft) [STDOUT]: onTick 116472
[16:38:27] [Render thread/INFO] (Minecraft) [STDOUT]: onTick 116473
Manual right click sent
[16:38:27] [Render thread/INFO] (Minecraft) [STDOUT]: Right click
preventWrongCast updated to current tick count + 60
[16:38:27] [Render thread/INFO] (Minecraft) [STDOUT]: preventWrongCast 116533
[16:38:27] [Render thread/INFO] (Minecraft) [STDOUT]: onTick 116474
[16:38:28] [Render thread/INFO] (Minecraft) [STDOUT]: onTick 116475
[16:38:28] [Render thread/INFO] (Minecraft) [STDOUT]: onTick 116476
[16:38:28] [Render thread/INFO] (Minecraft) [STDOUT]: onTick 116477
Manual right click sent
[16:38:28] [Render thread/INFO] (Minecraft) [STDOUT]: Right click
preventWrongCast updated to current tick count + 60
[16:38:28] [Render thread/INFO] (Minecraft) [STDOUT]: preventWrongCast 116537
[16:38:28] [Render thread/INFO] (Minecraft) [STDOUT]: onTick 116478
[16:38:28] [Render thread/INFO] (Minecraft) [STDOUT]: onTick 116479
[16:38:28] [Render thread/INFO] (Minecraft) [STDOUT]: onTick 116480
Issue is here, right click is sent, but SpellEvent.Completed is triggered afterwards which resets preventWrongCast
Manual right click sent
[16:38:28] [Render thread/INFO] (Minecraft) [STDOUT]: Right click
preventWrongCast updated to current tick count + 60
[16:38:28] [Render thread/INFO] (Minecraft) [STDOUT]: preventWrongCast 116540
Spell Completed event, preventWrongCast set to minimum int value
[16:38:28] [Render thread/INFO] (Minecraft) [STDOUT]: Spell completed
[16:38:28] [Render thread/INFO] (Minecraft) [STDOUT]: onTick 116481
[16:38:28] [Render thread/INFO] (Minecraft) [STDOUT]: onTick 116482
Auto attack sends a left click
[16:38:28] [Render thread/INFO] (Minecraft) [STDOUT]: Sending swing at 116482
Manual left click sent
[16:38:28] [Render thread/INFO] (Minecraft) [STDOUT]: Left click
#3040 did fix this issue but caused a more prominent issue so this should be solved differently