Commit graph

3 commits

Author SHA1 Message Date
Vadim Peretokin
930ea5af5c
infrastructure: trim the comments left behind by two merged QA fixes (#9708)
#### Brief overview of PR changes/additions
- Comment-only. `git diff origin/development...HEAD` changes no
statement, expression or declaration - every added and removed line is a
comment. 238 comment lines become 98.
- Applies the house standard to the comments added by "fix: a trigger
that re-creates itself freezes Mudlet" (#9697) and "Fix user key
bindings on Ctrl+1 to Ctrl+9 and Ctrl+Tab" (#9703): no historical
passages, and the rest cut to what a reader cannot derive from the code.
- Corrects four claims that were wrong, two of them inherited from those
PRs: a fires-per-line measurement taken with a smaller budget than the
one that shipped, an over-general note on `shortcutInstalledFor()`, a
`KeyUnit::disableKey()` note that had the mechanism backwards, and a
test comment crediting the `isEmpty()` guard for a result it does not
produce.

#### Motivation for adding to Mudlet
Both PRs merged while their comment-reduction pass was still in flight,
so the trim never landed with them.

#### Other info (issues closed, discussion etc)
The gotchas worth keeping survive in shorter form: why the same-line
creation budget is counted per pass rather than sharing the
`feedTriggers()` depth counter, why permanent triggers get
`deactivate()` and not `setIsActive(false)`, why `mCleanupSet` rather
than the deactivation is what stops `enableTrigger()` resurrecting a
spent trigger, that `QShortcutMap` retries with consumed modifiers
stripped, and the `Key_Backtab` versus `Shift+Tab` spelling.

The matching trim for "fix: stop treating long-time Mudlet users as
brand new players" (#9695) already landed separately as #9707, so it is
not repeated here.

No demo video: a comment-only change is not observable on screen.

**Test case:** `ctest` in the build directory - 79/80, with
`TelnetBenchmark` timing out only under parallel load (31s standalone
against a 60s limit) on a path this PR does not touch.
`TriggerSameLineMatchTest`, `UnitDeferredDeleteTest`,
`ProfileSwitchShortcutTest` and `ExperiencedPlayerGateTest` all pass.

Assisted-by: Claude:claude-opus-5
2026-08-06 15:43:35 +00:00
Vadim Peretokin
ca1648ae30
fix: a trigger that re-creates itself freezes Mudlet (#9697)
#### Brief overview of PR changes/additions

- A trigger whose script creates another trigger matching the same line
kept extending the list `TriggerUnit::processDataStream()` walks, so the
line never finished: 100% CPU and RSS climbing 1.7 GB to 5.9 GB in 44
seconds, from one ordinary line of game text. Same-line matching for
triggers created mid-pass now has a budget (100 per line); when it runs
out the offending trigger is named in an error and what the loop created
during that line is stopped - temporary ones removed, permanent ones
switched off for the session only, so nothing is saved to the profile.
- `enableTrigger()` could resurrect a killed or expired temporary
trigger during the window before its deferred delete runs, so a one-shot
fired twice and a `killTrigger()`ed trigger fired 49 more times. It now
skips anything queued for cleanup, which is what makes the guarantee
`TTrigger::match()` states actually true.
- The behaviour restored by #9458 ("fix: triggers created by other
triggers react to the current line again") is kept: triggers created
while a line is being processed still match that line, chained creation
included. 10 new tests, and the 6 that pin that behaviour still pass.

#### Motivation for adding to Mudlet

Release blocker for 5.0 - the freeze is reachable from ordinary server
text with the standard "one-shot trigger that re-arms itself" idiom, and
4.22.0 was not affected.

#### Other info (issues closed, discussion etc)

5.0 QA findings C11 (hang) and C12. C11 was introduced by eb2627383
(#9458), which deliberately restored pre-#9267 same-line semantics
without bounding them; #9368's depth guard cannot see it, because
nothing recurses. The budget is deliberately its own constant rather
than the `feedTriggers()` recursion depth: the two measure different
resources, and sharing one made a pass entered deep in nested
`feedTriggers()` abort before running anything.

**Test case:** run `function arm() tempRegexTrigger("^HP: 100/100$",
[[arm()]], 1) end arm()` then `feedTriggers("HP: 100/100\n")` - on
development Mudlet freezes for good; here it reports the trigger and
carries on.

Assisted-by: Claude:claude-opus-5
2026-08-06 12:18:33 +02:00
Vadim Peretokin
d0fa11f23e
Fix temporary trigger/alias/key/timer cleanup evicting same-named items (#9682)
#### Brief overview of PR changes/additions

- An expired trigger is deactivated before it is queued for deletion, so
a nested `feedTriggers()` pass cannot fire it again while the deferred
delete is still pending.
- Deleting a temporary trigger, alias, key or timer unlinks only that
item from the by-name lookup table instead of every item filed under the
same name, and `killAlias()`/`killKey()`/`killTimer()` scan past a
same-named item they cannot kill rather than report failure over it.
- `AliasUnit` and `KeyUnit` gain the double-free guards `TriggerUnit`
and `TimerUnit` already had; `stopAllNamedTriggers()` and
`IDMgr:emergencyStop()` now stop named regex triggers too.

#### Motivation for adding to Mudlet

The four lookup tables are `QMultiMap`s, so names are not unique, but
the temporary-item branch used the single-argument `remove(key)` and
evicted live same-named items with it: a permanent trigger could stay
alive yet become invisible to `enableTrigger()`, `killTrigger()` and
`exists()` for the rest of the session. The kill-by-name asymmetry is
the same defect one level up - a permanent item restored from the
profile precedes this session's temporaries in the root node list, so it
stranded the temporary behind it.

#### Other info (issues closed, discussion etc)

Closes #9646, closes #9648, closes #9649, closes #9650

Test case: `permRegexTrigger("Health", "", {"^permanent$"},
[[echo("permanent fired\n")]])`, then `tempComplexRegexTrigger("Health",
"^temp$", [[]], 0,0,0,0,0,0,0,0,0,0)`, `killTrigger("Health")` and
`feedTriggers("permanent\n")` - `exists("Health", "trigger")` still
finds the permanent trigger.

New coverage: `test/functional_tests/UnitDeferredDeleteTest.cpp` (17
cases across all four units) plus additions to `Trigger_spec.lua`,
`Alias_spec.lua`, `KeyBinds_spec.lua` and `IDManager_spec.lua`, three of
which were `pending()` markers for these bugs.

Review turned up an adjacent defect deliberately **not** fixed here:
expiry is accounted for after `execute()` runs, so a trigger whose *own*
script re-feeds the matching line overshoots its `expireAfter`. Fixing
that means moving the expiry accounting ahead of `execute()` while
keeping the "return true to extend" contract, so it is left for a
follow-up and recorded as a `pending()` spec in `Trigger_spec.lua`.

Assisted-by: Claude:claude-opus-5
2026-08-05 19:55:28 +02:00