#### Brief overview of PR changes/additions
- Removed 8 comments that only repeated the statement or assertion
message next to them
- Kept 1 of the 16 identical copies of the `lua_next()` key-copy note in
`TLuaInterpreterMedia.cpp`
- Comment-only: zero code lines changed
#### Motivation for adding to Mudlet
Reading a comment and then the code that says the same thing is wasted
effort; the rationale comments that document real gotchas are all
untouched.
#### Other info (issues closed, discussion etc)
Result of a pass over the last month of commits on `development`. The
vast majority of comments added there explain *why* rather than restate
*what*, so this is deliberately a small diff.
**Test case:** `git diff development...HEAD` shows only comment lines
removed; build and test suites are unaffected.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Vadim Peretokin <vadim.peretokin@mudlet.org>
#### Brief overview of PR changes/additions
The 12 functional-test sites that dereferenced the result of
`getActiveHost()`, `getArea()`, `getRoom()` or `getHost()` without first
checking it for null now guard the pointer with `QVERIFY(ptr)` before
use. The other lookup sites this PR touched already had a `QVERIFY` and
are left as they were.
#### Motivation for adding to Mudlet
CodeQL's `cpp/inconsistent-null-check` rule flagged these lookups
because some call sites checked the returned pointer for null while
others in the same files dereferenced it directly. Adding `QVERIFY(ptr)`
at the previously-unchecked sites makes the handling consistent: every
lookup result is verified before it is dereferenced, so a null result
fails the test loudly instead of crashing.
`QVERIFY` is the project's accepted idiom for guarding a pointer in a
test, so it is used here rather than an `if (!ptr) { QFAIL(...); }`
block. CodeQL does not recognise `QVERIFY(ptr)` as a null check (the
branch it generates tests `qVerify()`'s return value, not the pointer
itself), so it will keep reporting `cpp/inconsistent-null-check` on
these sites and on the pre-existing `QVERIFY` sites. Those alerts will
be dismissed as false positives rather than changing the test style to
satisfy the checker.
#### Other info (issues closed, discussion etc)
Test files only - no production code is touched. The 12 newly-guarded
sites are:
- `MapRoundTripTest.cpp` - 2 (`pAreaA`, `pAreaB`)
- `TriggerSameLineMatchTest.cpp` - 6 (`host`)
- `TFeedTriggersRecursionTest.cpp` - 4 (`host`)
**Test case:**
Build and run the five affected binaries under the functional-test flock
- all pass:
- `TFeedTriggersRecursionTest` - passed
- `TriggerSameLineMatchTest` - passed
- `MapRoundTripTest` - 6 passed, 0 failed
- `UndoServerWrapTest` - passed
- `UndoServerWrapReplay` - 2 passed, 1 skipped (manual replay tool;
skips without `REPLAY_CAPTURE`/`REPLAY_OUT`)
#### Brief overview of PR changes/additions
Convert raw millisecond integer literals at time-duration call sites to
`std::chrono` literals, and add `#include <chrono>` to each touched
translation unit. Examples:
- `QTimer::singleShot(0, ...)` → `QTimer::singleShot(0ms, ...)`
- `mpTimerReplay->setInterval(1000)` → `setInterval(1s)`
- `mPendingTimer.start(60000)` → `start(1min)`
- `QObject::startTimer(50)` → `startTimer(50ms)`
- `QTest::qWait(100)` → `QTest::qWait(100ms)`
- `QThread::msleep(10)` → `QThread::sleep(10ms)`
This is a semantics-preserving refactor - every duration is kept exactly
equal to before (e.g. `1000` ms becomes `1s`, `60000` ms becomes
`1min`). No behavioural change.
#### Motivation for adding to Mudlet
Chrono literals make time durations self-documenting and type-safe. `1s`
/ `100ms` read unambiguously where a bare `1000` / `100` forces the
reader to remember each API's unit, and the compiler now rejects unit
mismatches. Only genuine duration arguments were converted - loop
counts, scroll-line counts, sizes, ports and the like were deliberately
left as plain integers.
All targeted APIs provide `std::chrono` overloads in the minimum
supported Qt (6.8.2): `QTimer::singleShot`/`start`/`setInterval` (5.8),
`QObject::startTimer` (5.9), `QThread::sleep(std::chrono::nanoseconds)`
(6.6) and `QTest::qWait(std::chrono::milliseconds)` (6.7).
#### Other info (issues closed, discussion etc)
Test case: the full application builds cleanly and the entire functional
`ctest` suite passes. The only failing test is the known, pre-existing
`PasswordMigrationTest` LSan exit-leak (GTK3/fontconfig noise), which is
unrelated to this change.
Assisted-by: Claude:claude-opus-4-8
#### Brief overview of PR changes/additions
- TelnetServerStub now binds an OS-assigned (ephemeral) port; its log
reports the actual bound port
- All 11 functional tests that hardcoded listen ports (three shared port
4000, two pairs shared 4003/4004) now read the real port back via
serverPort()
#### Motivation for adding to Mudlet
Concurrent test runs (parallel CI jobs, multiple checkouts on one
machine) collided on the fixed ports, causing flaky bind failures and
tests connecting to the wrong run's server.
#### Other info (issues closed, discussion etc)
Follows the pattern GMCPCharLoginTest already used. Verified by running
two copies of TelnetTextDisplayedTest simultaneously - both passed on
distinct ports (37503/42303), impossible before.
**Test case:** Run the functional suite twice in parallel (two build
dirs or ctest -j2 repeated); no "address already in use" failures.
Assisted-by: Claude:claude-opus-4-8
#### Brief overview of PR changes/additions
Abort with a Lua error (naming the trigger) when trigger processing
recurses past a depth limit, instead of overflowing the stack. Covers
both `feedTriggers()` and `feedTelnet()` loops, each with a limit sized
to its stack usage.
#### Motivation for adding to Mudlet
A trigger whose action calls `feedTriggers()` with text that re-matches
it recurses the C++ stack until Mudlet hard-crashes (Sentry `fbda193d`,
`EXCEPTION_STACK_OVERFLOW`); this turns that into a clear, recoverable
script error. Review found `feedTelnet()` crashes the same way, so it
gets the same treatment.
#### Other info (issues closed, discussion etc)
Crash seen on Windows in Sentry. The same loop on Linux/macOS only
reaches Lua's own `C stack overflow` guard (~200 nested C calls) -
cryptic, but no crash; Windows' smaller ~1 MB stack dies first. The
fix's depth limit (50) trips before either, so every platform gets the
same clear, named error.
`feedTelnet()` needs a much lower limit (5): each nested telnet
processing level holds ~100KB of stack buffers, so a 1MB stack dies
after ~8 levels. Fixing this also surfaced a reentrancy bug where nested
`feedTelnet()` re-posted the ancestors' pending `mMudData`, duplicating
output lines and abort errors; `postData()` now detaches the data before
posting.
**Test case:**
1. New regex trigger: pattern `^loopme$`, script
`feedTriggers("loopme\n")`.
2. Run `feedTriggers("loopme\n")`.
3. Before: crash (Windows) / `<C stack overflow>` (Linux). After: it
stops with an error naming the trigger and stays running.
4. Same with `feedTelnet("loopme\n")` in the trigger and prompt (on a
disconnected profile): stops with a single error instead of crashing.
---------
Signed-off-by: Vadim Peretokin <vperetokin@hey.com>
