Walking Hammerdin bugfix #749

drallcom3 · 2022-05-06T02:14:40Z

The Hammerdin often accidentally clicked on monsters while walking to the end location. He did so while having Vigor active (because the bot thinks he's still walking). The existing fix to activate Concentration early did not work. Now it does.
You might have noticed many complaints about it in Discord.

If you want to test:
Do Eldritch or Trav 10x without the fix as a Hammerdin without any form of teleport. You will notice him casting hammers with Vigor active at the beginning.
With my fix that doesn't happen anymore.

Eld, Shenk, Trav, Pindle fixed. The others not, because they're impossible without teleport anyway.

The Hammerdin often accidentally clicked on monsters while walking to the end location. The fix to activate Concentration early did not work. Now it does.

D2RLegit · 2022-05-06T03:33:20Z

Not at a pc but it looks like you took out vigor altogether for the run and replaced it with concentration? Have you verified this still works with a hdin wearing a enigma and also when using charges?

Using teleport charges that much would get expensive.

drallcom3 · 2022-05-06T04:11:17Z

Vigor is still being use like before, up until the safe spot right before a boss. Just the last few meters are done with Concentration.
The bot was supposed to do that all along, but the functionality was broken.

Works fine with teleport charges.
I don't have an Enigma, but I don't see why it won't work. I haven't touched that part. Someone has to test it of course.

Hammerdin now uses teleport charges inside Pindle's house, because why not. Prevents the zombies from charging you. Before no teleport charges were being used.

drallcom3 · 2022-05-07T00:29:59Z

I realized that I could just test it in singleplayer, so I did that. No issues with Enigma. Teleports happily around.

aliig · 2022-05-09T09:08:12Z

src/char/hammerdin.py

-    def on_capabilities_discovered(self, capabilities: CharacterCapabilities):
-        # In case we have a running pala, we want to switch to concentration when moving to the boss
-        # ass most likely we will click on some mobs and already cast hammers
-        if capabilities.can_teleport_natively:
-            self._do_pre_move = False
-


Why was this function removed? I'm not 100% sure of the consequences of this but @VladimirMakaev might know

I was wondering the same which is why I asked if it still works for hdin with tele. Ive been running as a walkadin and seems fine. Ill double verify in a bit if enigma still works.

When debugging self._do_pre_move it turned out that it reverts back to true. The Hammerdin class logic expected it to stay false though. It was the main culprit of why the Hammerdin seemed to "attack" while having Vigor turned on. It should have switched to Concentration, but didn't. Not relying on self._do_pre_move fixed it immediately and then the on_capabilities_discovered wasn't required anymore.

Also it lead to unneccesarily complicated logic constructs that were hard to follow and could be done in a simpler and more readable way.

I wouldn't touch this code. It can easily break something for Enigma / Tele-Charge paladin . And you need to test 3 different version of paladin on all routes to make sure it's sound:

Walkdadin Trav/Shenk/...

Enigma Paladin Trav/Shenk/...

Tele-Charge Paladin Trav/Shenk

You should just cast Concentration and move with PreMove=False that way you won't change to Vigor.
preMove() will flip to Vigor in case of Walkadin or Tele-Charge (there is no way to pass that telecharge is to be used based on current code structure). So in case you're opting to tele-charge then you should manage PreMove part manually. E.g. casting concentration and doing PreMove=False, TeleCharge = True or PreMove=True if you want Vigor to be on for walking / running out of tele charges and walking.

I wouldn't touch this code. It can easily break something for Enigma / Tele-Charge paladin . And you need to test 3 different version of paladin on all routes to make sure it's sound:

1. Walkdadin Trav/Shenk/... 2. Enigma Paladin Trav/Shenk/... 3. Tele-Charge Paladin Trav/Shenk

It didn't even work the way it was intended to work. No need to re-introduce bugs.

This PR has been tested extensively with all three variants. Some people even happily and successfully use it for their daily runs.

You should just cast Concentration and move with PreMove=False that way you won't change to Vigor. preMove() will flip to Vigor in case of Walkadin or Tele-Charge (there is no way to pass that telecharge is to be used based on current code structure). So in case you're opting to tele-charge then you should manage PreMove part manually. E.g. casting concentration and doing PreMove=False, TeleCharge = True or PreMove=True if you want Vigor to be on for walking / running out of tele charges and walking.

Which is what I did.

D2RLegit · 2022-05-09T11:49:03Z

[0.7.3-dev 2022-05-09 06:47:41,781] INFO Capabilities: CharacterCapabilities(can_teleport_natively=False, can_teleport_with_charges=True)

Populates when using walkadin for pindle.
Walkadin does not work for pindle in this pr.

drallcom3 · 2022-05-09T18:45:35Z

I get:
INFO Capabilities: CharacterCapabilities(can_teleport_natively=False, can_teleport_with_charges=False)
and when adding checks further down the line:
DEBUG capabilities.can_teleport_natively: False
DEBUG capabilities.can_teleport_with_charges: False
when doing Pindle without any teleport equipped.

Did you get that message while having a teleport charge item equipped and ready?

I can only achieve
INFO Capabilities: CharacterCapabilities(can_teleport_natively=False, can_teleport_with_charges=True)
if I have a teleport item with charges equipped and a hotkey assigned in-game and a hotkey assigned in the config. Which is (I assume) cirrect behaviour. If you did the test with a teleport charge item equipped, you're not a Walkadin anymore. I just ran the test. With teleport charges the Hammerdin behaves exactly like in master. Which is correct behaviour, since this PR is about walking, not teleport charges. The Hammerdin needs some more fixes, as you have discovered, but I didn't want to make this PR too massive.

In short: I can't see a bug here (that didn't already exist).

D2RLegit · 2022-05-09T18:59:08Z

It saying i have teleport charges when i run pindle even though im not using any teleport or teleport charge items. it should be a walkdin and its being recognized that it has teleport charges. I will try again if you say your not getting that error.

drallcom3 · 2022-05-09T19:06:36Z

Thanks.
If your self.capabilities.can_teleport_with_charges were incorrectly true, that would be a bug well beyond the Hammerdin file (as it's not being set in there and the info shows up before any boss functions are executed).

D2RLegit · 2022-05-09T19:12:13Z

Pretty sure its because you removed

def on_capabilities_discovered(self, capabilities: CharacterCapabilities):
    # In case we have a running pala, we want to switch to concentration when moving to the boss
    # ass most likely we will click on some mobs and already cast hammers
    if capabilities.can_teleport_natively:
        self._do_pre_move = False

but i could be wrong. I am trying pindle on another pr real quick. Works on other pr's your seems to have problems with pindle and a walkadin.

drallcom3 · 2022-05-09T19:31:36Z

What exactly does happen to your Hammerdin when you do Pindle with this PR?

drallcom3 · 2022-05-10T00:12:35Z

It's actually this #758 bug.
The Hammerdin just made us discover it.

VladimirMakaev · 2022-05-12T10:05:36Z

src/char/hammerdin.py

-    def on_capabilities_discovered(self, capabilities: CharacterCapabilities):
-        # In case we have a running pala, we want to switch to concentration when moving to the boss
-        # ass most likely we will click on some mobs and already cast hammers
-        if capabilities.can_teleport_natively:
-            self._do_pre_move = False
-


I wouldn't touch this code. It can easily break something for Enigma / Tele-Charge paladin . And you need to test 3 different version of paladin on all routes to make sure it's sound:

Walkdadin Trav/Shenk/...

Enigma Paladin Trav/Shenk/...

Tele-Charge Paladin Trav/Shenk

VladimirMakaev · 2022-05-12T10:15:44Z

src/char/hammerdin.py

-    def on_capabilities_discovered(self, capabilities: CharacterCapabilities):
-        # In case we have a running pala, we want to switch to concentration when moving to the boss
-        # ass most likely we will click on some mobs and already cast hammers
-        if capabilities.can_teleport_natively:
-            self._do_pre_move = False
-


You should just cast Concentration and move with PreMove=False that way you won't change to Vigor.
preMove() will flip to Vigor in case of Walkadin or Tele-Charge (there is no way to pass that telecharge is to be used based on current code structure). So in case you're opting to tele-charge then you should manage PreMove part manually. E.g. casting concentration and doing PreMove=False, TeleCharge = True or PreMove=True if you want Vigor to be on for walking / running out of tele charges and walking.

Bugfix & refactoring

b6f7f8f

The Hammerdin often accidentally clicked on monsters while walking to the end location. The fix to activate Concentration early did not work. Now it does.

Less teleport charges use

78b4f4e

Using teleport charges that much would get expensive.

Added using teleport charges at Pindle

48f4ce4

Hammerdin now uses teleport charges inside Pindle's house, because why not. Prevents the zombies from charging you. Before no teleport charges were being used.

Added back one missing teleport for charge users at Trav

a72150a

aliig reviewed May 9, 2022

View reviewed changes

drallcom3 mentioned this pull request May 10, 2022

Hammerdin using Vigor #757

Closed

VladimirMakaev suggested changes May 12, 2022

View reviewed changes

drallcom3 closed this May 24, 2022

aliig mentioned this pull request Jun 14, 2022

Bugfix: Hammerdin and FOHdin small fixes, inappropriate vigor casting, etc #868

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Walking Hammerdin bugfix #749

Walking Hammerdin bugfix #749

drallcom3 commented May 6, 2022

D2RLegit commented May 6, 2022 •

edited

Loading

drallcom3 commented May 6, 2022

drallcom3 commented May 7, 2022

aliig May 9, 2022 •

edited

Loading

D2RLegit May 9, 2022

drallcom3 May 9, 2022

VladimirMakaev May 12, 2022

VladimirMakaev May 12, 2022

drallcom3 May 12, 2022

drallcom3 May 12, 2022

D2RLegit commented May 9, 2022

drallcom3 commented May 9, 2022

D2RLegit commented May 9, 2022 •

edited

Loading

drallcom3 commented May 9, 2022

D2RLegit commented May 9, 2022 •

edited

Loading

drallcom3 commented May 9, 2022

drallcom3 commented May 10, 2022

VladimirMakaev May 12, 2022

VladimirMakaev May 12, 2022

Walking Hammerdin bugfix #749

Walking Hammerdin bugfix #749

Conversation

drallcom3 commented May 6, 2022

D2RLegit commented May 6, 2022 • edited Loading

drallcom3 commented May 6, 2022

drallcom3 commented May 7, 2022

aliig May 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

D2RLegit commented May 9, 2022

drallcom3 commented May 9, 2022

D2RLegit commented May 9, 2022 • edited Loading

drallcom3 commented May 9, 2022

D2RLegit commented May 9, 2022 • edited Loading

drallcom3 commented May 9, 2022

drallcom3 commented May 10, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

D2RLegit commented May 6, 2022 •

edited

Loading

aliig May 9, 2022 •

edited

Loading

D2RLegit commented May 9, 2022 •

edited

Loading

D2RLegit commented May 9, 2022 •

edited

Loading