feat(timers): ability reuse timers — Mend outcomes, Lay on Hands/Harm Touch, and two dead disciplines - #60
Merged
Conversation
The EQTool port matches only the plain success and plain failure lines, so a
monk who crit-mended or worsened their wounds got no reuse timer at all. Mend
has four outcomes and the 6-minute reuse burns on every one of them.
Adds the critical success ("You magically mend your wounds and heal
considerable damage.") and critical failure ("You have worsened your
wounds!") lines, and covers all four at both the parser and handler level.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… line Swept every "* Discipline" in the book against the classes that can train it: 46 of 48 class/discipline pairs already produced a cooldown row, and two were silently dead for different reasons. Puretone (bard 60): its cast_on_you carries the file's trailing ".." artifact. YouFinishCastingParser strips a trailing ".." off the LOG line, so a ".." in the DATA can never be the lookup key either way. C# normalizes exactly one spell (Defensive Discipline); extend that to disciplines. Kept to disciplines on purpose — ~80 spells carry the artifact but 17 of those normalize onto a message another spell already owns, which would move existing best-guess matches. Whirlwind (monk 53): a later classless duplicate named "Whirlwind" claimed its message key first, and the C# index guard then dropped the classed self-buff entirely. Seat the trainable spell at the head instead; the duplicate stays as a lower-priority candidate. Also corrects Harm Touch's recast to 72 minutes — the file claims 30 s for both knight abilities and C# only fixes Lay on Hands. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The two longest reuses a P99 character owns (72 minutes each) had no timer in EQTool or here, so forgetting one is expensive. Both are real rows in spells_us.txt, so the cast messages and the reuse length come from the spell data rather than literals. Attribution is the hard part and it bounds what the handler may claim: these lines name the TARGET, never the user. Two gates keep the guess honest, both chosen to under-detect rather than invent a 72-minute row: - Class: only a paladin gets Lay on Hands, only a shadow knight Harm Touch. - Direction: you harm-touch mobs and lay hands on people, so Harm Touch needs an NPC-looking target and Lay on Hands a non-NPC one. That discards the two common impostors — NPC shadow knights harm-touching the tank on aggro, and NPC paladins laying hands on themselves at low health. The self-target form of Harm Touch is never yours for the same reason. is_npc alone is not enough in either direction (it misses unnamed mobs and matches plenty of ordinary player names), so looks_like_npc pairs it with EQ's own article convention. What survives: another paladin or shadow knight in view using theirs on the same kind of target. The row dismisses like any other, and a later "You can use the ability ... again in" line corrects the countdown. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
prokopto-dev
commented
Aug 10, 2026
prokopto-dev
left a comment
Owner
Author
There was a problem hiding this comment.
Changes requested. The Mend and discipline fixes look solid, and the full suite passes (2014 passed, 2 deselected). The new Harm Touch attribution has a false-positive path that contradicts its stated under-detection policy: common player names in the master NPC list are treated as NPC targets, so an NPC harm-touching such a player creates a false 72-minute timer for the active character.
Review catch: the Harm Touch branch used the same NPC predicate as Lay on
Hands, and that predicate trusts the master NPC list. The list is every NPC
name in the game and 1676 of its entries are single words, so it also matches
ordinary player names ("Bob", "Fang", "Raven"). An NPC shadow knight
harm-touching player Bob therefore started a false 72-minute row — the exact
failure this module claims to refuse, and the one direction where a wrong
"NPC" invents a timer instead of missing one.
The two directions need opposite biases, so there are now two predicates:
- certainly_npc (article convention only) gates Harm Touch, where a wrong
"yes" invents a row. Cost: a named raid mob no longer starts one.
- possibly_npc (article OR master list) gates Lay on Hands, where a wrong
"yes" only costs a missed row, so any suspicion declines.
Adds the collision regression across all three colliding names, asserting the
collision still exists so the test cannot rot into a tautology, plus a guard
that certainly_npc stays strictly weaker than possibly_npc.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reported as "there is no automated timer for mend". Mend turned out to be
implemented and wired already — it just misses half of its outcomes — so this
also audits the rest of the long-reuse abilities and disciplines.
1. Mend fires on all four outcomes (
fix)The EQTool port matches two of Mend's four outcomes. The 6-minute reuse burns
on all four:
You mend your wounds and heal some damage.You magically mend your wounds and heal considerable damage.You have failed to mend your wounds.You have worsened your wounds!Note for the reporter: the two original lines were verified to work end-to-end
before this change, so if a plain success still shows no timer, the actual log
line is worth capturing.
2. Lay on Hands and Harm Touch (
feat)The two longest reuses a P99 character owns (72 minutes each) had no timer
in EQTool or here. New
core/handlers/ability_cooldowns.py.These lines name the target, never the user, so attribution is a guess and
a wrong guess strands a 72-minute row. Two gates, both chosen to under-detect
rather than over-detect:
needs an NPC-looking target and Lay on Hands a non-NPC one. That discards the
two common impostors: NPC shadow knights harm-touching the tank on aggro, and
NPC paladins laying hands on themselves at low health. The self-target form of
Harm Touch (
You writhe in the grip of agony.) is never yours.is_npcalone is not enough in either direction — it misses unnamed mobs(
a froglok knight) and matches plenty of ordinary player names (Bobis inthe master list) — so
looks_like_npcpairs it with EQ's article convention.Known residual: another paladin/shadow knight in view using theirs on the
same kind of target. Nothing in the line distinguishes it. The row dismisses
like any other, and a later
You can use the ability ... again inline correctsthe countdown through
DisciplineCooldownHandler. Flagging this explicitly asthe main thing to push back on if you disagree with the tradeoff.
3. Two disciplines that could never match a log line (
fix)Swept every
* Disciplineagainst the classes that can train it. 46 of 48class/discipline pairs already worked across all seven discipline classes;
two were silently dead, each for a different reason:
cast_on_youcarries the file's trailing..artifact.
YouFinishCastingParserstrips a trailing..off the log line,so a
..in the data can never be the lookup key either way. C# normalizesexactly one spell (Defensive Discipline); extended to disciplines.
Whirlwindclaimed its message key first and the C# index guard then dropped the classed
self-buff entirely. The trainable spell now seats at the head; the duplicate
is demoted, not discarded.
Also corrects Harm Touch's recast to 72 min (the file says 30 s for both knight
abilities; C# only fixes Lay on Hands).
Deliberately scoped: ~80 spells carry the
..artifact, but 17 normalizeonto a message another spell already owns, which would move existing best-guess
matches. Kept to disciplines; the wider fix wants its own issue.
Verification
2014 passed(+~45 new), ruff clean.including the spell window rendering the countdowns and both impostor lines
correctly ignored:
Game data confirmed against the P99 wiki (Lay Hands,
Harm Touch, Mend).
Please merge with a merge commit, not squash, so the
fix/featcommitssurvive for semantic-release.
🤖 Generated with Claude Code