Add Communications Manager with GMCP and text-based parsing #427
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Communications System: - New CommsManager to handle all in-game communications - CommsWidget displaying communications in categorized tabs (Direct, Local, Global) - Real-time parsing from GMCP Comm.Channel messages - Fallback text-based parsing when GMCP unavailable - Tab-based organization (Tell/Whisper, Say/Emote, Narrate/Yell/etc) Features: - Color-coded communication types (customizable) - Talker identification (You, Player, NPC, Ally, Neutral, Enemy) - Font styling options (CAPS for yells, italic for whispers/emotes) - Optional timestamps - Communication log saving - Tab muting/filtering - Dockable communications panel Configuration: - Comprehensive color settings for each comm type - Talker-based coloring system - Font styling preferences - Log directory configuration - Per-tab muting controls GMCP Integration: - Requires PR-B (GMCP Broadcast) for sig_rawGameText signal - Parses Comm.Channel GMCP messages - Falls back to text parsing for compatibility UI: - Added Comms preferences page - Integrated with main window dock system - Tabbed interface for easy navigation This provides a centralized, organized view of all game communications with extensive customization options.
Reviewer's GuideIntroduces a new communications subsystem (CommsManager + CommsWidget) wired to GMCP and raw text, adds configurable comms colors/styles and hotkeys, a dockable Communications panel, and a map visibility filter with background/seasonal texture settings, while refactoring configuration and main window wiring to support these features and GMCP clock broadcasting. Sequence diagram for GMCP and raw text communications flowsequenceDiagram
actor Player
participant MumeServer
participant GameObserver
participant CommsManager
participant CommsWidget
participant AutoLogger
participant MainWindow
Player->>MumeServer: In‑game communication
MumeServer-->>GameObserver: GMCP Comm.Channel message
GameObserver-->>CommsManager: sig2_sentToUserGmcp(gmcp)
CommsManager->>CommsManager: slot_parseGmcpInput(gmcp)
CommsManager->>CommsManager: parseCommChannelText(gmcp)
CommsManager->>CommsManager: getCommTypeFromChannel(channel)
CommsManager->>CommsManager: getCategoryFromType(type)
CommsManager->>CommsManager: trackYellMessage(sender,message) [if YELL]
CommsManager-->>CommsWidget: sig_newMessage(CommMessage)
CommsManager-->>MainWindow: sig_log(module,message)
MumeServer-->>GameObserver: raw game text line
GameObserver-->>CommsManager: sig2_rawGameText(text)
CommsManager->>CommsManager: slot_parseRawGameText(text)
CommsManager->>CommsManager: parseFallbackYell(text)
CommsManager->>CommsManager: isRecentYellDuplicate(sender,message)?
alt not duplicate and matches yell pattern
CommsManager->>CommsManager: trackYellMessage(sender,message)
CommsManager-->>CommsWidget: sig_newMessage(CommMessage{YELL})
end
CommsWidget->>CommsWidget: slot_onNewMessage(msg)
CommsWidget->>CommsWidget: isMessageFiltered(msg)?
alt not filtered
CommsWidget->>CommsWidget: appendFormattedMessage(msg)
CommsWidget->>CommsWidget: stripAnsiCodes()
CommsWidget->>CommsWidget: cleanSenderName()
CommsWidget->>CommsWidget: getColorForTalker(talkerType)
CommsWidget->>CommsWidget: getColorForType(type)
CommsWidget->>CommsWidget: formatAndInsertMessage()
end
MainWindow->>CommsWidget: closeEvent()
CommsWidget->>CommsWidget: slot_saveLogOnExit()
CommsWidget->>AutoLogger: getCurrentFileNumber()
CommsWidget->>CommsWidget: build Comms_Log filename
CommsWidget->>CommsWidget: write QTextDocument to file
Class diagram for the new communications and configuration typesclassDiagram
class CommsManager {
+CommsManager(QObject *parent)
+~CommsManager()
+void slot_parseGmcpInput(const GmcpMessage &msg)
+void slot_parseRawGameText(const QString &rawText)
-void parseCommChannelText(const GmcpMessage &msg)
-void parseFallbackYell(const QString &rawText)
-CommType getCommTypeFromChannel(const QString &channel)
-CommCategory getCategoryFromType(CommType type)
-void trackYellMessage(const QString &sender, const QString &message)
-bool isRecentYellDuplicate(const QString &sender, const QString &message) const
-QHash~QString,qint64~ m_recentYells
}
class CommsWidget {
+CommsWidget(CommsManager &commsManager, AutoLogger *autoLogger, QWidget *parent)
+~CommsWidget()
+void slot_onNewMessage(const CommMessage &msg)
+void slot_loadSettings()
+void slot_saveLog()
+void slot_saveLogOnExit()
-void setupUI()
-void connectSignals()
-void resizeEvent(QResizeEvent *event)
-void slot_onFilterToggled(CommType type, bool enabled)
-void slot_onCharMobToggle()
-void appendFormattedMessage(const CommMessage &msg)
-void formatAndInsertMessage(QTextCursor &cursor, const CommMessage &msg, const QString &sender, const QString &message)
-QString stripAnsiCodes(const QString &text)
-QString cleanSenderName(const QString &sender)
-QColor getColorForType(CommType type)
-QColor getColorForTalker(TalkerType talkerType)
-bool isMessageFiltered(const CommMessage &msg) const
-void updateFilterButtonAppearance(QPushButton *button, bool enabled)
-void updateCharMobButtonAppearance()
-void rebuildDisplay()
-void updateButtonLabels()
-CommsManager &m_commsManager
-AutoLogger *m_autoLogger
-QTextEdit *m_textDisplay
-QToolButton *m_charMobToggle
-QMap~CommType,QPushButton*~ m_filterButtons
-QMap~CommType,bool~ m_filterStates
-CharMobFilterEnum m_charMobFilter
-QList~CachedMessage~ m_messageCache
-static const int MAX_MESSAGES
}
class CommMessage {
+CommType type
+CommCategory category
+QString sender
+QString message
+QString timestamp
+TalkerType talkerType
}
class CommsPage {
+CommsPage(QWidget *parent)
+~CommsPage()
+void slot_loadConfig()
-void setupUI()
-void connectSignals()
-void updateColorButton(QPushButton *button, const QColor &color)
-void slot_onColorClicked()
-void slot_onBgColorClicked()
-void slot_onYellAllCapsChanged(Qt::CheckState state)
-void slot_onWhisperItalicChanged(Qt::CheckState state)
-void slot_onEmoteItalicChanged(Qt::CheckState state)
-void slot_onShowTimestampsChanged(Qt::CheckState state)
-QPushButton *m_tellColorButton
-QPushButton *m_whisperColorButton
-QPushButton *m_groupColorButton
-QPushButton *m_askColorButton
-QPushButton *m_sayColorButton
-QPushButton *m_emoteColorButton
-QPushButton *m_socialColorButton
-QPushButton *m_yellColorButton
-QPushButton *m_narrateColorButton
-QPushButton *m_prayColorButton
-QPushButton *m_shoutColorButton
-QPushButton *m_singColorButton
-QPushButton *m_bgColorButton
-QPushButton *m_talkerYouColorButton
-QPushButton *m_talkerPlayerColorButton
-QPushButton *m_talkerNpcColorButton
-QPushButton *m_talkerAllyColorButton
-QPushButton *m_talkerNeutralColorButton
-QPushButton *m_talkerEnemyColorButton
-QCheckBox *m_yellAllCapsCheck
-QCheckBox *m_whisperItalicCheck
-QCheckBox *m_emoteItalicCheck
-QCheckBox *m_showTimestampsCheck
}
class Configuration {
+ParserSettings parser
+CanvasSettings canvas
+Hotkeys hotkeys
+CommsSettings comms
+MumeClockSettings mumeClock
}
class ParserSettings {
+bool enableYellFallbackParsing
}
class CanvasSettings {
+TextureSetEnum textureSet
+bool enableSeasonalTextures
+float layerTransparency
+bool enableRadialTransparency
+Advanced advanced
+VisibilityFilter visibilityFilter
}
class CanvasSettings_Advanced {
+bool useBackgroundImage
+QString backgroundImagePath
+int backgroundFitMode
+float backgroundOpacity
+float backgroundFocusedScale
+float backgroundFocusedOffsetX
+float backgroundFocusedOffsetY
+FixedPoint~1~ fov
+FixedPoint~1~ verticalAngle
+FixedPoint~1~ horizontalAngle
+FixedPoint~1~ layerHeight
}
class VisibilityFilter {
+NamedConfig~bool~ generic
+NamedConfig~bool~ herb
+NamedConfig~bool~ river
+NamedConfig~bool~ place
+NamedConfig~bool~ mob
+NamedConfig~bool~ comment
+NamedConfig~bool~ road
+NamedConfig~bool~ object
+NamedConfig~bool~ action
+NamedConfig~bool~ locality
+NamedConfig~bool~ connections
+bool isVisible(InfomarkClassEnum markerClass) const
+void setVisible(InfomarkClassEnum markerClass, bool visible)
+bool isConnectionsVisible() const
+void setConnectionsVisible(bool visible)
+void showAll()
+void hideAll()
+void registerChangeCallback(const ChangeMonitor::Lifetime &lifetime, const ChangeMonitor::Function &callback)
}
class Hotkeys {
+NamedConfig~QString~ fileOpen
+NamedConfig~QString~ fileSave
+NamedConfig~QString~ fileReload
+NamedConfig~QString~ fileQuit
+NamedConfig~QString~ editUndo
+NamedConfig~QString~ editRedo
+NamedConfig~QString~ editPreferences
+NamedConfig~QString~ editPreferencesAlt
+NamedConfig~QString~ editFindRooms
+NamedConfig~QString~ editRoom
+NamedConfig~QString~ viewZoomIn
+NamedConfig~QString~ viewZoomOut
+NamedConfig~QString~ viewZoomReset
+NamedConfig~QString~ viewLayerUp
+NamedConfig~QString~ viewLayerDown
+NamedConfig~QString~ viewLayerReset
+NamedConfig~QString~ viewRadialTransparency
+NamedConfig~QString~ viewStatusBar
+NamedConfig~QString~ viewScrollBars
+NamedConfig~QString~ viewMenuBar
+NamedConfig~QString~ viewAlwaysOnTop
+NamedConfig~QString~ panelLog
+NamedConfig~QString~ panelClient
+NamedConfig~QString~ panelGroup
+NamedConfig~QString~ panelRoom
+NamedConfig~QString~ panelAdventure
+NamedConfig~QString~ panelDescription
+NamedConfig~QString~ panelComms
+NamedConfig~QString~ modeMoveMap
+NamedConfig~QString~ modeRaypick
+NamedConfig~QString~ modeSelectRooms
+NamedConfig~QString~ modeSelectMarkers
+NamedConfig~QString~ modeSelectConnection
+NamedConfig~QString~ modeCreateMarker
+NamedConfig~QString~ modeCreateRoom
+NamedConfig~QString~ modeCreateConnection
+NamedConfig~QString~ modeCreateOnewayConnection
+NamedConfig~QString~ roomCreate
+NamedConfig~QString~ roomMoveUp
+NamedConfig~QString~ roomMoveDown
+NamedConfig~QString~ roomMergeUp
+NamedConfig~QString~ roomMergeDown
+NamedConfig~QString~ roomDelete
+NamedConfig~QString~ roomConnectNeighbors
+NamedConfig~QString~ roomMoveToSelected
+NamedConfig~QString~ roomUpdateSelected
+void read(const QSettings &conf)
+void write(QSettings &conf) const
}
class CommsSettings {
+NamedConfig~QColor~ tellColor
+NamedConfig~QColor~ whisperColor
+NamedConfig~QColor~ groupColor
+NamedConfig~QColor~ askColor
+NamedConfig~QColor~ sayColor
+NamedConfig~QColor~ emoteColor
+NamedConfig~QColor~ socialColor
+NamedConfig~QColor~ yellColor
+NamedConfig~QColor~ narrateColor
+NamedConfig~QColor~ prayColor
+NamedConfig~QColor~ shoutColor
+NamedConfig~QColor~ singColor
+NamedConfig~QColor~ backgroundColor
+NamedConfig~QColor~ talkerYouColor
+NamedConfig~QColor~ talkerPlayerColor
+NamedConfig~QColor~ talkerNpcColor
+NamedConfig~QColor~ talkerAllyColor
+NamedConfig~QColor~ talkerNeutralColor
+NamedConfig~QColor~ talkerEnemyColor
+NamedConfig~bool~ yellAllCaps
+NamedConfig~bool~ whisperItalic
+NamedConfig~bool~ emoteItalic
+NamedConfig~bool~ showTimestamps
+NamedConfig~bool~ saveLogOnExit
+NamedConfig~QString~ logDirectory
+NamedConfig~bool~ muteDirectTab
+NamedConfig~bool~ muteLocalTab
+NamedConfig~bool~ muteGlobalTab
+void read(const QSettings &conf)
+void write(QSettings &conf) const
}
class MumeClockSettings {
+int64_t startEpoch
+bool display
+NamedConfig~bool~ gmcpBroadcast
+NamedConfig~int~ gmcpBroadcastInterval
+MumeClockSettings()
+void read(const QSettings &conf)
+void write(QSettings &conf) const
}
class MainWindow {
+CommsManager *m_commsManager
+CommsWidget *m_commsWidget
+QDockWidget *m_dockDialogComms
+QAction *saveCommsLogAct
+void applyHotkeys()
+void registerGlobalShortcuts()
+void slot_setRadialTransparency()
}
class ConfigDialog {
+void createIcons()
+signals sig_commsSettingsChanged()
+signals sig_hotkeysChanged()
+signals sig_textureSettingsChanged()
}
class HotkeysPage
class GraphicsPage
class VisibilityFilterWidget
%% Relationships
CommsManager --> CommMessage
CommsWidget --> CommMessage
CommsWidget --> CommsManager
CommsWidget --> AutoLogger
CommsPage --> CommsSettings
Configuration --> ParserSettings
Configuration --> CanvasSettings
Configuration --> Hotkeys
Configuration --> CommsSettings
Configuration --> MumeClockSettings
CanvasSettings --> CanvasSettings_Advanced
CanvasSettings --> VisibilityFilter
MainWindow --> CommsManager
MainWindow --> CommsWidget
MainWindow --> VisibilityFilterWidget
ConfigDialog --> CommsPage
ConfigDialog --> HotkeysPage
ConfigDialog --> GraphicsPage
GraphicsPage --> CanvasSettings
HotkeysPage --> Hotkeys
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and found some issues that need to be addressed.
- In CommsManager::isRecentYellDuplicate(), you call m_recentYells[key] in a const method, which will not compile because QHash::operator[] is non-const and also inserts missing keys; use m_recentYells.value(key) (or find/const_iterator) instead to read without modifying the hash.
- You now have two different ANSI-stripping implementations (CommsWidget::stripAnsiCodes and the inline regex in CommsManager::parseFallbackYell); consider centralizing this into a shared helper to avoid divergence and keep the parsing consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In CommsManager::isRecentYellDuplicate(), you call m_recentYells[key] in a const method, which will not compile because QHash::operator[] is non-const and also inserts missing keys; use m_recentYells.value(key) (or find/const_iterator) instead to read without modifying the hash.
- You now have two different ANSI-stripping implementations (CommsWidget::stripAnsiCodes and the inline regex in CommsManager::parseFallbackYell); consider centralizing this into a shared helper to avoid divergence and keep the parsing consistent.
## Individual Comments
### Comment 1
<location> `src/comms/CommsManager.cpp:258-267` </location>
<code_context>
+ }
+}
+
+bool CommsManager::isRecentYellDuplicate(const QString &sender, const QString &message) const
+{
+ const QString key = QString("%1|%2").arg(sender, message);
+
+ if (!m_recentYells.contains(key)) {
+ return false;
+ }
+
+ // Check if it's recent (within last 2 seconds)
+ const qint64 timestamp = m_recentYells[key];
+ const qint64 now = QDateTime::currentMSecsSinceEpoch();
+ const qint64 age = now - timestamp;
</code_context>
<issue_to_address>
**issue (bug_risk):** Accessing m_recentYells via operator[] in a const method will not compile and also mutates the hash
In `isRecentYellDuplicate`, `m_recentYells[key]` calls the non-const `QHash::operator[]`, which inserts a default value for missing keys. This violates const-correctness and mutates the hash. Use a const-safe lookup instead, e.g. `m_recentYells.value(key, 0)` or `auto it = m_recentYells.constFind(key);` and check `it != m_recentYells.cend()` before reading the timestamp.
</issue_to_address>
### Comment 2
<location> `src/comms/CommsWidget.cpp:559-564` </location>
<code_context>
+ document.close();
+}
+
+void CommsWidget::slot_saveLogOnExit()
+{
+ const auto &comms = getConfig().comms;
+ if (!comms.saveLogOnExit.get()) {
+ return;
+ }
+
+ // Use the same directory as AutoLogger
+ const auto &autoLogConfig = getConfig().autoLog;
+ QString logDir = autoLogConfig.autoLogDirectory;
+ if (logDir.isEmpty()) {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Communications log directory ignores comms.logDirectory setting and always follows AutoLogger
`slot_saveLogOnExit` always uses `autoLog.autoLogDirectory` (or CWD) and never reads `comms.logDirectory`, even though the comms config exposes a dedicated `logDirectory`. If comms logs are meant to be independently configurable, this should first check `comms.logDirectory` (when non-empty) and only fall back to the AutoLogger directory when it is unset.
```suggestion
// Prefer dedicated comms log directory; fall back to AutoLogger directory, then CWD
const auto &autoLogConfig = getConfig().autoLog;
QString logDir = comms.logDirectory;
if (logDir.isEmpty()) {
logDir = autoLogConfig.autoLogDirectory;
}
if (logDir.isEmpty()) {
logDir = QDir::current().path();
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+258
to
+267
| bool CommsManager::isRecentYellDuplicate(const QString &sender, const QString &message) const | ||
| { | ||
| const QString key = QString("%1|%2").arg(sender, message); | ||
|
|
||
| if (!m_recentYells.contains(key)) { | ||
| return false; | ||
| } | ||
|
|
||
| // Check if it's recent (within last 2 seconds) | ||
| const qint64 timestamp = m_recentYells[key]; |
There was a problem hiding this comment.
issue (bug_risk): Accessing m_recentYells via operator[] in a const method will not compile and also mutates the hash
In isRecentYellDuplicate, m_recentYells[key] calls the non-const QHash::operator[], which inserts a default value for missing keys. This violates const-correctness and mutates the hash. Use a const-safe lookup instead, e.g. m_recentYells.value(key, 0) or auto it = m_recentYells.constFind(key); and check it != m_recentYells.cend() before reading the timestamp.
Comment on lines
+559
to
+564
| // Use the same directory as AutoLogger | ||
| const auto &autoLogConfig = getConfig().autoLog; | ||
| QString logDir = autoLogConfig.autoLogDirectory; | ||
| if (logDir.isEmpty()) { | ||
| logDir = QDir::current().path(); | ||
| } |
There was a problem hiding this comment.
suggestion (bug_risk): Communications log directory ignores comms.logDirectory setting and always follows AutoLogger
slot_saveLogOnExit always uses autoLog.autoLogDirectory (or CWD) and never reads comms.logDirectory, even though the comms config exposes a dedicated logDirectory. If comms logs are meant to be independently configurable, this should first check comms.logDirectory (when non-empty) and only fall back to the AutoLogger directory when it is unset.
Suggested change
| // Use the same directory as AutoLogger | |
| const auto &autoLogConfig = getConfig().autoLog; | |
| QString logDir = autoLogConfig.autoLogDirectory; | |
| if (logDir.isEmpty()) { | |
| logDir = QDir::current().path(); | |
| } | |
| // Prefer dedicated comms log directory; fall back to AutoLogger directory, then CWD | |
| const auto &autoLogConfig = getConfig().autoLog; | |
| QString logDir = comms.logDirectory; | |
| if (logDir.isEmpty()) { | |
| logDir = autoLogConfig.autoLogDirectory; | |
| } | |
| if (logDir.isEmpty()) { | |
| logDir = QDir::current().path(); | |
| } |
nschimme
reviewed
Dec 17, 2025
|
|
||
| public slots: | ||
| void slot_parseGmcpInput(const GmcpMessage &msg); | ||
| void slot_parseRawGameText(const QString &rawText); |
Contributor
There was a problem hiding this comment.
I don't see where these signals and slots are connected. I guess it got missed in the PR split?
nschimme
reviewed
Dec 17, 2025
| DELETE_CTORS_AND_ASSIGN_OPS(CommsManager); | ||
|
|
||
| public slots: | ||
| void slot_parseGmcpInput(const GmcpMessage &msg); |
Contributor
There was a problem hiding this comment.
Rather than slots we should follow the example of the GameObserver and AdventureTracker. GameObserver already has most of the wiring and we can use the same pattern as AdventureTracker to tap in.
nschimme
reviewed
Dec 17, 2025
| CommType CommsManager::getCommTypeFromChannel(const QString &channel) | ||
| { | ||
| // Map GMCP channel names to CommType (support both singular and plural forms) | ||
| if (channel == "tells" || channel == "tell") { |
Contributor
There was a problem hiding this comment.
What does MUME send? It doesn't make sense to support both if MUME only sends one type.
Contributor
There was a problem hiding this comment.
We actually get a list of available channels when we request support: https://mume.org/help/gmcp_comm.channel
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Communications System:
Features:
Configuration:
GMCP Integration:
UI:
This provides a centralized, organized view of all game communications with extensive customization options.
Summary by Sourcery
Introduce a configurable communications panel backed by a GMCP‑aware manager, extend graphics and canvas controls, and centralize hotkey handling with configurable shortcuts and visibility filters.
New Features:
Enhancements:
Build: