Files
my-botty/docs/codex_analysis.md
2026-06-07 17:52:55 +02:00

540 lines
19 KiB
Markdown

# Codex Fix Analysis
Source files read:
- `docs/fix_plan.md`
- `src/run/nihlathak.py`
- `src/town/town_manager.py`
- `src/inventory/vendor.py`
- `src/char/i_char.py` (`src/char.py` does not exist in this repo; CTA is implemented here)
- `config/game.ini`
- Supporting files needed to trace the failures: `src/town/a1.py`, `src/town/a5.py`, `src/town/a4.py`, `src/npc_manager.py`, `src/pather.py`, and the CTA key section in `config/params.ini`
## Priority 1: Nihlathak approach fails
### Finding
The Nihlathak route has three brittle points:
1. `approach()` returns success immediately after clicking the waypoint and never verifies that the Halls of Pain actually loaded.
2. Level 1 layout detection has no fallback if `NI1_A`, `NI1_B`, or `NI1_C` is stale.
3. `traverse_nodes_fixed()` always returns `True`, so a bad static path in `config/game.ini` cannot be detected until the stairs click times out.
The `config/game.ini` Nihlathak path keys are present:
```ini
ni1_a=871,472, 1205,600, 1162,600, 1169,584, 1169,584, 1232,213, 1221,237, 1164,228, 1145,572, 1146,547, 1223,185
ni1_b=23,187, 23,187, 23,187, 23,187, 12,192, 10,192, 10,190, 123,70, 378,120
ni1_c=118,500, 158,602, 187,577, 217,563, 184,551, 70,413, 127,240, 154,493, 197,504, 218,545, 83,246, 45,526, 300,380
```
So the immediate code fix is not "add missing keys"; it is to verify waypoint/area entry and reduce the failure blast radius when stale templates or coordinates are encountered.
### Exact code that needs to change
`src/run/nihlathak.py`:
```python
wait(0.4)
if waypoint.use_wp("Halls of Pain"): # use Halls of Pain Waypoint (5th in A5)
return Location.A5_NIHLATHAK_START
return False
```
```python
template_match = template_finder.search_and_wait(["NI1_A", "NI1_B", "NI1_C"], threshold=0.65, timeout=20)
if not template_match.valid:
return False
```
```python
self._pather.traverse_nodes_fixed(template_match.name.lower(), self._char)
```
### Proposed fix
Replace the waypoint block with a verified load:
```python
wait(0.4)
if not waypoint.use_wp("Halls of Pain"):
return False
if not template_finder.search_and_wait(["NI1_A", "NI1_B", "NI1_C"], threshold=0.55, timeout=8).valid:
Logger.error("Nihlathak approach: waypoint click did not land in Halls of Pain")
return False
return Location.A5_NIHLATHAK_START
```
Replace layout detection and static path traversal with a lower-threshold retry and an explicit path result check:
```python
template_match = template_finder.search_and_wait(["NI1_A", "NI1_B", "NI1_C"], threshold=0.65, timeout=20)
if not template_match.valid:
Logger.warning("Nihlathak: strict NI1 layout detection failed, retrying with grayscale/lower threshold")
template_match = template_finder.search_and_wait(
["NI1_A", "NI1_B", "NI1_C"],
threshold=0.55,
best_match=True,
timeout=6,
use_grayscale=True,
)
if not template_match.valid:
return False
```
```python
if not self._pather.traverse_nodes_fixed(template_match.name.lower(), self._char):
Logger.error(f"Nihlathak: failed static route {template_match.name.lower()}")
return False
```
If `search_and_wait()` does not support `use_grayscale` in this repo version, use this compatible form instead:
```python
if not template_match.valid:
start = time.time()
while time.time() - start < 6:
template_match = template_finder.search(
["NI1_A", "NI1_B", "NI1_C"],
grab(),
threshold=0.55,
best_match=True,
use_grayscale=True,
)
if template_match.valid:
break
wait(0.2)
```
That compatible form also needs imports:
```python
import time
from screen import grab, convert_abs_to_monitor
```
### Why it will work
This turns the approach from "clicked the waypoint, assume success" into "clicked the waypoint, confirm an NI1 layout is visible." If the waypoint interaction fails or lands somewhere unexpected, the run fails immediately instead of burning 600+ seconds.
The lower-threshold/grayscale retry handles the likely stale-template case without permanently weakening the first pass. The strict threshold still wins when templates are good; the fallback only runs when the current behavior would fail.
The static route check makes future changes safer. `traverse_nodes_fixed()` currently returns `True`, but guarding the call is still correct because it protects this runner if path traversal later gains real validation.
Fresh templates and re-recorded `ni1_*` coordinates are still required if the fallback logs low-confidence matches or reaches the wrong stairs side. The code fix limits total game loss and gives a useful failure point.
## Priority 2: Vendor trade button not found
### Finding
The failure is not in `src/inventory/vendor.py`; that file buys items after the vendor panel is already open. The failing log comes from `src/npc_manager.py`:
```python
Logger.error(f"Could not find {action_btn_key} btn. Should not happen! Continue...")
```
The current town code also returns a vendor location even if pressing the trade button failed. For A5 Malah:
```python
def open_trade_menu(self, curr_loc: Location) -> Location | bool:
if not self._pather.traverse_nodes((curr_loc, Location.A5_MALAH), self._char, force_move=True): return False
if open_npc_menu(Npc.MALAH):
press_npc_btn(Npc.MALAH, "trade")
return Location.A5_MALAH
return False
```
And `press_npc_btn()` does not return `True` on success:
```python
if res.valid:
mouse.move(*res.center_monitor, randomize=3, delay_factor=[1.0, 1.5])
wait(0.2, 0.4)
mouse.click(button="left")
wait(0.04, 0.08)
center_mouse()
else:
Logger.error(f"Could not find {action_btn_key} btn. Should not happen! Continue...")
keyboard.send("esc")
```
The template threshold is also hard-coded very high for white and blue text:
```python
filtered_inp_w, 0.85, roi=Config().ui_roi["cut_skill_bar"]
```
### Exact code that needs to change
`src/npc_manager.py`, `press_npc_btn()` needs to return a boolean and use a retry/fallback. A5/A1/A4 trade functions need to check that boolean or verify the vendor panel.
### Proposed fix
Replace `press_npc_btn()` with:
```python
def press_npc_btn(npc_key: Npc, action_btn_key: str) -> bool:
global npcs
for threshold in (0.85, 0.78):
img = grab()
img = escape_dialogue(img)
_, filtered_inp_w = color_filter(img, Config().colors["white"])
res = template_finder.search(
npcs[npc_key]["action_btns"][action_btn_key]["white"],
filtered_inp_w,
threshold,
roi=Config().ui_roi["cut_skill_bar"],
)
if not res.valid and "blue" in npcs[npc_key]["action_btns"][action_btn_key]:
_, filtered_inp_b = color_filter(img, Config().colors["blue"])
res = template_finder.search(
npcs[npc_key]["action_btns"][action_btn_key]["blue"],
filtered_inp_b,
threshold,
roi=Config().ui_roi["cut_skill_bar"],
)
if not res.valid:
res = template_finder.search(
npcs[npc_key]["action_btns"][action_btn_key]["white"],
img,
threshold,
roi=Config().ui_roi["cut_skill_bar"],
use_grayscale=True,
)
if res.valid:
mouse.move(*res.center_monitor, randomize=3, delay_factor=[1.0, 1.5])
wait(0.2, 0.4)
mouse.click(button="left")
wait(0.2, 0.3)
center_mouse()
return True
if "red" in npcs[npc_key]["action_btns"][action_btn_key]:
img = grab()
_, filtered_inp_r = color_filter(img, Config().colors["red"])
res = template_finder.search(
npcs[npc_key]["action_btns"][action_btn_key]["red"],
filtered_inp_r,
0.78,
roi=Config().ui_roi["cut_skill_bar"],
)
if res.valid:
Logger.warning(f"Cannot afford {action_btn_key} (red button detected). Skipping...")
keyboard.send("esc")
wait(0.3)
return False
Logger.error(f"Could not find {action_btn_key} btn. Should not happen! Continue...")
keyboard.send("esc")
return False
```
Then change A5 Malah trade from:
```python
if open_npc_menu(Npc.MALAH):
press_npc_btn(Npc.MALAH, "trade")
return Location.A5_MALAH
return False
```
to:
```python
if open_npc_menu(Npc.MALAH):
if press_npc_btn(Npc.MALAH, "trade") and is_visible(ScreenObjects.GoldBtnVendor):
return Location.A5_MALAH
return False
```
Make the same pattern in `src/town/a1.py`:
```python
if open_npc_menu(Npc.AKARA):
if press_npc_btn(Npc.AKARA, "trade") and is_visible(ScreenObjects.GoldBtnVendor):
return Location.A1_AKARA
return False
```
and in `src/town/a4.py`:
```python
if open_npc_menu(Npc.JAMELLA):
if press_npc_btn(Npc.JAMELLA, "trade") and is_visible(ScreenObjects.GoldBtnVendor):
return Location.A4_JAMELLA
return False
```
### Why it will work
The bot currently proceeds as if trade opened even when the button was not clicked. Returning `False` stops `TownManager.buy_consumables()` at the correct point:
```python
new_loc = self._acts[curr_act].open_trade_menu(curr_loc)
if not (new_loc and common.wait_for_left_inventory()): return False, items
```
The fallback search keeps the current exact template behavior first, then retries with a slightly lower threshold and grayscale. That covers text color/anti-aliasing differences without making every match permissive.
Verifying `ScreenObjects.GoldBtnVendor` makes the action result state-based. Even if the template click returns true, the caller only continues when the vendor panel is actually open.
Fresh `TRADE` / `TRADE_BLUE` templates are still recommended, but this code fix prevents false success and reduces sensitivity to minor UI rendering differences.
## Priority 3: Stash detection fails
### Finding
The stash failures are in act-specific methods, not in `TownManager.stash()` itself. A1 and A5 use default `IChar.select_by_template()` threshold `0.68`:
```python
if not self._char.select_by_template(["A1_TOWN_0"], stash_is_open_func):
return False
```
```python
if not self._char.select_by_template(["A5_STASH", "A5_STASH_2"], stash_is_open_func, telekinesis=True):
return False
```
The common selector only closes the waypoint menu for A5 stash templates:
```python
if type(template_type) == list and "A5_STASH" in template_type:
# sometimes waypoint is opened and stash not found because of that, check for that
if is_visible(ScreenObjects.WaypointLabel):
keyboard.send("esc")
```
So A1 stash can fail when a waypoint/dialog is left open, and both A1/A5 have no lower-threshold retry.
### Exact code that needs to change
`src/town/a1.py`:
```python
if not self._char.select_by_template(["A1_TOWN_0"], stash_is_open_func):
return False
```
`src/town/a5.py`:
```python
if not self._char.select_by_template(["A5_STASH", "A5_STASH_2"], stash_is_open_func, telekinesis=True):
return False
```
`src/char/i_char.py`:
```python
if type(template_type) == list and "A5_STASH" in template_type:
# sometimes waypoint is opened and stash not found because of that, check for that
if is_visible(ScreenObjects.WaypointLabel):
keyboard.send("esc")
```
### Proposed fix
Change the selector guard in `src/char/i_char.py` to handle all stash templates:
```python
templates = template_type if isinstance(template_type, list) else [template_type]
if any(template in ["A1_TOWN_0", "A5_STASH", "A5_STASH_2"] for template in templates):
# sometimes waypoint is opened and stash not found because of that, check for that
if is_visible(ScreenObjects.WaypointLabel):
keyboard.send("esc")
wait(0.2, 0.3)
```
Change A1 stash to retry lower after the default threshold fails:
```python
if not self._char.select_by_template(["A1_TOWN_0"], stash_is_open_func, threshold=0.68, timeout=4.0):
Logger.warning("A1 stash: default threshold failed, retrying with lower threshold")
if not self._char.select_by_template(["A1_TOWN_0"], stash_is_open_func, threshold=0.58, timeout=4.0):
return False
```
Change A5 stash similarly:
```python
if not self._char.select_by_template(["A5_STASH", "A5_STASH_2"], stash_is_open_func, threshold=0.68, timeout=4.0, telekinesis=True):
Logger.warning("A5 stash: default threshold failed, retrying with lower threshold")
if not self._char.select_by_template(["A5_STASH", "A5_STASH_2"], stash_is_open_func, threshold=0.58, timeout=4.0, telekinesis=True):
return False
```
### Why it will work
The default threshold remains unchanged for normal cases. The lower threshold only runs after a specific stash attempt fails, which narrows the risk of false-positive clicks.
Closing the waypoint menu for A1 and A5 prevents stale UI overlays from blocking the stash templates. This directly addresses the logged sequence where town/stash templates are not found after other town interactions.
The success function already checks for stash/inventory gold buttons:
```python
found = is_visible(ScreenObjects.GoldBtnInventory, img)
found |= is_visible(ScreenObjects.GoldBtnStash, img)
```
That means a lower-threshold click must still produce the actual stash UI to count as success.
Fresh `A1_TOWN_0`, `A5_STASH`, and `A5_STASH_2` templates should still be captured if logs continue to show low match confidence. The code change makes the current templates less brittle and prevents open UI overlays from causing avoidable failures.
## Priority 4: CTA weapon switch fails
### Finding
The CTA code is in `src/char/i_char.py`. It depends on `Config().char["weapon_switch"]`, which comes from `config/params.ini`, not `config/game.ini`:
```ini
weapon_switch=w
battle_orders=f6
battle_command=f5
```
The current CTA routine has two reliability problems:
1. It invalidates active-skill cache implicitly by switching weapons but does not reset `_active_skill`.
2. It verifies the switch back by comparing a screenshot of the previous right-skill icon. That can fail when the same skill exists on both swaps, when the icon is visually similar, or when the UI updates slightly late.
Current code:
```python
while time.time() - start < 4:
keyboard.send(Config().char["weapon_switch"])
wait(0.4, 0.45)
keyboard.send(Config().char["battle_command"])
wait(0.2, 0.3)
if skills.is_right_skill_selected(["BC", "BO"]):
switch_sucess = True
break
else:
Logger.warning("Failed to find Battle Command, swapping weapons again.")
```
```python
while time.time() - start < 4:
keyboard.send(Config().char["weapon_switch"])
wait(0.4, 0.45)
skill_after = cut_roi(grab(), Config().ui_roi["skill_right"])
_, max_val, _, _ = cv2.minMaxLoc(cv2.matchTemplate(skill_after, skill_before, cv2.TM_CCOEFF_NORMED))
if max_val > 0.8:
switch_sucess = True
break
else:
Logger.warning("Failed to switch weapon, try again")
wait(0.5)
return switch_sucess
```
### Exact code that needs to change
Add a helper inside `IChar` and use it whenever the weapon switch key is sent in `_pre_buff_cta()`.
### Proposed fix
Add this method to `IChar`:
```python
def _weapon_switch(self):
keyboard.send(Config().char["weapon_switch"])
self._set_active_skill("left", "")
self._set_active_skill("right", "")
wait(0.55, 0.65)
```
Change the first CTA-side check from:
```python
if skills.is_right_skill_selected(["BC", "BO"]):
keyboard.send(Config().char["weapon_switch"])
wait(0.4, 0.45)
```
to:
```python
if skills.is_right_skill_selected(["BC", "BO"]):
self._weapon_switch()
```
Change the switch-to-CTA loop from:
```python
keyboard.send(Config().char["weapon_switch"])
wait(0.4, 0.45)
keyboard.send(Config().char["battle_command"])
```
to:
```python
self._weapon_switch()
keyboard.send(Config().char["battle_command"])
```
Change the switch-back loop from:
```python
keyboard.send(Config().char["weapon_switch"])
wait(0.4, 0.45)
skill_after = cut_roi(grab(), Config().ui_roi["skill_right"])
_, max_val, _, _ = cv2.minMaxLoc(cv2.matchTemplate(skill_after, skill_before, cv2.TM_CCOEFF_NORMED))
if max_val > 0.8:
switch_sucess = True
break
else:
Logger.warning("Failed to switch weapon, try again")
wait(0.5)
```
to:
```python
self._weapon_switch()
if not skills.is_right_skill_selected(["BC", "BO"]):
switch_sucess = True
break
skill_after = cut_roi(grab(), Config().ui_roi["skill_right"])
_, max_val, _, _ = cv2.minMaxLoc(cv2.matchTemplate(skill_after, skill_before, cv2.TM_CCOEFF_NORMED))
if max_val > 0.8:
switch_sucess = True
break
Logger.warning("Failed to switch weapon, try again")
wait(0.5)
```
Also fix the spelling while touching this code:
```python
switch_success = False
```
instead of:
```python
switch_sucess = False
```
### Why it will work
Resetting `_active_skill` after weapon swap prevents `_select_skill()` from skipping a hotkey press because it thinks the old right skill is still selected. Weapon swap changes the available skill bar, so the cache is no longer trustworthy.
The longer wait gives D2R more time to update the skill icon and weapon state before validation. The current 0.4 second delay is close to the UI transition timing and is likely why the failure is intermittent.
The switch-back validation now accepts the most direct state: the right skill is no longer Battle Command/Battle Orders. The old image comparison remains as a fallback, so this still handles cases where BC/BO detection briefly lags.
The configured key should be checked in `config/params.ini`, not `config/game.ini`. If the user's actual D2R weapon switch key is Caps Lock, then this line must change:
```ini
weapon_switch=w
```
to:
```ini
weapon_switch=capslock
```
Only do that if the in-game key binding is actually Caps Lock; otherwise leave it as `w`.