#### Brief overview of PR changes/additions
- A saved variable with a function, userdata or coroutine anywhere
inside it now exports only the members registered in `savedVars`, which
for a table ticked while empty is an empty group. Tables holding nothing
but data keep the full save added in #9762.
- The save-time walk records which saved globals hold such a value;
`XMLexport` turns the ride-along off for those variables only. Silent,
and it costs nothing outside a save.
- New `SavedVariableFenceTest` pins all 11 measured shapes, on disk and
after a reload.
#### Motivation for adding to Mudlet
A half-restored table defeats the `if next(t) == nil then rebuild() end`
guard packages carry, which kills cron-daemon's scheduler permanently on
its first minute wake after a restart.
#### Other info (issues closed, discussion etc)
Fixes#9857
**Test case:** install cron-daemon, tick `cron` in the Variables view,
add a job with a `command` function, restart and wait for the minute
rollover - the daemon keeps running (measured 2/2 canary fires, against
0/2 before).
Assisted-by: Claude:claude-opus-5
Signed-off-by: Vadim Peretokin <vadim.peretokin@mudlet.org>
#### Brief overview of PR changes/additions
- The `MAIN_LOOP_END` block that reacts to a received GA sits **inside**
the per-byte loop of `processSocketData()`. The normal path clears
`recvdGA` before handing the line to `gotPrompt()`; the `mFORCE_GA_OFF`
path only appended a newline and left the flag set.
- So once a GA arrived, every remaining byte of that read re-entered the
block and appended another newline: the rest of the read was emitted one
character per line, and each of those lines ran the full trigger set.
- Clear `recvdGA` in that branch too.
#### Motivation for adding to Mudlet
With this option enabled, a single GA turned the remainder of the read
into a vertical column of single characters and made the client crawl,
since every character was processed as its own line.
#### Other info (issues closed, discussion etc)
Only reachable with "Force telnet GA signal interpretation off" enabled
(Settings -> Special Options). That flag is copied into `cTelnet` at
connect time (`ctelnet.cpp:497`), so it applies from the next connection
- which is also why the group box is labelled as needing a restart.
The option exists so Mudlet ignores GA signalling from older game
drivers, and ignoring the signal is precisely what this branch is for;
it simply never consumed the flag.
**Test case:** with a server that sends text, then `IAC GA`, then more
text **in a single write** so they arrive in one read, and with the
option enabled before connecting: before this change everything after
the GA appears one character per line; after it, the trailing text
renders on one line as expected. `cTelnetBufferTest`,
`TelnetSgrDefaultColorTest`, `TelnetStringSequenceRecoveryTest`,
`GMCPCharLoginTest` and `TriggerSameLineMatchTest` all pass.
---------
Signed-off-by: Jay Howard <jay.patrick.howard@gmail.com>
#### Brief overview of PR changes/additions
- The `ISO 8859-1` branch of `translateToPlainTextInner()` wrapped each
decoded character in a temporary `QString` before appending it, costing
one heap allocation per character on the per-character decode path.
- Append the bare `QChar` instead, which is what the adjacent branch
five lines above (bytes below 128 for table-based encodings) already
does.
- Second commit adds a `latin1_*` phase to `PipelineBenchmark`, which
previously set no server encoding and so never entered this branch at
all.
#### Motivation for adding to Mudlet
Removes a heap allocation per character of received text for every
profile using ISO 8859-1 — worth about **23% more throughput** on that
decode path.
#### Other info (issues closed, discussion etc)
**Measured** with the new `latin1_*` phase on macOS/arm64, Release,
`-DUSE_SANITIZER=""`, two binaries from one toolchain run in 12
interleaved ABBA pairs so run-order effects cancel:
| metric | before | after | delta | pairs favouring | t |
|---|---|---|---|---|---|
| `latin1_lines_per_sec` | 79,629 | 98,322 | **+23.47%** | 12/12 |
+28.61 |
| `latin1_best_pass_ms` | 314.0 | 254.3 | **-19.01%** | 12/12 | -25.58 |
| `text_best_pass_ms` (control) | 294.9 | 296.2 | +0.42% | 4/12 | +0.13
|
ISO 8859-1 has no lookup table, so **every** byte takes the changed line
— roughly 2.4 million heap allocations removed per pass over the 2.4 MB
corpus. The control metric not moving is the check that the delta is
real.
Note the first commit's message says "No benchmark figure is quoted";
that statement is superseded by the second commit and the table above.
Caution when reading the phases: do not compare `latin1_*` against
`text_*` directly. Latin-1 decoding is intrinsically cheaper than UTF-8,
so that phase reads faster regardless of this change. Only the
before/after of the same metric is meaningful.
This is an inconsistency rather than a deliberate choice: both branches
were written in the same commit (#969), five lines apart, and the ISO
8859-1 one was never revisited. `git log -L` over these lines shows only
incidental touches since — CP437 support in #3579, the GBK/GB18030
decoder, a comment typo fix in #1495, a signed/unsigned cast pass, and
the brace-formatting pass in #1115.
Behaviour is unchanged by construction: both forms evaluate the same
`QChar::fromLatin1(ch)` and hand the identical `QChar` to
`QString::append()`; only the temporary `QString` disappears.
**Test case:** `lua setServerEncoding("ISO 8859-1")` then `lua
feedTriggers("caf\233 na\239ve \253\254 \160\176\191\208\247\n", false)`
renders `café naïve ýþ °¿Ð÷`, exercising bytes 0xE9, 0xEF, 0xFD, 0xFE,
0xA0, 0xB0, 0xBF, 0xD0 and 0xF7 through the changed line. Output is
byte-identical before and after, verified against before/after builds on
macOS.
Note for anyone testing: do not use byte 0xFF. `CHAR_END_OF_FILE '\xff'`
(`TStringUtils.h:32`) is Mudlet's internal line-commit and prompt marker
— `CHAR_IS_COMMIT_CHAR` lists it beside `\n` and `\r`, and
`TBuffer.cpp:1668`/`:1684` use it to set `promptBuffer` — so it can
never arrive as displayable text regardless of this change.
`GlyphOverflowTest`, `CopyAsImageTest`, `WindowBackgroundTest`,
`ProfileRoundTripTest`, `TriggerSameLineMatchTest`, `cTelnetBufferTest`,
`TelnetSgrDefaultColorTest` and `TelnetStringSequenceRecoveryTest` all
pass.
---------
Signed-off-by: Jay Howard <jay.patrick.howard@gmail.com>
#### Brief overview of PR changes/additions
- `match_exact_match()` did `QString text = haystack;` then chopped a
trailing newline. The assignment is copy-on-write and cheap, but
`chop()` mutates, forcing the detach: a heap allocation plus a full
character copy of the line.
- The newline is always present - `TMainConsole::runTriggers()` appends
one to every line before dispatch (`TMainConsole.cpp:1570`) - so the
chop always fires and the copy always happens, once per exact-match
pattern, per reachable trigger, per line of game text.
- Use a `QStringView`. Chopping a view moves only its own end pointer,
so nothing is allocated, and the comparison against the needle is
unchanged. Both chop one UTF-16 code unit, so behaviour is identical for
every input.
#### Motivation for adding to Mudlet
Removes a per-line heap allocation and line copy from the trigger
matching path, which runs for every line of game text.
#### Other info (issues closed, discussion etc)
Measured with `PipelineBenchmark` on macOS/arm64, Release,
`-DUSE_SANITIZER=""`, two binaries from one toolchain run in 24
interleaved ABBA pairs so run-order effects cancel: trigger throughput
**+3.28%** (t=6.41), trigger overhead **-6.37%** (t=-4.94). The
text-only control moved +0.23% (t=-0.56, 12/24 paired wins), i.e. no
effect, which is the check that the trigger deltas are real.
Worth stating plainly: the stock benchmark corpus contains **no**
exact-match patterns, so `match_exact_match()` is never entered by it.
Twelve were added locally purely to measure this. The gain therefore
scales with how many exact-match patterns a profile actually has, and is
zero for a profile with none.
**Test case:** behaviour-neutral, so the checks are for regressions
around where the chop lands. Exact-match triggers fire correctly on a
plain ASCII line, on a line with an accented character, on one with an
em dash, and on one containing an emoji (a surrogate pair, i.e. two
UTF-16 code units - the case most likely to expose a code-unit-based
chop). A line with trailing whitespace before the newline correctly does
*not* match. Verified against before/after builds on macOS, plus 5
trigger functional test suites.
---------
Signed-off-by: Jay Howard <jay.patrick.howard@gmail.com>
#### Brief overview of PR changes/additions
- `Host::saveProfile()` no longer pumps the event loop between marking
the save as started and making the `QFutureWatcher` that retires that
mark. A quit is queued (`closeMudlet()` arms it on a zero timer), so it
could be delivered in that gap and run the entire application shutdown
from inside the save - and the rest of `saveProfile()` then carried on
using a `Host` that teardown had already destroyed.
- The nested `Host::waitForProfileSave()` was what let that shutdown
through: it was waiting for a finish notification whose watcher did not
exist yet, and its escape hatch counted a thousand event loop passes,
which on a fast machine are over in well under a millisecond. It is
bounded in wall-clock time now, waits out the background writes on every
pass, and the state it prints if it does give up says something that can
be acted on.
- `saveProfileAs()` had the same pump, and announced
`profileSaveStarted()` before finding out it was going to refuse - which
left the editor's Save Profile action disabled and captioned "Saving…"
with no `profileSaveFinished()` ever coming.
#### Motivation for adding to Mudlet
Uninstall a package and quit straight away and Mudlet can crash on the
way out. It showed up on CI as an intermittent SIGSEGV after a fully
green run, on macOS arm64 and windows64 but not on the slower legs,
preceded by `waitForProfileSave() WARNING - save did not complete after
1000 event loop iterations. State: mWritingHostAndModules=true, writers
pending=0` - which is exactly what a save looks like in the window
between the mark and the watcher.
#### Other info (issues closed, discussion etc)
Closes#9807. Adjacent to #9653/#9684 and #9690, and composes with them:
this is the re-entrancy that opened the window rather than another
dangling pointer.
Reproduced deterministically under AddressSanitizer as a
heap-use-after-free in `Host::pendingXmlSaveFutures()` called from
`Host::saveProfile()`, on a `Host` freed by `~Host()`. Draining a save
that a package change has only queued was considered and left out: the
close path never reaches it (it has always either just started a save or
found one running), and it would turn a multi-select module import from
one coalesced save into one full save per module.
**Test case:** uninstall a package, then quit Mudlet immediately - it
exits cleanly. New `ProfileSaveShutdownRaceTest`; both cases verified to
fail against the unfixed source, the shutdown one by aborting the run
under the sanitizer.
Assisted-by: Claude:claude-opus-5
#### Brief overview of PR changes/additions
- `slot_scriptsSelected()` now drops the noted "Add User Event" item
after tearing the Registered Events list down, instead of relying on
`saveScript()` doing it beforehand. `QListWidget::clear()` drops the
selection before it deletes the items, and that selection change runs
`slot_scriptMainAreaEditHandler()`, which re-notes the item that is
about to be freed.
- Also releases the row the "-" button takes out of that list -
`takeItem()` hands ownership over, and the returned item was being
dropped.
- New `ScriptEventHandlerLifetimeTest` covers the four ways the list
gets torn down while an entry is noted (switch script, re-click the same
script, add a script, jump from the search results), plus renaming and
deleting so the fix cannot overreach.
#### Motivation for adding to Mudlet
Pressing "+" after switching scripts dereferenced freed memory and
killed the client, losing whatever was unsaved.
#### Other info (issues closed, discussion etc)
Fixes#9835. Reported on Windows 10 / Mudlet 4.22.0, and reproduced on
Windows 11 against both 4.22.0 and the current PTB.
Confirmed under ASan as a `heap-use-after-free` in
`QListWidgetItem::text()` from `slot_scriptMainAreaAddHandler()`, freed
by `QListModel::clear()` from `slot_scriptsSelected()`. Where the
replacement item happens to land on the freed block there is no crash
and "+" silently renames the wrong entry instead.
Note this also stops a name typed into "Add User Event" but never added
from following you to the next script.
**Test case:** Script editor > new script, type an event name into "Add
User Event" and press "+", save. Make a second script, save. Select the
first script, click its entry under "Registered Events", then click the
second script and press "+" - 4.22.0 crashes, this branch adds the
handler to the second script.
Assisted-by: Claude:claude-opus-5
Assisted-by: Claude:claude-fable-5
Signed-off-by: Vadim Peretokin <vadim.peretokin@mudlet.org>
---------
Signed-off-by: Vadim Peretokin <vadim.peretokin@mudlet.org>
#### Brief overview of PR changes/additions
- TelnetServerStub::sendRaw() now queues bytes sent before the server
has accepted the client connection and flushes them on accept, instead
of dropping them with only a warning
- Between sessions it still warns and drops, so a dead session's bytes
cannot leak into the next connection; the destructor reports bytes that
were never delivered
- The flush checks the write() return like the adjacent welcome-message
path
#### Motivation for adding to Mudlet
Tests only wait on the client-side connected signal, which can fire
before the stub's server side accepts - on fast runners the first
payload was lost, failing TelnetStringSequenceRecoveryTest twice in a
row on macOS arm64 in #9861's CI. The stub is shared by ~50 functional
tests.
#### Other info (issues closed, discussion etc)
First validated on #9861 (run 31674918225 failed twice on arm64; run
31687605776 with the fix went green on all four platforms); it will be
dropped from that PR once this merges.
**Test case:** TelnetStringSequenceRecoveryTest,
TriggerSameLineMatchTest and UndoServerWrapTest pass (run twice each
locally with the final version).
Assisted-by: Claude:claude-fable-5
#### Brief overview of PR changes/additions
- A line opening with a list marker - `[1364]`, `(3)`, `2.`, `3)` or a
bullet - is now treated as a new line rather than the continuation of
the one above it, so help indexes, shop stock and menus survive the
option being on.
- Only markers that word wrap could not itself produce at the start of a
continuation count. A spaced dash and a parenthesised number over 3
digits are deliberately excluded, since both open genuine wrapped prose.
- 3 new functional tests, including negative controls proving ordinary
prose still rejoins.
#### Motivation for adding to Mudlet
With "undo the game's word wrapping" enabled at 78 columns, a help index
came out with entries glued together - each entry is a sentence that can
end right at the wrap column, so nothing but its marker distinguished it
from a wrapped paragraph.
#### Other info (issues closed, discussion etc)
Also raises the Windows Lua test step to the 3 minute budget Linux and
macOS already have - it was left at 2 and timed out on a slow runner
while the specs were still running. Folded in here rather than split
out, by request.
Follow-up to #9455, which added the option. The check reads the
continuation only, so the last entry of a list can still absorb a
full-width prose line that follows it - left alone deliberately, as
requiring a marker on both lines would stop the first entry of a list
detaching from a header above it.
**Test case:** enable Settings -> Display -> "undo the game's own word
wrapping" at 78, then on a game with a numbered help index (e.g. `help
viking`) confirm each `[NNN]` entry stays on its own line. `ctest -R
UndoServerWrapTest` covers it - 15/15 pass.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Vadim Peretokin <vadim.peretokin@mudlet.org>
---------
Signed-off-by: Vadim Peretokin <vadim.peretokin@mudlet.org>
#### Brief overview of PR changes/additions
- The base UI's chat capture now recognises `You gossip, 'test!'`-style
lines and copies them into the Tells tab
- New `you gossip` shape under the tells gate (18 of 20 gate substrings
used), mirrored in `chatPatterns` so gossip lines stay out of the vitals
harvester
- Package version 1.3.0, `.mpackage` rebuilt; corpus lines added to
StarterUiTriggerCostTest and UI_spec
#### Motivation for adding to Mudlet
Gossip is a staple channel on Diku-style games, and the starter UI
missed these lines entirely.
#### Other info (issues closed, discussion etc)
Only your own gossips are captured for now - the incoming `Bob gossips,
'...'` counterpart can be added the same way if wanted.
**Test case:** `ctest -R StarterUiTriggerCost`, or gossip on any
Diku-style game with the base UI up and see the line land in the Tells
tab.
Assisted-by: Claude:claude-fable-5
#### Brief overview of PR changes/additions
- A profile save read a fresh variable tree by walking all of `_G`
first-seen-wins, so a saved table another global name reached first was
filed under that name and written nowhere. Saves now read only the
globals the profile saves, each in its own dedup scope, and a name the
user saves is never deduped away.
- Same change takes the save off the size of `_G`: 0.269 s -> 0.007 s
per save at 20,000 globals, 0.013 s -> 0.001 s on a default profile
(4.22.0 is 0.002 s).
- `iterateTable()` names the table when it stops at 99 levels of nesting
instead of handing back an empty one, and the save tells the user which
saved variables that leaves empty.
#### Motivation for adding to Mudlet
Silent, permanent data loss on every save with no user action: a stock
4.x profile with EMCO/AdjustableContainer packages lost 1416 of its 1444
saved variable entries on the first 5.0 session.
#### Other info (issues closed, discussion etc)
Fixes#9755. Keeps #9704's fix (quitting with the editor on the
Variables tab) intact - the export still builds a throwaway tree, so the
Variables editor's tree items are never stranded.
Measured on a real profile (`Pox`, fresh isolated HOME): 25 variables /
4 groups / 4.3 KB before, 2267 / 535 / 497 KB after, identical on a
second session. 4.22.0 wrote 1163 / 281 / 258 KB. The difference above
4.22.0 is the live EMCO and AdjustableContainer objects the profile
keeps inside its saved `demonnic` table, which the ride-along rule from
#9517 says to save.
**Test case:** `lua qaShared = {a = "alpha"}`, tick `qaShared` in the
editor's Variables view, then `lua aaaAliasOfShared = qaShared`, quit
and reopen - `qaShared.a` is still there. `ctest -R
XMLexportVariablesTest` covers it; 5 of the 8 new cases were verified to
fail against the unfixed source.
#### Brief overview of PR changes/additions
- A DCS/SOS/PM/APC/OSC sequence (`ESC` followed by `P`, `X`, `^`, `_` or
`]`) now ends at the end of the line it started on if the game never
sends a terminator, instead of discarding everything that follows it for
the rest of the session.
- Half of a sequence left over from a dead connection is dropped when
the next one starts, and the 4096-byte cap no longer leaves the parser
stuck with no way out.
- Well-formed sequences are untouched: Sixel images and Kitty graphics
are still swallowed whole, including ones larger than the cap or split
across packets. Their idle-flush carriage returns no longer hide a
String Terminator either.
#### Motivation for adding to Mudlet
One stray escape byte from a game could black out every line it sent
afterwards - across line breaks, `clear()` and a full reconnect - with
only a BEL able to restore output.
#### Other info (issues closed, discussion etc)
Fixes#9757. The `ESC ]` half of this is not a 5.0 regression, it
behaves identically in 4.22.0, so the bound covers the whole
`mGotOSC`/`mGotString` path rather than just the four introducers 5.0
added. Answering the open question in the issue: text lost to the
blackout is missing from the log as well, because it never reaches the
buffer the log is written from -
`logFollowsTheDisplayThroughAnUnterminatedSequence` covers that.
New `TelnetStringSequenceRecoveryTest` - 46 cases over a real TCP socket
via `TelnetServerStub`. With the fix reverted, 24 of them fail.
**Test case:** connect to any game and have it send `PRE<ESC>Ppayload`
followed by a newline and a few more lines. Every line after it must
still be displayed. Repeat with `X`, `^`, `_` and `]`.
#### Brief overview of PR changes/additions
- Stop importing settings from the pre-4.19 NativeFormat store (macOS
plists / Windows registry) into a freshly created Mudlet.ini
- On those platforms the old store ignores HOME/XDG overrides, so any
machine that ever ran Mudlet <= 4.18 made every fresh install inherit
stale keys and classify as an experienced player - skipping the starter
UI, tour and hints, and failing two functional tests
- Add tripwire asserts so any future write into a fresh Mudlet.ini
before init() fails with a self-explaining message
#### Motivation for adding to Mudlet
A fresh install must be recognised as a new player; anyone who ran 4.19+
was already migrated, and a direct <= 4.18 upgrade only loses UI
preferences (window geometry, appearance, storePasswordsSecurely,
deletedDefaultMuds), never profiles.
#### Other info (issues closed, discussion etc)
Fixes#9741
**Test case:** ExperiencedPlayerGateTest (18/18) and DefaultPackagesTest
(28/28) pass; on a Mac that ran Mudlet <= 4.18 (check with `defaults
read com.mudlet.Mudlet pos`), both suites now pass and a fresh profile
gets the starter UI.
Assisted-by: Claude:claude-opus-5
#### Brief overview of PR changes/additions
- "Copy as image" with nothing selected now copies the visible screen
(timestamps included) instead of doing nothing. With a selection it is
unchanged.
- Copy, Copy HTML and Search on ... genuinely need a selection, so
without one they are greyed out with a tooltip saying so rather than
silently doing nothing.
- Copy as image no longer wipes the clipboard when it runs out of its 3s
budget, crops the drawn lines instead of squashing the whole selection
to fit, and copies a run of blank lines rather than nothing.
#### Motivation for adding to Mudlet
Right-clicking the console and picking "Copy as image" left nothing on
the clipboard, with no hint that a selection was needed.
#### Other info (issues closed, discussion etc)
Fixes#9715
Also fixes two ways a selection could outlive the lines it covers:
clearing a console now clears its selection (`clearWindow()` then "Copy
as image" aborted on Qt's bounds assert), and a selection stranded by
the buffer hitting its size limit is followed down with its lines
instead of copying whatever took their place.
New `CopyAsImageTest` functional test, 15 cases. Verified on X11 with
`xclip -selection clipboard -t image/png -o`.
Assisted-by: Claude:claude-opus-5
**Test case:** Right-click the main console with nothing selected, pick
"Copy as image", and paste - you get a picture of the screen. Select
some text and repeat - you get just the selection.
#### Brief overview of PR changes/additions
- The notepad and IRC client are parentless windows freed only in
`Host::closeChildren()`; a `Host` destroyed without that call orphaned
them. `~Host()` now closes and deletes them, nulling each `QPointer`
first so both teardown paths stay single-delete. Closing (not just
deleting) the notepad also saves the notes and window state.
- The toolbars are not leaked at exit, but survived their profile on
screen holding a freed `TAction`; `~Host()` now deletes them
synchronously.
#### Motivation for adding to Mudlet
Same defect class PR #9700 "fix: trigger editor and deleted item
subtrees leaking memory" fixed for the editor. Two narrow production
paths reach `~Host()` without `closeChildren()` (`requestClose()`
returns early when `mpConsole` is already gone, and `~HostManager` runs
from `~mudlet`), and the test harness takes the second on every run.
#### Other info (issues closed, discussion etc)
Test case: new `HostChildTeardownTest` - three teardown orderings, each
fails without the fix; under ASan+LSan the binary goes from 851,208 to
3,334 leaked bytes (residue is the settings floor fixed in #9694). 79/79
functional tests pass twice; profile-close/quit with the notepad open
are clean under ASan on Xvfb.
Assisted-by: Claude:claude-opus-5
#### Brief overview of PR changes/additions
- `ProfileLifecycleTest` (merged this morning in #9776) seeds
`$XDG_CONFIG_HOME/mudlet` without the `profiles/` subdirectory that
#9712 made the opt-in, so on any machine whose `~/.config/mudlet` holds
profiles it resolves to that instead and fails its own line 331
assertion. Reproduced here before the one-line fix. Its build legs all
finished on 10 Aug 13:52-14:55 UTC and #9712 merged at 22:17 that
evening, so it was merged 16 hours later on green CI that predates the
rule it breaks.
- `XdgRecipeConsistencyTest` stops the next one. It scans `test/*.cpp`
and `test/functional_tests/*.cpp` the way `CMakeListsConsistencyTest`
scans `src/`, and fails on a `mkpath()`/`mkdir()` whose argument spells
a path ending in `/mudlet` unless the file also creates the `profiles/`
opt-in. A test that means it says so with an `xdg-recipe-guard: allow`
comment.
- Comments, strings and raw strings are parsed out first, so a recipe in
prose is not code and an assertion against a `"%1/mudlet"` literal is
not a creation. The sweep reads this file too: its own fixtures spell
the stale recipe out inside string literals.
Test case: the sweep names `ProfileLifecycleTest.cpp:317` before the
fix, and both pre-#9810 files at lines 154 and 173 when those are
checked out of `8901b59d8`; the other 99 test sources are clean, and the
suite is 98/98 locally.
Assisted-by: Claude:claude-opus-5
#### Brief overview of PR changes/additions
- New `ProfileLifecycleTest` (18 test functions) covering the
profile-lifecycle Lua API the busted suite cannot reach, since busted
runs inside a single profile of an application it must leave standing:
`loadProfile`, `setActiveProfile`, `closeProfile`, `closeMudlet`, and
the cross-profile half of `raiseGlobalEvent`.
- Drives each function from a profile's own Lua state and checks the
application state that follows - host pool, tab, main console, active
profile, socket - not just the return value. Teardown is asserted with
`QPointer`s, so the closed profile's objects have to be genuinely gone.
- Runs against a `QTemporaryDir` config dir and an ephemeral stub port,
so it never touches the developer's own profiles and does not collide
with parallel test runs.
#### Motivation for adding to Mudlet
These five functions had no automated coverage at all. Between them they
disagree on almost every convention - `loadProfile`/`closeProfile`
refuse with `nil`, `setActiveProfile` with `false`; all three resolve
names case-insensitively; `raiseGlobalEvent` serialises its arguments to
strings and appends the sender - and none of that was pinned anywhere.
Bug found while writing it, not fixed here: `raiseGlobalEvent`'s
argument-type rejection is a `lua_error()`, which longjmps out of the C
function so the `TEvent` being filled in on the stack is never
destroyed. `raiseGlobalEvent('name', {})` leaks the arguments collected
before the bad one (170 bytes, confirmed under LeakSanitizer). The test
asserts the refusal with the argument-#1 form, which has collected
nothing yet; a comment marks the realistic form as untested until the
leak is fixed.
#### Other info (issues closed, discussion etc)
Coordinated with #9706 (fix-host-child-teardown): no overlap - that PR
covers notepad/IRC/toolbar teardown, this one the Lua API and the host
pool, and both insert into `test/functional_tests/CMakeLists.txt` at
different points.
**Test case:** `ctest -R ProfileLifecycleTest` - 18 tests, ~8s,
LeakSanitizer-checked; full functional suite 59/59 twice;
sabotage-verified by breaking `setActiveProfile`'s tab switch,
`raiseGlobalEvent`'s sender exclusion, `closeProfile`'s close request
and `loadProfile`'s offline flag - 6 of the 18 fail, and only those.
Assisted-by: Claude:claude-opus-5
#### Brief overview of PR changes/additions
- Renames the six test executables whose filenames trip Windows' UAC
installer detection: `UpdaterChecksumTest` to
`ReleaseChecksumPairingTest`, `UpdaterPlatformAssetTest` to
`ReleasePlatformAssetTest`, `UpdaterTeardownTest` to
`NewReleaseDialogTeardownTest`, `PackageSelfUninstallTest` to
`PackageSelfRemovalTest`, `PackageUninstallSaveTeardownTest` to
`PackageRemovalSaveTeardownTest`, `ActionSelfUninstallTest` to
`ActionSelfRemovalTest`. Assertions are untouched.
- Adds a configure-time gate in `test/CMakeLists.txt` that fails with an
actionable message if any test executable name contains install, setup,
update or patch. It checks both the targets a configuration builds and
the test source filenames, so conditionally registered tests cannot slip
past it.
- Documents the naming rule in `test/README.md`.
#### Motivation for adding to Mudlet
Windows treats an unsigned executable named that way as an installer and
refuses to start it, so those six tests reported `BAD_COMMAND` for
anyone running the suite from an ordinary Windows shell - and because CI
runners are elevated, nothing caught it as more tests were added.
#### Other info (issues closed, discussion etc)
Fixes#9748
The gate was verified to fire on a target named after the guard
statement, on one in a subdirectory, on `EventDispatcherTest` (the
message names the offending substring, since "dispatch" contains
"patch"), on a test registered only under `USE_UPDATER` when configuring
with the updater off, and to fail loudly if the walk ever stops finding
executables.
**Test case:** `cmake --build build && ctest --test-dir build` - 92/92
pass; adding a test named e.g. `FooUpdateTest` fails the configure with
an explanation.
Assisted-by: Claude:claude-opus-5
#### 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
- `61c2afd40` (#9712) made `$XDG_CONFIG_HOME/mudlet/profiles` the opt-in
marker for an isolated config root, and an empty
`$XDG_CONFIG_HOME/mudlet` no longer qualifies. It updated three
functional tests to the new recipe and missed two, so
`ProfileDeletionSafetyTest` and `ConnectionDialogCrashTest` resolved to
the real `~/.config/mudlet` and failed in `initTestCase()` on every leg
(run 31428353403). Every open PR inherits that red, because PR builds
merge the dev tip.
- Both now pre-create `mudlet/profiles`, matching the ten sibling tests
and `src/mudlet-lua/tests/README.md`. Product code is untouched.
- An empty `profiles/` still reads as a fresh install
(`anyProfilesExist()` counts subdirectories), so neither test's
first-launch expectations move.
Test case: `ctest -R
"ProfileDeletionSafetyTest|ConnectionDialogCrashTest"` against a
`~/.config/mudlet` that holds profiles - both fail on `development`,
both pass here; the full functional suite is otherwise unchanged.
Assisted-by: Claude:claude-opus-5
#### Brief overview of PR changes/additions
- Appends `QT_ASSUME_STDERR_HAS_CONSOLE=1` to the `ENVIRONMENT` test
property of every registered test - 95 of 95 confirmed with `ctest
--show-only=json-v1` - leaving each test's existing `ASAN_OPTIONS`,
`QT_QPA_PLATFORM` and `ENVIRONMENT_MODIFICATION` untouched.
- One `cmake_language(DEFER CALL)` per directory that registers tests,
rather than the variable copy-pasted into a dozen strings, so a test
added later cannot miss it wherever in the file it lands.
- Drops the 8 `QT_FORCE_STDERR_LOGGING` entries from the four workflows.
`shouldLogToStderr()` is `forceStderrLogging() ||
stderrHasConsoleAttached()`, so the test property now covers what CI was
setting by hand, and every one of those steps runs nothing but `ctest`.
#### Motivation for adding to Mudlet
Qt on Windows diverts QTest's output to `OutputDebugString` unless it
believes stderr has a console attached, and an MSYS2 shell gives it
none, so a failing test reported an exit code with no `FAIL!` lines, no
compared values and no totals. Setting it as a test property fixes local
runs and CI from one place instead of two.
#### Other info (issues closed, discussion etc)
**Test case:** full Linux `ctest` suite 95/95 pass; `ctest
--show-only=json-v1` shows all 95 tests carrying the variable with no
other property changed; `ctest -V` shows it in the test process
environment. The Windows behaviour itself is not reproducible on Linux -
it rests on the reporter's 219 vs 12233 byte A/B and on
`QPlainTestLogger::outputMessage`, which only calls `OutputDebugStringA`
when `!QtPrivate::shouldLogToStderr()`.
Closes#9747
Assisted-by: Claude:claude-opus-5
#### Brief overview of PR changes/additions
- `TMediaPlayer::~TMediaPlayer()` blocks its `QMediaPlayer`'s signals
before the `stop()`/`setSource(QUrl())` that unloads the media, so no
handler is called with a player whose members are going away. The unload
itself is unchanged, and Qt still emits `destroyed()`.
- New `TMediaLoopTest::test_destroyingAPlayerAnnouncesNothing()`, with a
control unload on a player that is not being destroyed so it cannot pass
vacuously.
- `test_continuingToTheNextPassClearsTheEarlierAnnouncement()` declares
its flag ahead of the player, so a lambda connected to that player
always has live stack to write to.
#### Motivation for adding to Mudlet
Emitting signals from a destructor is a landmine for any connected code:
today TMedia's own handlers survive it only because each one locks an
already-expired `weak_ptr`, and the test suite, which does not, aborted.
#### Other info (issues closed, discussion etc)
Closes#9740 "TMediaLoopTest aborts under AddressSanitizer with
stack-use-after-scope".
Reproduced and verified on Linux with a clang ASan Debug build
(`USE_SANITIZER=address`): pre-fix `TMediaLoopTest` aborts with
`stack-use-after-scope` in `~TMediaPlayer` ->
`QMediaPlayer::setSource()`, post-fix the suite is clean. The new test
fails without the destructor change (counts 1 announcement) and passes
with it. GCC does not poison per-variable within a scope, so the abort
only shows up under clang/AppleClang.
**Test case:** `cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Debug
-DCMAKE_CXX_COMPILER=clang++` then `cmake --build build --target
TMediaLoopTest && ctest --test-dir build -R TMediaLoopTest` - passes
instead of `***Exception`.
Assisted-by: Claude:claude-opus-5
**Not for 5.0 - please hold this for the release after 5.0.**
#### Brief overview of PR changes/additions
- `TTextEdit` lays text out in cells of `QFontMetrics::height()`, a
typographic measure rather than the glyph ink box, so at many font sizes
the ink of `_ g j p q y $ @ ( )` reaches a pixel past the bottom of its
cell. On Bitstream Vera Sans Mono at 14pt the underscore is a 1px bar
sitting entirely below the cell, so it disappears completely.
- The screen is now rendered with each line's cell backgrounds painted
before the previous line's glyphs, the screen pixmap has a spare row for
the bottom line's overflow, and partial repaints and scroll blits put
back the overflow they would otherwise erase.
- With backgrounds no longer able to clobber overflow, the narrowed
background-fill condition from #9288 goes back to what #8887 intended.
Measured with an offscreen A/B of the same scene: 15.0-15.2ms per frame
against 16.5-16.9ms before, on both the full-repaint and the scrolling
path (ASan build, so treat the absolute numbers as relative only).
#### Motivation for adding to Mudlet
#9288 tried to fix this by letting the overflow pixel survive into the
next line's cell, but four separate things still erase it: the next
line's background fill whenever the colour differs (coloured text,
selection, search highlight, caret, background image, alpha), the
partial-repaint clear, the bottom edge of the screen pixmap, and the
scroll blit. That is why the reporter sees different `print`-style
functions behave differently at the same size.
#### Other info (issues closed, discussion etc)
Fixes#9719. Completes #9070, which #9288 only partly addressed.
`GlyphOverflowTest` renders a real console offscreen across two bundled
fonts and 22 font sizes and compares the underscore's ink against the
same glyph drawn on its own. All five of its cases fail on `development`
and pass here: the line below carrying a default, coloured, bright or
selected background; the bottom visible line; a partial repaint; a
scroll-back; and a miniconsole.
Copy-as-image is fixed too: its pixmap is exactly one cell per selected
line, so the bottom line's ink was cut off every time.
Known limitations, both unchanged from before this PR:
- The topmost visible line's ink can overflow above the pixmap and be
clipped. Fixing it would mean shifting the whole screen-pixmap
coordinate system.
- When a pane's height is an exact multiple of the line height there is
no leftover strip below the last line, so the bottom line's overflow has
no pixel to live in. Measured at roughly 1 pane height in every 22 for
both the main console and the split-screen lower pane. The only fix is
to drop a row when there is no slack, and because rows are quantised
that costs a whole line of text plus a blank line-height strip at the
same ~4.5% of heights, which is a worse trade than the pixel it buys.
Resizing the pane by one pixel restores it.
Test case: `ctest -R GlyphOverflowTest`, or set Bitstream Vera Sans Mono
at 14pt and `cecho("<yellow>plain _underscore_\n<white:blue>coloured
line\n")`.
Assisted-by: Claude:claude-opus-5
https://github.com/user-attachments/assets/cbda2288-1e3e-4f24-9763-7275d2f09134
#### Brief overview of PR changes/additions
- An empty `$XDG_CONFIG_HOME/mudlet` silently beat a populated
`~/.config/mudlet`, so a stray `mkdir` hid every profile and Mudlet ran
its first-launch onboarding as though the user were new. It also stuck:
the first such launch wrote `Mudlet.ini` into that directory, which then
kept it winning.
- The two candidate roots are now ranked (`profiles/` > `Mudlet.ini` >
exists > absent) and the stronger claim wins, with
`$XDG_CONFIG_HOME/mudlet` taking ties so a fresh install and a
deliberate opt-in both still land there. A directory that cannot be
listed counts as populated rather than empty, so a permission bit cannot
re-enter the bug.
- Creating `profiles/` is now the opt-in a test harness uses; the
`mudlet` directory alone is not, because other tooling creates that by
accident. Where both roots hold profiles, `setupConfig()` names the one
it is ignoring instead of leaving those profiles apparently gone.
#### Motivation for adding to Mudlet
Data-loss-shaped regression from #9552 "improve: honor XDG_CONFIG_HOME
for Mudlet's config directory" (`e6c268cb0`). The profiles are orphaned
rather than destroyed, but a returning user sees "5.0 wiped my
profiles". `src/mudlet-lua/tests/README.md` itself instructed `mkdir -p
"$CONFIG_DIR/mudlet"`, so following Mudlet's own test docs triggered it.
#### Other info (issues closed, discussion etc)
Test case: create `~/.config/mudlet/profiles/{AlphaGame,BetaGame}`,
`mkdir -p $XDG_CONFIG_HOME/mudlet`, launch. Before: no profiles and the
onboarding dialog. After: both profiles listed.
`ConfigDirOverrideTest` covers the resolution table including the sticky
`Mudlet.ini` state, both-populated, symlinked and unreadable
directories; each new guard was mutation-checked. The busted suite
passes 2422/0 against an isolated `$XDG_CONFIG_HOME/mudlet/profiles`
root.
Not fixed here, and pre-existing rather than 5.0 regressions:
`CredentialManager` stores passwords and the OAuth reconnect token under
`AppConfigLocation` while the config root is `confPath`, so exporting
`XDG_CONFIG_HOME` strands them, and the plaintext-password migration
reads one path, writes the other and deletes the original. Both
reproduce identically on the 4.22.0 binary and need their own migration
path.
Assisted-by: Claude:claude-opus-5
#### Brief overview of PR changes/additions
- MXP `<FRAME>` windows were positioned against the whole main window,
so an edge-aligned frame landed on top of space a package had reserved
with `setBorderRight()` and friends. They now lay out inside the area
the user borders leave.
- Frames are repositioned when those borders or the window size change,
instead of staying where they were first put.
- Frames in a window of their own (`EXTERNAL`) are left alone by that
repositioning.
#### Motivation for adding to Mudlet
With the base UI installed, a game using MXP frames drew its frames on
top of the UI panel instead of beside it.
#### Other info (issues closed, discussion etc)
Fixes#9698
New functional test `MxpFramePlacementTest` (16 cases) covers each edge,
border changes, window resizes, nested frames, external frames, and a
Geyser `Adjustable.Container` attached to the right border, which is how
the base UI reserves its space. 8 of the cases fail on `development`.
**Test case:** With the base UI installed, connect to
`eden-test.rpgframework.de 4000` - the MXP frames sit beside the UI
panel rather than under it.
Assisted-by: Claude:claude-opus-5
#### Brief overview of PR changes/additions
- Hoists the list `slot_deleteProfile()` checks a never-played profile
against out of the function to
`dlgConnectionProfiles::scmConnectionDetailFiles`, beside the file's
other `scm*` constants, and points at it from both `writeProfileData()`
implementations.
- Adds a `ProfileDeletionSafetyTest` case that sets a profile up through
the real dialog (New profile, name it, fill the connection form in,
re-select it) and fails if anything it wrote is missing from the list.
- Pins the deliberate exclusions too: a profile holding a stored
password or a typed-in character name still asks before removal.
#### Motivation for adding to Mudlet
Nothing linked that list to the ~15 places profile data gets written, so
it could silently go stale; because it is an allowlist a stale entry
only ever costs an extra confirmation prompt, but the maintenance trap
was worth closing.
#### Other info (issues closed, discussion etc)
Follows up
https://github.com/Mudlet/Mudlet/pull/9722#discussion_r3740581754 on
#9722 (fix: a profile named "." or ".." deletes every profile when
removed). No behaviour change.
**Test case:** `ctest -R ProfileDeletionSafetyTest` (20 cases). Removing
an entry from the constant makes it fail naming the file; adding `login`
makes the character-name case fail.
Assisted-by: Claude:claude-opus-5
#### Brief overview of PR changes/additions
- Chat capture ships as a permanent trigger tree in the base UI package
instead of triggers created at runtime, so it is visible and copyable in
the editor: one root folder, three cheap substring gates (tells / speech
/ channel tags), each gating its Perl regex shapes as children.
- Lifecycle moves from `tempRegexTrigger`/`killTrigger` to
`enableTrigger`/`disableTrigger` on the three gates, with a
`BaseUI.chatTriggersArmed()` probe replacing the id bookkeeping.
- Vitals capture is unchanged and stays Lua-armed: its prefilter is
machine-built from the label tables, so an XML copy would drift from
what `parseVitalsLine` reads.
#### Motivation for adding to Mudlet
The gate pattern - a cheap substring parent chaining to regex children -
is the thing new scripters most need to learn, and shipping it as a
readable tree teaches it where invisible runtime triggers could not.
#### Other info (issues closed, discussion etc)
A non-matching line now costs ~17 substring scans and zero regex work.
Tightening the speech gate's literals fixed real leakage in the process:
the stems `say`/`ask`/`yell` had been letting "essay", "task", "asked"
and "yellow" through to the regex layer.
New tests: the tree's leaf shapes are asserted equal to
`BaseUI.chatShapeRegexes()`, gates are asserted substring-only, and
every gate literal must have a corpus line - the enforcer for the gate
contract, since a case-mismatched gate would otherwise drop lines
silently. Each was verified to fail by mutating the shipped package.
**Test case:** Start a new profile, connect to a game with tells and
channels, and confirm chat lines still sort into their tabs; open the
Triggers editor and confirm `mudlet-base-ui` -> `Mudlet base UI chat
capture` shows three gates with regex children. `ctest -R
StarterUiTriggerCost` covers both.
Assisted-by: Claude:claude-opus-5
<!-- Keep the title short & concise so anyone non-technical can
understand it,
the title appears in PTB changelogs -->
#### Brief overview of PR changes/additions
Don't offer updates that don't yet have a sha256 sum
#### Motivation for adding to Mudlet
They are incomplete anyhow.
#### Other info (issues closed, discussion etc)
#### Brief overview of PR changes/additions
- The starter UI armed **77 always-active PCRE triggers** (12 chat + 65
vitals) at package load, so every line a game sent was matched against
all of them - and every line one matched was then re-walked in Lua with
all 77 patterns **recompiled from source**, because `rex.match` given a
pattern string compiles it afresh on every call. They are now fronted by
4 triggers (3 chat-routing groups + 1 vitals prefilter) and compiled
once. The 65 vitals shapes and 12 chat shapes are byte-identical and
still do all the reading.
- The plain-text vitals layer now retires itself once GMCP or MSDP holds
the source lock, since `applyVitals` discards its readings from that
point anyway, and re-arms on disconnect.
- `PipelineBenchmark` created its profile through the production
new-profile path, so the starter UI was **inside** the
`text_lines_per_sec` baseline backing the "no more than 10% throughput
loss" gate for #9011 - the guard built to catch this class of regression
could not see it. Pipeline metrics now come from a profile with default
packages suppressed; the shipped configuration is reported separately as
`defaults_*` and gated in its own right.
#### Motivation for adding to Mudlet
Every new 5.0 profile was paying roughly half its text throughput to a
default package, and the perf guard had the cost baked into its own
baseline so nothing flagged it.
#### Other info (issues closed, discussion etc)
Findings C17 and C18 of the 5.0 QA sweep. Bisected there to `69cd06b1c`
- "add: starter interface with health bars, map and chat for new
players" (#9454); the benchmark half is the interaction of that with
`7d67d4bfb` - "infrastructure: perf baseline" (#9509).
Measured on a quiet 16-core box, Release, no ASan, alternating paired
runs so drift is shared between arms:
| workload | before | after | |
| --- | --- | --- | --- |
| `TelnetBenchmark` `benchLargeData`, 1000 lines that match nothing |
22.25 ms `[22.1-22.6]` | 12.0 ms `[11.9-12.2]` | **1.85x** |
| `PipelineBenchmark`, 25k lines of realistic game output, new-user
profile | 9,998 lines/s `[9,856-10,072]` | 16,503 lines/s
`[16,257-16,632]` | **1.65x** |
Complete separation in both (21 and 9 pairs; within-arm spread ±1.7% and
±1.5%, so ~3% is the smallest effect distinguishable from noise - the
effect is 85% and 65%). The bare pipeline measures 116,000 lines/s, so
the starter UI's remaining cost on that corpus is 7.0x, down from 11.6x;
the residual is the capture layer doing its designed work on a corpus
where 1 line in 11 is a tell and another 1 in 11 a vitals prompt.
Two notes for reviewers:
- `config.lua` is bumped to 1.1.0, so mpkg offers the update - but
default packages are installed at profile creation, so **profiles
already created on a 5.0 PTB keep the old copy** until they update it.
- Touches `src/mudlet.cpp` / `src/mudlet.h` /
`test/functional_tests/CMakeLists.txt`, which #9695 also touches; the
CMakeLists hunk will likely conflict trivially (both append a test
file).
**Test case:** create a fresh profile against any game without GMCP,
confirm the health/mana gauges and chat tabs still appear from prompt
and chat lines, then `ctest -R StarterUiTriggerCostTest`.
Assisted-by: Claude:claude-opus-5
#### Brief overview of PR changes/additions
- Hardens how OSC 8 link payloads and link text are handled before they
are run or displayed; link commands are no longer built by
string-formatting remote text into Lua source.
- Adds a per-profile setting (General → Game protocols) to turn OSC 8
hyperlinks off, which also reports `0` for every `OSC_HYPERLINKS*`
NEW-ENVIRON variable and sends an INFO update if toggled mid-session.
- Fixes `selected=` callbacks on `send:` links, which never fired, and
keeps emoji and Persian/Arabic/Indic text intact in tooltips and menu
labels.
#### Motivation for adding to Mudlet
Inspired by
[conversation](https://discord.com/channels/279748146316312576/1416447642472284261/1535109010066251890)
on the MUD Discord and updates to terminal emulators.
OSC 8 sequences arrive from the game server — and often from another
player whose say/tell text the server relays — so they have to be
treated as untrusted input rather than as content the user chose to
load.
#### Other info (issues closed, discussion etc)
New unit tests: `LuaLiteralTest` (28 cases, including an exhaustive
sweep over the bracket alphabet, each evaluated in a real Lua 5.1 state)
and `UntrustedTextTest` (26 cases covering emoji sequences, non-Latin
shaping and the two sanitization policies). There is no automated
NEW-ENVIRON coverage anywhere in the repo, so that path was verified
manually against a live server instead.
**Test case:**
1. `say !osc8-docs` — every documented feature still works.
2. Send a link whose command ends in `]`, e.g. `send:say [OOC]` —
clicking sends the literal text (previously the click silently did
nothing).
3. Settings → General → Game protocols → uncheck "Enable OSC 8
hyperlinks from the server" — links stop rendering and the server is
told without a reconnect; re-check and they return.
4. Send a tooltip or menu label containing a multi-part emoji such as
👨🍳 — it renders normally, not as its component parts.
---------
Signed-off-by: Michael Conley <sousesider@gmail.com>
#### Brief overview of PR changes/additions
- Validation: a typed profile name must be a folder of its own - rejects
a lone `.` and anything containing `..`. Folders already on disk stay
exempt
- Containment: `reallyDeleteProfile()` refuses any path that is not a
direct child of `profiles/`, and now checks `removeRecursively()`
instead of failing mute
- Confirmation: the "nothing to delete" shortcut no longer fires when a
map, stored password or dictionary is present
#### Motivation for adding to Mudlet
A profile named `.` or `..` turned **Remove** into a wipe: `.` resolves
to `profiles/`, `..` to the whole `~/.config/mudlet`. The name was
accepted with no error, and the confirmation was skipped because a fresh
profile looks empty - two clicks deep on the first screen every user
sees.
#### Other info (issues closed, discussion etc)
Pre-existing, not a 5.0 regression - shipped 4.22.0 behaves identically.
Dots have been allowed deliberately since 2011 (`ee1fd051c`), and
`Achaea 2.0` keeps working.
**Test case:** name a new profile `.` and press Remove - previously
every profile was deleted with no prompt, now the name is refused.
New `ProfileDeletionSafetyTest` drives the real dialog against a
temporary config dir, plus `profileFolderPath`/`profileNameUsableAsIs`
rows in `ProfileNameValidationTest`. 81/81 ctest pass.
Assisted-by: Claude:claude-opus-5
#### Brief overview of PR changes/additions
- One budget of 100 covered every root trigger created while a line was
processed, and tripping it deactivated all of them, so a script arming
101 unrelated triggers lost all 101.
- Triggers created mid-line now carry the creation lineage they belong
to and how many generations deep they sit in it. A batch is one
generation however big it is; only a trigger that re-creates itself
keeps adding generations, so that is the only shape the budget counts.
The limit is 1000 generations, and only the runaway lineage is stopped
and named.
- Generations do not bound a lineage that widens as it deepens, so past
20000 creations on one line new triggers stop being offered that line.
Nothing is disowned there - they are all still armed for the lines that
follow.
#### Motivation for adding to Mudlet
#9697 fixed a real freeze, but its counter had no lineage, so it
destroyed legitimate triggers along with the runaway. Any routine arming
more than 100 triggers from a trigger loses them, permanent ones
included, which is a regression against 4.22.0.
#### Other info (issues closed, discussion etc)
Fixes a regression introduced by merged #9697; release-blocking for 5.0.
Test case: `lua fired=0; tempTrigger("GATE", function() for i=1,200 do
tempTrigger("PAY", function() fired=fired+1 end) end end);
feedTriggers("\nGATE\n"); feedTriggers("\nPAY\n"); print(fired)` -
prints 0 before, 200 after.
A/B against a shipped 4.22.0 binary (4.22.0 / 5.0 RC / this PR), counted
on the line after the one that armed them: 1001 unrelated temp triggers
1001 / 0 / 1001; two scripts of 600 each 600+600 / 0+0 / 600+600; 1001
permanent triggers 1001 / 0 / 1001. #9697's freeze is still stopped and
bounded (601 runaway lines in 8 s, RSS flat at ~820 MB) and its own
tests still pass. Eleven new tests parameterise the creation count, and
cover nested passes and folder/filter-chain children, which none of
#9697's did - that is why this shipped.
Assisted-by: Claude:claude-opus-5
#### Brief overview of PR changes/additions
- Every CI job on development has been red since 6c67a138 (#9704):
`PackageSelfUninstallTest.cpp:627` calls
`XMLexport::cloneExportDocument()`, which fd822ebb (#9699) had renamed
to `takeExportDocument()` four hours earlier. #9704's checks were green
against the older base and were not re-run before merge, so the
collision only appeared once both were on development.
- The call site is a local `XMLexport` used once and destroyed
immediately after, so handing the document over instead of copying it is
equivalent here.
#### Test case
`ctest -R PackageSelfUninstall` passes, and a full build of all targets
is clean.
Assisted-by: Claude:claude-opus-5
#### Brief overview of PR changes/additions
- `getWindowGeometry("main")` and `windowVisible("main")` (and the `""`
spelling of the same) now answer instead of returning `nil, 'window
"main" not found'`; geometry is `0, 0` plus whatever
`getMainWindowSize()` reports, so the two functions cannot disagree
- `windowVisible()` reads `isVisibleTo(mpConsole)` rather than
`isVisible()`, so a profile that is not the front tab - whose whole
console Mudlet hides - stops reporting every one of its labels,
miniconsoles, scroll boxes, command lines and text edits as invisible. A
child of a hidden user window still reports `false`, which is the
documented behaviour
- New two-profile `WindowStateGettersTest` (the busted suite is always
the single front profile, so it cannot reach this), and the two
`UI_spec` specs that asserted the old refusal are replaced
#### Motivation for adding to Mudlet
Both getters are new in 5.0, and as shipped the first one tells a script
the main window does not exist while the second answers wrong for every
profile the user is not currently looking at.
#### Other info (issues closed, discussion etc)
5.0 QA findings C6 (main rejected) and D10 F1 (background profiles). The
background-profile half is the same defect `7bb20fa2c` ("add: widget
state getters for titles, stylesheets, tooltips and scroll bars
(#9645)") fixed for `getScrollBarVisible`; `windowVisible` landed a week
earlier in `1227bc377` ("add: getWindowGeometry(), windowVisible() and
getLabelText() functions (#9528)") and was left reading the widget.
**On excluding `main` - the counter-argument, weighed.** #9528 made that
choice deliberately: it shipped two `UI_spec` specs asserting the
refusal, with the comment "mirrors moveWindow/resizeWindow, which
likewise do not act on main", and the Area 51 draft says the same. So
this overturns a decision rather than filling an oversight. I still
think it is wrong: the message claims a window that manifestly exists
was *not found*, whereas `moveWindow("main", ...)` is a silent no-op and
claims nothing; `windowType("main")` in the same readback family
answers; and `isMain()`, `getRowCount`, `getColumnCount`,
`getWindowWrap` and `getScrollBarVisible` all take `"main"`/`""`.
Refusing is only defensible when there is no sensible answer, and there
is one. The Area 51 text for both functions needs the matching edit
before it goes to the manual.
Two things deliberately left alone, reported rather than fixed:
`Host::windowType()` still special-cases `"main"` without `""`, and a
user element literally named `"main"` is now shadowed by the main
window.
Assisted-by: Claude:claude-opus-5
**Test case:** `print(getWindowGeometry("main"))` and
`print(windowVisible("main"))` answer; with two profiles open,
`createLabel("probe", 0, 0, 50, 50, 1)` in profile A then
`windowVisible("probe")` from a timer while profile B is in front still
returns `true`.
#### Brief overview of PR changes/additions
- `lowerWindow()` moved the main display below the full-window
background widget, so with a background set the whole console (text,
split, scrollbar, command line) vanished for the rest of the session; it
now keeps the background bottom-most.
- `setBorderColor` lived only in `mpMainFrame`'s palette, which
`changeColors()` rebuilds from constants - a game sending an OSC palette
change wiped it with no user action at all - and `getBorderColor()`
returned `0,0,0` under a full-window background. The colour is now
stored on the console.
- `cover` mode scaled the whole source before cropping, so a 3000x100
image in a 1920x1080 window built a 32400x1080 (~140MB) intermediate on
every resize event; it now crops to the target aspect first. Measured
133.5MB/15ms to 0.1MB/3ms, and 890MB for a 20000x100 source.
#### Motivation for adding to Mudlet
All three are in the 5.0 full-window background feature and the first
one makes an ordinary `lowerWindow()` call blank the entire console.
#### Other info (issues closed, discussion etc)
Finding C10 of the 5.0 QA sweep. Introduced by `ae6b017c8` "add:
full-window background image/gradient support" (#9394).
`raiseWindow("main")` and `lowerWindow("main")` both return false today
(`"main"` is never registered in any of the window maps), so there was
no way to undo the first bug from a script - verified, not changed here.
New `WindowBackgroundTest` (17 cases). Five of its assertions were
confirmed to fail before the fix and pass after; the crop-then-scale
order is pinned by comparing the installed brush against a crop-first
render, which a scale-first implementation fails.
Assisted-by: Claude:claude-opus-5
**Test case:** `setBackgroundImage("main",
getMudletHomeDir().."/bg.png", "cover", true)` then `createLabel("l",
10, 10, 100, 100, 1)` then `lowerWindow("l")` - the console stays
visible.
#### Brief overview of PR changes/additions
- Variables: the export skipped its refresh whenever the editor's
Variables view was on screen, so anything a script wrote into a saved
variable while it sat open was dropped from the save - including the
session's last save, which is taken with whatever view the editor was
left on. The variables are now read into a throwaway tree, which also
stops a save stranding the editor's variable search results.
- Packages: a save taken while a unit was still executing an item of a
package that had just been uninstalled wrote that package's items back
into the profile, where they returned as orphans the Package Manager
could not remove. The XML writers now skip what the units have queued
for a deferred delete, the module writer included - reloading a module
from a script used to write both the pre- and post-reload copies of its
items into the module file.
- `LuaInterface::getVars()` is now `setjmp`-guarded like every other
Lua-touching method there, so a panic cannot jump past the export's
scope with its variable tree and registry references still held.
#### Motivation for adding to Mudlet
Both are silent data loss in everyday use: quitting with the editor on
the Variables tab, and the `mpkg`/auto-updater shape of uninstalling a
package from a script.
#### Other info (issues closed, discussion etc)
From the 5.0 QA sweep, findings C13 and C14. The variables half re-opens
the loss that `20009c5ec` "fix: variables added while playing are no
longer lost when saving (#9492)" fixed, via the guard it added; the
packages half is the missing counterpart to the self-uninstall deferral
in `276e8bbfd` (#9383) and its follow-ups. #9492's own cases still pass
unchanged.
**Test case:** create a table from the command line, tick it to be saved
in the editor's Variables view, leave the editor there, run `lua
myTable.later = "x"`, quit and reopen - `later` is still there. `ctest
-R 'XMLexportVariablesTest|PackageSelfUninstallTest'` covers both
halves; all 12 new cases were verified to fail against the unfixed
source.
Assisted-by: Claude:claude-opus-5
#### Brief overview of PR changes/additions
- **Right-clicking the games list with nothing selected killed Mudlet.**
`dlgConnectionProfiles::slot_profileContextMenu()` dereferenced
`currentItem()` unguarded. That line is byte-identical in 4.22.0, so the
null deref itself is long-standing and latent - what is new is that it
became reachable: "improve: split the games list into My games and All
games tabs" (#9452) leaves a user with no saved profiles an empty but
still right-clickable "My games" tab, a state 4.22.0's always-populated
list never had. About 40 seconds into a fresh install.
- **Copying a profile while the list was rebuilt was a use-after-free.**
The copy runs on a thread pool and its completion handler kept the
`QListWidgetItem*` it had made; clicking the other games tab meanwhile
calls `fillout_form()`, which destroys every item. The handler now finds
the copy by name, and the `QFutureWatcher` is parented so it cannot
outlive the dialog.
- **Quitting before the connection dialog had been shown dereferenced
null.** The queued `0ms` lambda in `mudlet::slot_showConnectionDialog()`
used `mpConnectionDialog`, which `mudlet::closeEvent()` closes (it is
`WA_DeleteOnClose`) and clears.
#### Motivation for adding to Mudlet
All three came out of the 5.0 QA sweep and are confirmed with
AddressSanitizer. The first is the serious one - it is the default state
of a brand-new install, so a new user can lose Mudlet before they have
connected to anything.
Scope note on the third: it is **not** a 5.0 regression. It has been
there since "Fix: Improve tab indicators and detached window UX" (#7965)
and is unchanged in 4.22.0; #9493 only turned the literal `0` into
`0ms`. Nor could I reach it by clicking: I drove *Games -> Close
profile* followed by quitting at six delays from 0 to 2000 ms and the
dialog was always painted first. It reproduces deterministically
in-process, and QA reproduced it 2/2 driving the close from Lua. Worth
guarding - the pointer is documented to go null - but latent rather than
routinely hit.
#### Other info (issues closed, discussion etc)
Test case: `ctest -R ConnectionDialogCrashTest` - with the fix reverted,
four of its tests reproduce the original ASan reports exactly (two SEGVs
in `slot_profileContextMenu`, a heap-use-after-free in
`slot_itemClicked`, the SEGV in `QWidget::show()` from the lambda); two
more are controls that pass either way, one of them pinning that the
menu still opens for a selected profile so the guard cannot degenerate
into an unconditional early return. Full suite 79/79.
Assisted-by: Claude:claude-opus-5
#### Brief overview of PR changes/additions
Four security fixes to the GMCP `Char.Login` v2 browser sign-in added in
#9378 *Add: sign in to supported games using your browser (e.g. Google,
Discord, or the game's own account)*, found by the 5.0 QA sweep and
reproduced on the wire. The game server is untrusted throughout: Mudlet
connects to arbitrary user-specified MUDs.
- **A saved sign-in no longer goes out in the clear.**
`Char.Login.Reconnect` carries a bearer token that signs into the
player's account without their password, and Mudlet replayed it on
whatever transport was live at the time - so a sign-in earned over TLS
went out over plain telnet on the next connect. It is now refused on a
cleartext transport, mirroring the `Char.Login.AuthCode` guard already
in the same file; the player is told why and the sign-in falls back to
the provider resume or the game's own sign-in screen.
- **A server can no longer open browser tabs at will.** The
client-driven OAuth path reached `QDesktopServices::openUrl()` with a
server-chosen address and no guard at all, once per frame the server
sent (5 frames measured, 5 tabs). Both flows now go through one decision
point with a budget of one automatic hand-off per connection, refilled
whenever the player sends something to the game. The client-driven flow
still opens the browser on connect - that is the sign-in the player came
for - but a burst of frames buys one tab, not a tab each, and
`Char.Login.URL`, which a server may push at any moment, still needs
actual input first.
- **The OIDC nonce reaches the game, and a `Char.Login.Default` flood no
longer buys credential-store churn.** With `"nonce": true` Mudlet
generated a nonce and put it in the authorization URL but never sent it
on, so the party that actually validates the ID token could not check
its `nonce` claim; it now rides in `Char.Login.AuthCode`. Separately
each `Char.Login.Default` frame started a fresh credential-store read (a
114 KB flood measured 4002 reads); sign-in attempts are now throttled to
one per second, and dropped outright if the connection that scheduled
them has gone.
#### Motivation for adding to Mudlet
All four are new in 5.0 and none was covered by a test, which is why
they shipped. The token replay is the serious one: it hands a
password-equivalent account credential to anyone on the path.
Two decisions worth a second opinion:
- The Char.Login 2 draft on Area 51 defines no `nonce` field in
`Char.Login.AuthCode`, so this adds one. Without it `"nonce": true`
cannot mean anything - nobody is in a position to verify the value.
**The spec needs the field added to match.**
- The same draft explicitly permits sending the reconnect token over
plain telnet ("a server may choose to issue and accept tokens only over
`telnets://`"). It does not require a client to, so refusing is
conformant, but it is deliberately stricter than the spec.
#### Other info (issues closed, discussion etc)
Test case: connect a profile with a saved sign-in to a game offering
`Char.Login 2` over plain telnet - Mudlet says the connection is not
encrypted and hands off to the game's own sign-in screen instead of
replaying the token.
`GMCPCharLoginTest` gains a TLS-capable stub (embedded self-signed
loopback certificate) and a loopback OpenID discovery stub, so the
encrypted-transport and client-driven paths are exercised for real
rather than assumed. The seven existing token tests move onto it, which
is itself the proof that they were previously passing over cleartext.
80/80 `ctest` green.
Assisted-by: Claude:claude-opus-5
#### Brief overview of PR changes/additions
- Ending a track is now done before its player is handed to the next
one, so pausing a sound (or music) and playing a *different* file plays
it, and reports the paused track's own ending rather than a spurious
`sysMediaFinished` carrying the new request's key and tag.
- A `play()` call now owns the player it is setting up, so a
`sysMediaFinished` handler that starts media of its own can no longer
take that player over and make the caller's higher-priority request
disappear.
- `purgeMediaCache()` returns `nil, message` instead of a bare `false`,
a media URL whose scheme is not http(s) now raises `sysDownloadError`
instead of being refused silently, and the last pass of a `loops = N`
track can no longer have its ending swallowed.
#### Motivation for adding to Mudlet
"Pause the ambience, start the combat theme" is an ordinary GMCP/script
sequence and in 5.0 it produced silence plus a misleading event.
#### Other info (issues closed, discussion etc)
Found by the 5.0 QA sweep (finding C16). The three commits behind it
were each reviewed alone and share one state machine: `92f01b850` "fix:
Client.Media loops=-1 plays once (#9569)", `3474cb58d` "fix: playing a
sound again right after stopping it (#9611)" and `8dd99e4db` "fix: media
that fails to load never reports it (#9612)".
Pause `a.wav`, then play `b.wav`, on `origin/development` and on this
branch:
```
before sysMediaFinished file=a.wav key=k2 tag=t2 <- old file, new request's key
(nothing playing, the paused track is gone too)
after sysMediaFinished file=a.wav key=k1 tag=t1
sysMediaStarted file=b.wav key=k2 tag=t2 <- b.wav plays
```
Contract change worth a changelog line: `purgeMediaCache()` returned
`false` on failure and now returns `nil` plus a message. `if not
purgeMediaCache()` still behaves the same; `== false` no longer matches.
Adjacent and deliberately left alone: `stopSounds{fadeaway = true}`
issued while a track is still loading is lost, and the track then plays
forever. That is byte-identical in 4.22.0, so it is not a 5.0
regression. `downloadFile()`'s five other refusals (path traversal,
three directory-creation failures, an invalid URL) are still silent,
also pre-existing.
**Test case:** `playSoundFile{name = "a.wav", key = "k1"}`,
`pauseSounds()`, then `playSoundFile{name = "b.wav", key = "k2"}` -
b.wav plays and exactly one `sysMediaFinished` arrives, naming a.wav and
k1. Seven new specs in `Media_spec.lua` (six of them fail against
`origin/development`) and one new case in `TMediaLoopTest` cover this,
pause-then-play for music, a refused-on-priority request, re-entrant
`play()`, `loops = 2`, and both failure contracts.
Assisted-by: Claude:claude-opus-5
#### Brief overview of PR changes/additions
- `createMapper()` raises `mapOpenEvent` again when the profile's map is
already loaded, and repopulates/reselects the mapper's area dropdown on
that path
- The redundant second `map->restore()` that `2a3334a6a` ("Fix: don't
load a map if trying to create a mapper & map is already loaded
(#9415)") was written to skip is still skipped - only the event raise
and the combo box setup move back out of its guard, matching the shape
`Host::createMapper()` has always had
- New `EmbeddedMapperCreationTest` functional test covering both sides
of the branch; the busted suite structurally cannot, since an embedded
mapper and the dockable map widget are mutually exclusive for the life
of a profile
#### Motivation for adding to Mudlet
Every returning user has a saved map, so `createMapper()` and
`Geyser.Mapper{embedded = true}` hit exactly this path, and third-party
mapping scripts that finish setting themselves up on `mapOpenEvent`
silently stopped running.
#### Other info (issues closed, discussion etc)
Found in the 5.0 QA sweep (finding C4), verified on two pristine
profiles: 0 rooms raises the event once, 1 room raised it zero times.
The same guard also swallowed
`updateAreaComboBox()`/`resetAreaComboBoxToPlayerRoomArea()`, which left
the area dropdown reading "Default Area" while the player room was
elsewhere.
Assisted-by: Claude:claude-opus-5
**Test case:** on a profile with a saved map, run
`registerAnonymousEventHandler("mapOpenEvent", function()
echo("\nmapOpenEvent fired\n") end) createMapper(0, 0, 400, 400)` - the
echo appears, and the mapper's area dropdown shows the player's area.
#### Brief overview of PR changes/additions
- Adds `CI/check-release-tag.sh`: APP_VERSION must be three-component,
and a release tag must be exactly `Mudlet-<APP_VERSION>`.
- Wires it into the tag-build validation (`CI/validate_deployment.sh`,
`CI/validate-deployment-for-windows.sh`) so a bad tag fails minutes
after the push, before an asset exists, and into
`create-github-release.yml` as the last gate before anything is
published. The PTB path gets the version-shape half, which nothing
checks on `development` today.
- Covers it with `test/ci/release-tag-version-test.sh`, registered as
`ReleaseTagVersionTest`.
#### Motivation for adding to Mudlet
Tagging `Mudlet-5.0` instead of `Mudlet-5.0.0` would strand the entire
4.22.0 user base with no error anywhere, and it is the one version
mistake CI does not currently catch.
The updater takes the version it offers from the tag, not the binary:
`Release::Release()` strips the `Mudlet-` prefix
(`src/updater/Release.cpp:49`) and `SemVer::getRegExp()` needs three
components (`src/updater/SemVer.cpp:111`), so `"5.0"` is invalid,
`Release::operator<` (`src/updater/Release.cpp:96`) reports the release
as not newer, and `Feed::getUpdates()` returns nothing. The update check
goes on logging `0 update(s) available` - the same line as a week with
no release.
The asymmetry is what makes it dangerous. A stale APP_VERSION with a
correct tag fails loudly, because `CI/prepare-release-assets.sh:62`
rejects assets by tag prefix. A short tag with a correct APP_VERSION
passes everything, because `Mudlet-5.0.0-linux-x64.AppImage.tar`
genuinely does start with `Mudlet-5.0`.
**Why the build scripts and not only the workflow:**
`create-github-release.yml` is `workflow_run`-triggered, so it cannot
fail before the assets are built - by the time it runs, the full matrix
has already finished. The validate scripts run at the start of every tag
build on all three platforms and already parse APP_VERSION, so that is
where the fast failure belongs. The workflow keeps a copy because it
always runs from the default branch, so it still guards a tag placed on
a commit that predates this change.
APP_VERSION is deliberately left at 4.22.0 - bumping it is a release
decision, not a QA fix. This guard is what catches a mismatch when the
bump happens.
#### Other info (issues closed, discussion etc)
From the 5.0 release QA sweep, finding C1, "A two-component release tag
silently disables auto-update for every existing user". Pre-existing
mechanism, no single commit introduced it.
Three claims from an earlier draft did not survive checking and were
corrected: `src/sparkleupdater.mm` installs no
`versionComparatorForUpdater:`, so Sparkle's default component-wise
comparator would still offer `5.0` over `4.22.0` (macOS breaks on the
opposite mismatch instead); the update check does log, it is just
indistinguishable from having nothing to offer; and SemVer does accept a
prerelease component, so rejecting `Mudlet-5.0.0-rc1` follows from
APP_VERSION being unable to carry a suffix, not from the updater.
No video - a CI guard is not visually observable. The shell output below
is the evidence instead.
**Test case:** `ctest -R ReleaseTagVersionTest`, and the guard run
directly:
```
$ CI/check-release-tag.sh 5.0.0 Mudlet-5.0.0
Release tag 'Mudlet-5.0.0' matches APP_VERSION '5.0.0'.
exit=0
$ CI/check-release-tag.sh 5.0.0 Mudlet-5.0
error: release tag 'Mudlet-5.0' does not match APP_VERSION '5.0.0'.
The tag has to be exactly 'Mudlet-5.0.0'.
Publishing under a mismatched tag breaks auto-update, without saying so. [...]
exit=1
```
Replayed over every release tag since 4.18.5, each against the
APP_VERSION at that tag - all accepted, so the guard blocks nothing
Mudlet has actually shipped. Executing the real `Determine release type`
step under GitHub's shell flags fails on `Mudlet-5.0` + `5.0.0` and on a
PTB with APP_VERSION `5.0`, and passes on `Mudlet-5.0.0` + `5.0.0` and
on a normal PTB. The updater trace was confirmed by compiling
`Release.cpp` + `SemVer.cpp` and comparing: tag `Mudlet-5.0.0` gives
`(4.22.0 < release) = true`, tag `Mudlet-5.0` gives `false`.
Assisted-by: Claude:claude-opus-5
#### Brief overview of PR changes/additions
- 43 of 45 functional tests now run with `detect_leaks=1` and fail CI if
they leak (the suite leaked 11.83MB before this PR). The LSan
suppression hooks move into an OBJECT library so they actually bind into
the test binaries.
- Fixes the three production leaks behind nearly all of it: the
unparented `QSettings` (now application-parented, since the Updater
holds the pointer past window close), edbee re-initialisation orphaning
its managers on every repeat `mudlet::init()`, and the preferences
dialog's menu `QAction`s (`QMenu::addAction()` does not take ownership).
- New `EdbeeReinitTest` and a preferences-reopen test lock the fixes in.
Still excluded: `dlgTriggerEditorUndoRedoTest` (fixed in #9700) and
`UpdaterTeardownTest` (Qt's one-time CA-store load).
#### Motivation for adding to Mudlet
Leaks in code covered by the functional suite now fail the build instead
of accumulating silently, and two of the three fixes stop real leaks in
production.
#### Other info (issues closed, discussion etc)
Test case: full serial suite passes twice, 45/45; settings persistence
verified end-to-end under Xvfb. Startup unchanged (median 213ms vs
214ms); the suite's +62s is LSan's at-exit scan, no individual test
regressed.
Assisted-by: Claude:claude-fable-5
#### Brief overview of PR changes/additions
- Fixes#9670 "Mudlet stops responding on macOS part way through the
package specs": the test-only `waitForEvent()` ran a nested
`QEventLoop::exec()`, which the Cocoa dispatcher services by re-entering
`-[NSApplication run]` from inside the timer callout it is already in -
so no Qt timer, including the wait's own timeout, ever fires again.
`EventLoopPump::pumpFor()` drives `processEvents()` against a deadline
instead, which activates Qt's timers on every pass.
- Lifts the macOS pending gate the package specs have carried since they
were written: ~40 specs now run on both macOS legs, green.
- Fixes four pre-existing crashes (each reproduced on `development`
under ASan) when a profile or the application is closed from a handler
delivered during a wait: profile closes are held off while a pump runs,
application shutdowns postpone rather than cancel.
#### Motivation for adding to Mudlet
macOS was the only platform not running the package specs, and the hang
wedged real CI runs.
#### Other info (issues closed, discussion etc)
Test case: #9689's harness with this fix runs the previously hanging
suite to completion (2277/0) on both macOS runners; this PR's own legs:
2370/0 on both macOS arches, 2426/0 on Linux with ASan and leak
checking.
Assisted-by: Claude:claude-opus-5
#### Brief overview of PR changes/additions
- Closing a profile no longer frees the map out from under a running
import, export or download. `TMap` counts the operations that pump
`qApp->processEvents()`, and `mudlet::closeHost()` - which one of those
pumps is what delivers it - stops the operation and destroys the `Host`
once it has unwound, instead of half way through it.
- Discord presence fields keep their last character and are only ever
cut between characters: each buffer is now the documented limit plus
room for its terminator, and a new `utils::copyUtf8String()` walks the
cut back to a character boundary.
- An interrupting `ttsSpeak()` announces the utterance it starts, and
the `Ready` an engine reports for the utterance it cut off no longer
drains `ttsQueue()` over the top of the one the script asked for.
#### Motivation for adding to Mudlet
Each is a filed defect, and each was reproduced before it was fixed. The
map one is a use-after-free: ASan reports `heap-use-after-free` inside
`TMap::readJsonMapFile()`, freed by `~TMap` <- `~Host` <-
`HostManager::deleteHost` <- `mudlet::closeHost` delivered by the
import's own `processEvents()`. The Discord one is worse than one field
looking wrong: a single over-long non-ASCII field makes the whole
`SET_ACTIVITY` payload undecodable, so the entire presence update is
discarded - the fake Discord client recorded exactly that. The TTS one
silently drops speech: `ttsQueue()` plus an interrupting `ttsSpeak()`
speaks the queued line and never speaks the requested one.
#### Other info (issues closed, discussion etc)
Closes#9520, closes#9634, closes#9659.
`MapCloseDuringImportTest` stages the close through
`mudlet::slot_closeProfileByName()` and lets the map operation's own
pump deliver it; the functional tests build with ASan, so the pre-fix
run is a sanitizer report rather than an inference.
`TtsInterruptingSpeakTest` hands `ttsStateChanged()` the `Ready` a real
engine sends, which Qt's mock engine never does - the mock-visible half
is pinned in `Media_spec.lua`, where the two specs that recorded the old
behaviour are updated. `Discord_spec.lua` gains four end-to-end specs
against `CI/discord-ipc-fixture.py` asserting that the captured frame
still decodes as JSON and that a field is cut on a character boundary,
and `DiscordTest.cpp` covers the same at unit level. Every new or
changed test was confirmed to fail without its fix.
Two things deliberately left alone, both older than this PR:
`Host::requestClose()` still runs nested inside the map operation's pump
(it saves the profile there), and an XML import or a map download has no
cancel to poll, so a close waits for it rather than stopping it.
**Test case:** Export a large map with `exportJsonMap()` and close the
profile's tab while it runs; then `setDiscordDetail(string.rep("ä",
65))` and confirm the presence still updates; then `ttsQueue("queued
line") ttsSpeak("first")` followed immediately by `ttsSpeak("second")`
and confirm "second" is what gets spoken.
Assisted-by: Claude:claude-opus-5
#### 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
#### Brief overview of PR changes/additions
- Comment-only follow-up to #9695 "fix: stop treating long-time Mudlet
users as brand new players" - no code line changes at all.
- Removes every passage describing what the gate used to do; git history
and the #9695 body carry that, and in source it goes stale the moment
someone touches the line.
- Cuts what remains to the constraints a reader cannot derive from the
code: the `init()` ordering requirement, why no filesystem timestamp can
substitute for the recorded date, and why the fallback deliberately errs
towards 'experienced'.
#### Motivation for adding to Mudlet
Applies the house comment standard to code that landed a few hours
earlier: 112 comment lines out, 33 shorter ones in.
#### Other info (issues closed, discussion etc)
Test names carry the intent in `ExperiencedPlayerGateTest`, so
per-assertion narration went; the `QVERIFY2` failure messages already
say what each case is asserting. What was kept there: the permissions
restore-before-assert ordering, the `$XDG_CONFIG_HOME/mudlet` opt-in
marker, and the note that the live-singleton case must run last because
`experiencedMudletPlayer()` memoises.
**Test case:** `ctest --output-on-failure` - 79/79, unchanged.
Assisted-by: Claude:claude-opus-5
#### Brief overview of PR changes/additions
- The profile tab switching shortcuts added in #9460 "add: keyboard
shortcuts to switch between game tabs" (`b649b60f0`) are `QShortcut`s on
the main window, and Qt's `QShortcutMap` consumes a matching key inside
`QApplication::notify` before the `KeyPress` ever reaches the command
line. `TCommandLine::handleCtrlTabChange()`'s "let user-defined Ctrl+#
keys match first" branch became unreachable, so user key bindings on
Ctrl+1..Ctrl+9, Ctrl+Tab and Ctrl+Shift+Tab silently stopped working.
- `TCommandLine::event()` now claims `QEvent::ShortcutOverride` for
exactly those key sequences when a user binding matches, which is the
same escape hatch the accessibility caret shortcut already uses. Asking
needs a non-executing query, hence `TKey`/`KeyUnit::wouldMatch()` -
`keybindingMatched()` runs the binding and would fire it on every
override probe. The match reproduces `QShortcutMap`'s own retries, so
Ctrl and a numpad digit, and Ctrl+Shift and a digit on layouts that need
Shift for the top row (French AZERTY), are covered too.
- Precedence, stated explicitly: a user binding wins over the built-in
tab switch, which is what that comment always intended. A binding that
is disabled, or sits in a disabled group, does not claim the key, and
every other application shortcut is unaffected.
#### Motivation for adding to Mudlet
Ctrl+1 to Ctrl+9 is a common combat/target hotkey range and the one
Mudlet's own key editor offers. Upgrading silently broke those bindings
with no error and no warning, and the escape hatch (clearing the
shortcut in Preferences) is undiscoverable.
#### Other info (issues closed, discussion etc)
Test case: `ctest -R ProfileSwitchShortcutTest` - 15 cases covering the
claim, the no-claim controls, disabled bindings and groups, the keypad
and shifted-digit spellings, Ctrl+Shift+Tab's `Key_Backtab` spelling, a
cleared shortcut not claiming every key, and that a claimed binding runs
exactly once. Verified to fail without the fix.
Not fixed here, reported instead: the caret-mode Ctrl+Tab toggle lives
on `Host::mCaretShortcut` rather than `ShortcutsManager`, so #9449's
shortcut clash warning still cannot see its collision with the "Next
profile" default.
Assisted-by: Claude:claude-opus-5
#### 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
#### Brief overview of PR changes/additions
- `experiencedMudletPlayer()` decided veteran status from profile
**directory** mtimes. The per-profile data writes (url, port, password,
`profile.ini`, command history) all land straight in that directory and
bump its mtime every session, so only a profile *abandoned* for six
months ever looked old - the more you use Mudlet, the more certainly it
called you new. All 13 profiles on the maintainer's machine classified
as "brand new player".
- Mudlet renewed those timestamps itself: the connection dialog rewrites
`url`/`port`/`description` for the selected profile at startup, so
merely launching Mudlet reset the value the gate read.
- Replaced with a `firstLaunchDate` key recorded in QSettings on a
genuinely fresh install (written in `init()`, before anything can create
a profile or save a setting). An installation with any trace of earlier
use - a profile, or any other setting already on file - has no
recoverable start date and is treated as experienced; timestamps cannot
recover one, since a copied or restored profile keeps its modification
times only if the tool used happened to preserve them, and loses its
birth time regardless.
#### Motivation for adding to Mudlet
In 4.22.0 this gate only suppressed three one-line hints, but 5.0 hung
the full-window "Welcome to Mudlet! New here?" tour (#9385, 69d7d4169
"Add: UI tour to complement the Mudlet tutorial") and the starter UI
package (#9454, 69cd06b1c "add: starter interface with health bars, map
and chat for new players") off it, so essentially every active 4.22.0
user upgrading to 5.0 would get a beginner tour dropped on top of their
session.
#### Other info (issues closed, discussion etc)
The heuristic dates back to ae0564e6e "Improve: revise splitscreen
tutorial (try 2)" (#7341); the two 5.0 consumers above are what turned
it into a release blocker. New `ExperiencedPlayerGateTest` (18 cases)
covers fresh install, upgrader with freshly-written profiles, settings
restored without profiles, the six-month boundary, a profile restored
from backup, future-dated, unparseable and unwritable records, and two
live-singleton cases that pin the `init()` call site and the memoised
read. Both directions were mutation-tested: dropping the `init()` call
and restoring the old mtime heuristic each fail the suite. When in doubt
the gate errs towards *experienced* - a veteran shown a new-user tour is
a much worse outcome than a newcomer who misses it.
**Test case:** Seed a HOME that looks like an existing user (one
profile, `Mudlet.ini` without `uiTourShown`), launch Mudlet and connect
- before, the 6-step "Welcome to Mudlet!" tour appears; after, it does
not. A HOME whose `Mudlet.ini` has a recent `firstLaunchDate` still gets
the tour, and a genuinely empty HOME records one.
Assisted-by: Claude:claude-opus-5
#### Brief overview of PR changes/additions
- A profile save hands the modules that are set to sync to a thread pool
task and returns. Two closes wait for nothing - answering "No" to "Save
profile?", and any close that finds the main console already gone - so
the `Host` is destroyed with the write still going, and the write went
on reading it: its `XMLexport`, and then its name. The job now carries
its own copy of each module's document plus every path it needs, and
`writeModuleFiles()`/`updateModuleZip()` are static, so nothing it
touches belongs to the profile.
- Both save watchers are now owned by the profile that made them. They
were unparented, and the `deleteLater()` they are wired to needs an
event loop that is still running to be delivered - which on the way out
there is not, and for a destroyed `Host` there is no owner left either.
- The unpacked module folder is created before the write that goes into
it rather than after, so a module whose folder the user removed no
longer fails to write and then loses its stale XML from its archive with
no replacement going in.
#### Motivation for adding to Mudlet
Save a profile that has a module set to sync - Lua `saveProfile()`, the
editor's autosave, a package install - then close the profile without
saving. That is a crash or a half-rewritten `.mpackage` on the way out,
i.e. another "Mudlet crashed when I closed it". Same shape as #9653, and
it survived 850 sanitizer runs only because no test profile had a module
in it: with none installed the save's module list comes out empty and
the write returns at its first line, so the entire path was unreachable
in the fixtures.
#### Other info (issues closed, discussion etc)
Test case: install a module, `enableModuleSync()` it, `saveProfile()`,
then close the profile answering "No" - Mudlet exits cleanly and the
module still lands on disk and in its archive.
New `ModuleSaveTeardownTest` is the module coverage that was missing
anywhere in the tree - no C++ test installed a module at all. It holds
the thread pool so the write is provably still queued when the `Host` is
destroyed, then lets it run: without the fix that kills the run under
AddressSanitizer inside `Host::writeModuleFiles()`. The watcher half is
pinned by asserting the watchers are owned by the profile and gone with
it, because LSan cannot see this one (a pending `QFutureCallOutEvent`
keeps it reachable at exit) and the functional tests run with
`detect_leaks=0` anyway. `Package_spec.lua` gains a synced-module save
so the write also runs under the busted job's leak-checked ASan.
Assisted-by: Claude:claude-opus-5
#### Brief overview of PR changes/additions
- `add-milestone` resolves the milestone by exact title first and then
by version prefix, so `5.0.0` finds `5.0.0 next release` again, and
fails loudly instead of assigning nothing. 191 PRs merged since 4.22.0
have no milestone.
- `TKeySequenceEditTest`'s two focus traversal cases no longer fail
under a bare Xvfb: ctest pins the offscreen platform on X11, and a
direct run without a window manager skips with a message instead of
burning the activation timeout twice.
#### Motivation for adding to Mudlet
Both were failing silently. The milestone step matched a title that no
longer exists and exited 0; and the traversal tests were the one red
mark in an otherwise green local suite, which everybody had to re-derive
as environmental.
The milestone lookup is now a script under `.github/scripts/`, covered
by a new `MilestoneResolutionTest` that runs it against a stubbed `gh`.
Re-introducing the original bug makes that test fail.
Worth knowing: `add-milestone` on this PR still assigned nothing,
because `pull_request_target` runs the copy of the workflow that is on
the base branch. It takes effect for pull requests opened after this
merges.
#### Other info (issues closed, discussion etc)
- Closes#9671 - CI: add-milestone silently assigns nothing - metadata
says "4.23.0" but the milestone is titled "4.23.0 next release"
- Closes#9575 - TKeySequenceEditTest: two traversal tests fail under
bare Xvfb (no window manager)
Test case: `ctest -R 'MilestoneResolutionTest|TKeySequenceEditTest'`,
plus `xvfb-run --auto-servernum ctest -R TKeySequenceEditTest` for the
case #9575 is about.
Assisted-by: Claude:claude-opus-5
