#### 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
Convert raw millisecond integer literals at time-duration call sites to
`std::chrono` literals, and add `#include <chrono>` to each touched
translation unit. Examples:
- `QTimer::singleShot(0, ...)` → `QTimer::singleShot(0ms, ...)`
- `mpTimerReplay->setInterval(1000)` → `setInterval(1s)`
- `mPendingTimer.start(60000)` → `start(1min)`
- `QObject::startTimer(50)` → `startTimer(50ms)`
- `QTest::qWait(100)` → `QTest::qWait(100ms)`
- `QThread::msleep(10)` → `QThread::sleep(10ms)`
This is a semantics-preserving refactor - every duration is kept exactly
equal to before (e.g. `1000` ms becomes `1s`, `60000` ms becomes
`1min`). No behavioural change.
#### Motivation for adding to Mudlet
Chrono literals make time durations self-documenting and type-safe. `1s`
/ `100ms` read unambiguously where a bare `1000` / `100` forces the
reader to remember each API's unit, and the compiler now rejects unit
mismatches. Only genuine duration arguments were converted - loop
counts, scroll-line counts, sizes, ports and the like were deliberately
left as plain integers.
All targeted APIs provide `std::chrono` overloads in the minimum
supported Qt (6.8.2): `QTimer::singleShot`/`start`/`setInterval` (5.8),
`QObject::startTimer` (5.9), `QThread::sleep(std::chrono::nanoseconds)`
(6.6) and `QTest::qWait(std::chrono::milliseconds)` (6.7).
#### Other info (issues closed, discussion etc)
Test case: the full application builds cleanly and the entire functional
`ctest` suite passes. The only failing test is the known, pre-existing
`PasswordMigrationTest` LSan exit-leak (GTK3/fontconfig noise), which is
unrelated to this change.
Assisted-by: Claude:claude-opus-4-8
<!-- Keep the title short & concise so anyone non-technical can
understand it,
the title appears in PTB changelogs -->
#### Brief overview of PR changes/additions
Remove unnecessary else/else-if after return, break, continue, and throw
statements - and in places where fixing them is more trouble than its
worth, added NOLINT.
#### Motivation for adding to Mudlet
https://clang.llvm.org/extra/clang-tidy/checks/readability/else-after-return.html,
so it doesn't pop up in PR reviews.
#### Other info (issues closed, discussion etc)
---------
Co-authored-by: Stephen Lyons <slysven@virginmedia.com>
Co-authored-by: Vadim Peretokin <vadi2@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
### Refactor: replace raw pointer ownership with smart pointers across
core subsystems
#### Brief overview of PR changes/additions
Replaces raw pointer ownership patterns with `std::unique_ptr` and
`std::map`
across several core subsystems:
- **Host**: `mStopWatchMap` (`QMap<int, stopWatch*>` → `std::map<int,
unique_ptr<stopWatch>>`), `profileShortcuts` (`QMap<QString,
QKeySequence*>` → `std::map<QString, unique_ptr<QKeySequence>>`).
Removes `qDeleteAll` in destructor and `delete mMMCPServer`.
- **TMap**: `mpRoomDB` raw pointer → `unique_ptr`
- **VarUnit**: `base` raw pointer → `unique_ptr`
- **TTrigger**: condition map storage converted to `unique_ptr`,
destructor simplified
- **discord**: handler and presence maps converted from raw pointer
`QMap` to `unique_ptr` + `std::map`
- **Updater**: `mFeed` and `mUpdateDialog` converted to `unique_ptr`
#### Motivation for adding to Mudlet
These patterns were identified as sources of memory leaks and potential
use-after-free bugs. Using smart pointers makes ownership explicit,
eliminates manual cleanup code, and ensures correct destruction even on
early-exit paths.
#### Other info (issues closed, discussion etc)
sorry this one is still pretty big, but most of the changes are the same
for each thing so reviewing them together probably makes sense. sadly
there isn't much to see here other than no slow uptick of heap size :-[
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
## Summary
Players had no clear way to control what Discord shows about their
Mudlet activity. The old checkbox in the connection pane only gated
server GMCP data but didn't prevent Discord from showing "Playing
Mudlet", and the privacy controls were confusing. This PR replaces all
of that with three straightforward modes via radio buttons in Profile
Preferences > Chat:
- **Show full game details (if supported)** - full game integration with
server-provided presence (default)
- **Show Mudlet only** - only shows "playing Mudlet", game server is not
told about Discord
- **Disabled** - Discord shows nothing about Mudlet
Players pick the mode that matches their comfort level, and the existing
privacy checkboxes (hide detail, hide state, etc.) remain available in
Game details mode for finer control.
### What changed
- **Three-mode radio buttons** in Profile Preferences > Chat with a
two-column layout (modes on the left, privacy controls on the right),
replacing the old connection-pane checkbox
- **Server-origin tracking** so privacy checkboxes only gate data sent
by the game server - Lua API calls always pass through (only Disabled
mode blocks Lua entirely)
- **Mid-session mode switching** via dynamic GMCP negotiation
(Core.Supports.Add/Remove + External.Discord.Hello/Get)
- **Deferred RPC init** - Discord RPC now starts when a profile loads,
not on app launch
- **Username restriction improvements** - takes effect immediately,
case-insensitive (Discord usernames are lowercase-only since 2023),
shuts down RPC when mismatched
- **Shows logged-in Discord user** in preferences next to the
restriction field, with a tooltip explaining the desktop app requirement
when not connected
- **Presence fix** - empty string fields now send nullptr so Discord
hides them instead of showing blanks
- **Memory leak fix** - presence allocations are now freed in the
destructor regardless of RPC state
### Cleanup
- Removed obsolete discriminator field
(`mRequiredDiscordUserDiscriminator`) - Discord removed discriminators
in 2023
- Removed dead code (`getDiscordUserDetails()`, never called)
- Restored `Discord_ClearPresence` function pointer for potential future
use
- Use proper `Host::DiscordOptionFlags` types instead of raw `int`
(thanks @SlySven)
### Known quirks
- The "Hide timer" checkbox correctly omits timestamps from presence
data, but Discord's client starts its own activity timer for any
presence without a timestamp - this is Discord client behavior outside
our control.
- The "Hide large icon" setting clears the image key, but some Discord
clients fall back to the application's default icon instead of hiding it
entirely.
### Test plan
- [ ] Open Profile Preferences > Chat tab
- [ ] Switch between the three radio button modes and verify Discord
presence updates accordingly
- [ ] In Game details mode, toggle privacy checkboxes and verify fields
are hidden/shown
- [ ] Use Lua API (e.g. `setDiscordDetail("test")`) in Mudlet only mode
- should work. Try in Disabled mode - should fail with error
- [ ] Set a username restriction and verify presence clears immediately
if mismatched
- [ ] Run unit tests: `cd build && ./test/DiscordTest`
- [ ] Run functional tests: `cd build && ctest -R TDiscordModeTest -V`
Closes#6967. Supersedes #7438.
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: SlySven <slysven@virginmedia.com>
#### Brief overview of PR changes/additions
Ran clang-format on all 134 CPP files in src/ using the project's
.clang-format config
#### Motivation for adding to Mudlet
Ensures consistent code formatting across the codebase.
#### Other info (issues closed, discussion etc)
None
**Test case:** Build the project and verify it compiles successfully.
#### Brief overview of PR changes/additions
Replace C-style string copy functions (`strcpy`/`strncpy`) with safer
`memcpy` + explicit null termination across 5 files. Adds null checks
for memory allocation failures.
#### Motivation for adding to Mudlet
Follows [curl's security
guidance](https://daniel.haxx.se/blog/2025/12/29/no-strcpy-either/) on
eliminating unsafe string functions. Also fixes a latent bug in
`TAlias.cpp` where a dangling pointer could occur from temporary object
lifetimes.
#### Other info (issues closed, discussion etc)
**Test case:** Verify triggers, aliases, and Discord Rich Presence still
work:
1. Create a regex alias (e.g. `^test (.+)$`) and confirm it captures
groups correctly
2. Create a trigger with a filter chain and confirm child triggers match
3. If Discord integration is enabled, verify game status displays in
Discord
No functional changes intended - this is a defensive code quality
improvement.
---------
Co-authored-by: Claude <noreply@anthropic.com>
## Brief overview of PR changes/additions
This PR fixes three memory leaks in Mudlet's core components that were
causing gradual memory growth during extended sessions:
- **Trigger Editor:** Fixed 96KB leak per editor session by transferring
auto-complete provider ownership to Edbee via `giveProvider()`
- **Profile Management:** Fixed 272-byte leak per profile load/unload by
properly cleaning up QKeySequence shortcuts
- **Discord Integration:** Fixed 48-byte leak on shutdown by moving
event handler cleanup outside conditional block
## Motivation for adding to Mudlet
These memory leaks were causing Mudlet to gradually consume more memory
over time, particularly noticeable during extended play sessions. The
fixes prevent unnecessary memory growth and improve overall application
stability.
## Other info (issues closed, discussion etc)
- All changes maintain Qt6/C++20 compatibility
- The auto-complete provider ownership is transferred to Edbee via
`giveProvider()`, which handles cleanup automatically at app shutdown
- Changes have been tested with successful builds on macOS
Based off of logs
[message.txt](https://github.com/user-attachments/files/24362610/message.txt)
[message_1.txt](https://github.com/user-attachments/files/24362611/message_1.txt)
from this Discord
[discussion](https://discord.com/channels/283581582550237184/427919962561052673/1454549976276533360).
<!-- Keep the title short & concise so anyone non-technical can
understand it,
the title appears in PTB changelogs -->
#### Brief overview of PR changes/additions
Remove unnecessary else blocks following return statements throughout
the codebase.
#### Motivation for adding to Mudlet
Better code readability
#### Other info (issues closed, discussion etc)
Other anti patterns are visible in this PR thanks to this, but let's
keep this PR focused on one pattern only.
---------
Co-authored-by: Claude <noreply@anthropic.com>
<!-- Keep the title short & concise so anyone non-technical can
understand it,
the title appears in PTB changelogs -->
#### Brief overview of PR changes/additions
Remove support for leak detection using MSVC - this functionality hasn't
been in use in a decade, and we're switching to to using
[LeakSanitizer](https://github.com/google/sanitizers/wiki/AddressSanitizerLeakSanitizer)
instead.
#### Motivation for adding to Mudlet
Cleaner code.
#### Other info (issues closed, discussion etc)
---------
Co-authored-by: Claude <noreply@anthropic.com>
#### Brief overview of PR changes/additions
In all places in our code where we use `Q_UNUSED(...)` to silence a
warning about an argument to a method/function that isn't used this PR
removes any following `;` as this macro does NOT need it.
#### Motivation for adding to Mudlet
For consistency across the entirety of our code, so that all our usage
of this macro is "correct" and doesn't include something that is
unneeded.
#### Other info (issues closed, discussion etc)
There are places in third-party code that does include it but it isn't
our job to clean those up!
Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
<!-- Keep the title short & concise so anyone non-technical can
understand it,
the title appears in PTB changelogs -->
#### Brief overview of PR changes/additions
Replace Q_OS_WIN32 macro with Q_OS_WINDOWS.
#### Motivation for adding to Mudlet
We're not really checking for 32bit Windows here - these checks apply
[both for 32bit and
64bit](https://doc.qt.io/qt-5/qtglobal.html#Q_OS_WIN32).
#### Other info (issues closed, discussion etc)
Co-authored-by: Vadim Peretokin <vadi2@users.noreply.github.com>
<!-- Keep the title short & concise so anyone non-technical can
understand it,
the title appears in PTB changelogs -->
#### Brief overview of PR changes/additions
Change the URL of file mentioned at
https://osdn.net/projects/mingw/downloads/68260/mingw-get-0.6.3-mingw32-pre-20170905-1-bin.zip/
#### Motivation for adding to Mudlet
Builds were failing, but not every time, like
https://ci.appveyor.com/project/Mudlet/mudlet/builds/49542655
Last lines were
```
==== compiling and installing mingw-get ====
---- Downloading ----
Exception calling "DownloadFile" with "2" argument(s): "The underlying connection was closed: Could not establish
trust relationship for the SSL/TLS secure channel."
```
I looked in the script files and found URL associated with mingw-get and
downloaded it myself a few times. It uses a 302 Location redirect going
to 2 different URLs, sometimes working and others with an expired
certificate.
Then I found the link above which is similar URL but different `m=`.
Looks like `m` is for mirror, and the old one is name of a German
university that maybe is no longer mirroring for them or something.
#### Other info (issues closed, discussion etc)
Discussion started with
https://discord.com/channels/283581582550237184/283582439002210305/1225440152621551698
<!-- Keep the title short & concise so anyone non-technical can
understand it,
the title appears in PTB changelogs -->
#### Brief overview of PR changes/additions
Mark read-only variables as const - that is, not intended to change
after they've been declared
#### Motivation for adding to Mudlet
So we don't change them by accident later on, and also to clearly state
intentions for the variables
#### Other info (issues closed, discussion etc)
This is also recommended by the [C++ Core
Guidelines](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es25-declare-an-object-const-or-constexpr-unless-you-want-to-modify-its-value-later-on)
#### Brief overview of PR changes/additions
Clazy was picking up "connect-3arg-lambda" warnings, explained as item 3
here: https://www.kdab.com/nailing-13-signal-slot-mistakes-clazy-1-3 .
#### Motivation for adding to Mudlet
It seemed to fix crashes I was getting when I was trying to change the
password secure storage setting, see:
https://github.com/Mudlet/Mudlet/issues/6671#issuecomment-1483211392
however those were not happening every time - but that could be down to
which threads the two ends of a connect was happening on, which the
extra argument helps to determine AFAICT.
I also think it might help in other places where we were getting odd
crashes - maybe even #6671 ?
Also:
* remove some redundant `QObject` prepended to `connect(...)` calls
* rename some arguments passed into some lambda so that they do not
shadow other variables in the parent methods.
* identify `dlgProfilePreferences::hidePasswordMigrationLabel()` as a Qt
SLOT, renamed to `slot_hidePasswordMigrationLabel()`.
Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
There is also some minor cosmetic changes, mainly with members initialised
within the body of the constructors. Also some bogus uses of `nullptr`
**pointer** value instead of a `QColor()` default value for a couple of
`QColors` in the `dlgRoomSybol` class.
I have steered away from using `{}` to initialise simple, POD data types in
favour of explicitly stating what their default values are.
Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
* reset function
* copied more than necessary
* Update TLuaInterpreter.cpp
* Update TLuaInterpreter.cpp
* move it to local function
* Update discord.cpp
* Update discord.cpp
* Drop all locks.
Exception: The map import lock is actually necessary, but we're not
doing any multithreading *or* scoped unlocking here. Thus a simple
boolean flag works just as well.
* restore Discord::getDiscordUserDetails code
* ... and remove the locking.
* lua_error()+return 1 => return lua_error()
* Missed fix-ups from the last cleanup patch
* Flip conditions to remove return-error-else antipatterns
* also fix TForkedProcess
* Remove QString::fromUtf8 calls
They're superfluous and make the code less readable.
Classed as "High Impact":
CID Type Detail
1492499 "Uninitialized scalar variable (UNINIT) "5. uninit_use: Using
uninitialized value error."
1485860 "No virtual destructor" "A1. dtor_in_derived:
Class `XMLimport` has a compiler-generated destructor. It is non-empty
because of its field `mpHost`. A pointer to class `XMLimport` is upcast to
class `QXmlStreamReader` which doesn't have a virtual destructor."
Classed as "Medium Impact":
1492834 "Uninitialized scalar field (UNINIT_CTOR)" "2. uninit_member:
Non-static class member `mIsEndTag` is not initialized in this constructor
nor in any functions that it calls.
4. uninit_member: Non-static class member `mIsEmptyTag` is not initialized
in this constructor nor in any functions that it calls.
6. uninit_member: Non-static class member `mReadingAttrValue` is not
initialized in this constructor nor in any functions that it calls.
8. uninit_member: Non-static class member `mOpeningQuote` is not
initialized in this constructor nor in any functions that it calls."
1488910 "Uninitialized scalar field (UNINIT_CTOR)" "2. uninit_member:
Non-static class member `mPlayerRoomStyle` is not initialized in this
constructor nor in any functions that it calls."
"4. uninit_member: Non-static class member
`mPlayerRoomOuterDiameterPercentage` is not initialized in this
constructor nor in any functions that it calls."
"6. uninit_member: Non-static class member
`mPlayerRoomInnerDiameterPercentage` is not initialized in this
constructor nor in any functions that it calls."
1478854 "Uninitialized pointer field (UNINIT_CTOR)" "4. uninit_member:
Non-static class member `mpOutOfBandDataIncomingCodec` is not initialized
in this constructor nor in any functions that it calls."
1468478 "Unchecked return value (CHECKED_RETURN)" "10. check_return:
Calling `luaL_loadstring` without checking return value (as is done
elsewhere 17 out of 21 times)."
1468477 "Unchecked return value (CHECKED_RETURN)" "14. check_return:
Calling `luaL_loadstring` without checking return value (as is done
elsewhere 17 out of 21 times)."
1468474 "Unchecked return value (CHECKED_RETURN)" "16. check_return:
Calling `luaL_loadstring` without checking return value (as is done
elsewhere 17 out of 21 times)." - x 2
1468468 "Logically dead code (DEADCODE)" "dead_error_line:
Execution cannot reach this statement: `return 1;`"
1415097 "Dereference null return value (NULL_RETURNS)" "8. dereference:
Dereferencing timer, which is known to be `nullptr`"
1415092 "Identical code for different branches (IDENTICAL_BRANCHES)"
"identical_branches: The same code is executed regardless of whether
`areaExit` is true, because the 'then' and 'else' branches are identical.
Should one of the branches be modified, or the entire 'if' statement
replaced?"
1415023 "Dereference null return value (NULL_RETURNS)" "26. dereference:
Dereferencing a pointer that might be `nullptr` `pR->name` when calling
`QString`.
1414989 "Explicit null dereferenced (FORWARD_NULL)" "81. var_deref_op:
Dereferencing null pointer
`this->originalExits.value(dirCode, TExit * const(NULL))`." x 11
1414977 "Logically dead code (DEADCODE)" "dead_error_line:
Execution cannot reach this statement: `return false;`."
Also removed unused:
* (int) cTelnet::curX & curY,
* (double) cTelnet::networkLatencyMin & networkLatencyMax
* (QMutex) TimerUnit::mTimerUnitLock
Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
* Don't be redundant on Linux
* add macOS and Windows
* Update discord.cpp
* Rework error message not to say 'not found' when it couldn't load
* Trim lengthy message
From a user's point of view, it doesn't really matter if it is additional or not.
This entails changing the name of the existing 32 Bits one so that another
one for 64 Bits can be placed alongside it in the same directory. The
Discord release zip file puts them (and the Linux and MacOs 64 bit ones) in
separate directories - which is a little inconvenient for us...
I have also put in some qDebug() messages - because I am having difficulty
getting the Discord RPC to respond to local builds - though it works fine
for Linux AppImages - which use the very same copy of the library for THAT
OS. They are enabled with a DEBUG_DISCORD #define visible to the
discord.cpp compilation unit...
Turns out the origin Windows libraries I had put in were both 32-bit ones -
the ones inserted now are described differently by the MSYS2 `file` command as:
* discord-rpc32.dll: "PE32 executable (DLL) (console) Intel 80386, for MS
Windows"
* discord-rpc64.dll: "PE32+ executable (DLL) (console) x86-64, for MS
Windows"
Further more they were certified and signed on "27 November 2018 17:20" so
that is consistent with them being the (current) version 3.4.0 release
files from that same date.
Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
