Add GMCP broadcast enhancements and clock synchronization #424
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
GMCP Improvements: - Enhanced GMCP message handling in proxy layer - Added raw game text signal (sig_rawGameText) for fallback parsing - Improved GMCP module support detection - Better error handling for GMCP communication Clock Broadcasting: - Convert gmcpBroadcast and gmcpBroadcastInterval to NamedConfig - Add change callbacks for immediate broadcaster restart - Fix checkbox/interval controls to work without reconnecting - Configurable broadcast interval (default: 2.5 seconds) - Auto-start broadcaster when client supports MUME.Time module GameObserver Integration: - Connect raw game text to GameObserver for fallback parsing - Enable communication parsing when GMCP unavailable - Support for yell fallback parsing configuration Documentation: - Added mudlet_clock_example.lua showing GMCP integration - Example demonstrates clock synchronization in Mudlet This PR establishes the foundation for real-time GMCP features and enables better synchronization between MMapper and clients.
Reviewer's GuideAdds a GMCP-aware clock broadcaster over the proxy (driven by new MUME.Time GMCP support), wires raw game text into GameObserver for fallback parsing, and extends configuration/UI (including new NamedConfigs, hotkey/comms/canvas options) plus Mudlet example script to support real‑time time/season info and communication features. Sequence diagram for GMCP MUME.Time clock broadcastingsequenceDiagram
actor MudClient
participant UserTelnet
participant Proxy
participant MumeClock
participant MumeServer
MudClient->>UserTelnet: send GMCP Core.Hello
UserTelnet->>UserTelnet: virt_receiveGmcpMessage(Core.Hello)
UserTelnet->>UserTelnet: sendSupportedGmcpModules()
UserTelnet-->>MudClient: GMCP Core.Supports.Set (MUME.Time 1, ...)
MudClient->>UserTelnet: enable GMCP module MUME.Time
UserTelnet->>UserTelnet: receiveGmcpModule(MUME.Time, true)
UserTelnet->>UserTelnet: m_gmcp.modules.insert(mod)
UserTelnet->>UserTelnet: m_gmcp.supported[type] = version
UserTelnet->>UserTelnetOutputs: onGmcpModuleEnabled(MUME_TIME, true)
UserTelnetOutputs->>Proxy: virt_onGmcpModuleEnabled(MUME_TIME, true)
Proxy->>Proxy: startClockBroadcaster()
Proxy->>Proxy: check config.mumeClock.gmcpBroadcast
Proxy->>Proxy: isUserGmcpModuleEnabled(MUME_TIME)
Proxy->>Proxy: create QTimer m_clockBroadcastTimer
Proxy->>Proxy: m_clockBroadcastTimer.start(interval)
Proxy->>Proxy: broadcastClockInfo() (initial)
loop every gmcpBroadcastInterval
Proxy->>Proxy: broadcastClockInfo()
Proxy->>MumeClock: getMumeMoment()
Proxy->>MumeClock: getPrecision()
MumeClock-->>Proxy: MumeMoment, precision
Proxy->>Proxy: build QJsonObject with time/season/moon
Proxy->>Proxy: create GmcpMessage(MUME_TIME_INFO, json)
Proxy->>UserTelnet: gmcpToUser(MUME.Time.Info)
UserTelnet-->>MudClient: send GMCP MUME.Time.Info
MudClient->>MudClient: handle gmcp.MUME.Time.Info (Lua script)
end
Note over Proxy,MudClient: Broadcaster stops when MUME.Time disabled or Proxy destroyed
Sequence diagram for raw game text fallback parsing via GameObserversequenceDiagram
participant MumeServer
participant Proxy
participant MumeXmlParser
participant GameObserver
participant FallbackParser
MumeServer-->>Proxy: raw Telnet data
Proxy->>MumeXmlParser: parse(TelnetData, isGoAhead)
MumeXmlParser->>MumeXmlParser: accumulate m_lineToUser
alt full line ready
MumeXmlParser-->>Proxy: sendToUser(FromMud, m_lineToUser)
MumeXmlParser-->>MumeXmlParser: emit sig_rawGameText(m_lineToUser)
Proxy->>Proxy: connected lambda for sig_rawGameText
Proxy->>GameObserver: observeRawGameText(text)
GameObserver->>GameObserver: sig2_rawGameText.invoke(text)
GameObserver-->>FallbackParser: raw game text callback
FallbackParser->>FallbackParser: parse yells and comms when GMCP unavailable
end
Sequence diagram for GMCP config change callbacks restarting broadcastersequenceDiagram
participant User
participant MumeProtocolPage
participant Configuration_mumeClock as MumeClockSettings
participant Proxy
User->>MumeProtocolPage: toggle gmcpBroadcast checkbox
MumeProtocolPage->>Configuration_mumeClock: gmcpBroadcast.set(enabled)
Note over Configuration_mumeClock,Proxy: NamedConfig change callback registered in Proxy::init
Configuration_mumeClock-->>Proxy: gmcpBroadcast change callback
Proxy->>Proxy: stopClockBroadcaster()
Proxy->>Proxy: startClockBroadcaster()
User->>MumeProtocolPage: change gmcpBroadcastInterval spinbox
MumeProtocolPage->>Configuration_mumeClock: gmcpBroadcastInterval.set(value)
Configuration_mumeClock-->>Proxy: gmcpBroadcastInterval change callback
Proxy->>Proxy: stopClockBroadcaster()
Proxy->>Proxy: startClockBroadcaster()
Updated class diagram for configuration, proxy, GMCP, clock, and observerclassDiagram
class Configuration {
+General general
+ConnectionSettings connection
+CanvasSettings canvas
+Hotkeys hotkeys
+CommsSettings comms
+MumeClockSettings mumeClock
}
class Configuration_General {
+bool enableYellFallbackParsing
}
class CanvasSettings {
+TextureSetEnum textureSet
+bool enableSeasonalTextures
+float layerTransparency
+bool enableRadialTransparency
+Advanced advanced
+VisibilityFilter visibilityFilter
+void read(QSettings conf)
+void write(QSettings conf) const
}
class Advanced {
+bool useBackgroundImage
+QString backgroundImagePath
+int backgroundFitMode
+float backgroundOpacity
+float backgroundFocusedScale
+float backgroundFocusedOffsetX
+float backgroundFocusedOffsetY
+FixedPoint fov
+FixedPoint verticalAngle
+FixedPoint horizontalAngle
+FixedPoint layerHeight
+void registerChangeCallback(ChangeMonitor_Lifetime lifetime, ChangeMonitor_Function callback)
}
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(ChangeMonitor_Lifetime lifetime, 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~ panelComms
+NamedConfig~QString~ panelDescription
+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(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(QSettings conf)
+void write(QSettings conf) const
}
class MumeClockSettings {
+int64_t startEpoch
+bool display
+NamedConfig~bool~ gmcpBroadcast
+NamedConfig~int~ gmcpBroadcastInterval
+MumeClockSettings()
+void read(QSettings conf)
+void write(QSettings conf) const
}
class Proxy {
-QTimer* m_clockBroadcastTimer
+void init()
+void gmcpToUser(GmcpMessage msg)
+void startClockBroadcaster()
+void stopClockBroadcaster()
+void broadcastClockInfo()
+~Proxy()
}
class UserTelnetOutputs {
+void onAnalyzeUserStream(RawBytes bytes, bool flush)
+void onRelayGmcpFromUserToMud(GmcpMessage msg)
+void onRelayNawsFromUserToMud(int width, int height)
+void onRelayTermTypeFromUserToMud(TelnetTermTypeBytes bytes)
+void onGmcpModuleEnabled(GmcpModuleTypeEnum type, bool enabled)
..virtual..
+virt_onAnalyzeUserStream(RawBytes bytes, bool flush)
+virt_onRelayGmcpFromUserToMud(GmcpMessage msg)
+virt_onRelayNawsFromUserToMud(int width, int height)
+virt_onRelayTermTypeFromUserToMud(TelnetTermTypeBytes bytes)
+virt_onGmcpModuleEnabled(GmcpModuleTypeEnum type, bool enabled)
}
class UserTelnet {
+void virt_receiveGmcpMessage(GmcpMessage msg)
+bool virt_isGmcpModuleEnabled(GmcpModuleTypeEnum name) const
-void receiveGmcpModule(GmcpModule mod, bool enabled)
-void resetGmcpModules()
-void sendSupportedGmcpModules()
}
class MumeClock {
-int64_t m_mumeStartEpoch
-MumeSeasonEnum m_lastSeasonEmitted
+MumeMoment getMumeMoment() const
+MumeClockPrecisionEnum getPrecision() const
+int64_t getLastSyncEpoch() const
+void parseMumeTime(QString mumeTime)
+void parseMumeTime(QString mumeTime, int64_t secsSinceEpoch)
+void parseWeather(MumeTimeEnum time, int64_t secsSinceEpoch)
+void parseClockTime(QString clockTime)
+void parseClockTime(QString clockTime, int64_t secsSinceEpoch)
+void parseMSSP(MsspTime msspTime)
-void onUserGmcp(GmcpMessage msg)
+signal sig_log(QString msg, QString category)
+signal sig_seasonChanged(MumeSeasonEnum newSeason)
}
class GameObserver {
+Signal2~GmcpMessage~ sig2_sentToUserGmcp
+Signal2~bool~ sig2_toggledEchoMode
+Signal2~QString~ sig2_rawGameText
+void observeConnected()
+void observeDisconnected()
+void observeSentToUser(QString text)
+void observeSentToUserGmcp(GmcpMessage msg)
+void observeToggledEchoMode(bool echo)
+void observeRawGameText(QString text)
}
class MumeXmlParser {
+signal sig_rawGameText(QString text)
+void parse(TelnetData data, bool isGoAhead)
+void parseGmcpCharVitals(JsonObj obj)
}
class GmcpMessage {
+GmcpMessageTypeEnum type
+GmcpJson payload
+bool isCoreHello() const
+ConstString getName() const
}
class GmcpModule {
+GmcpModuleTypeEnum getType() const
+bool isSupported() const
+bool hasVersion() const
+int getVersion() const
+std::string getNormalizedName() const
}
class MumeProtocolPage {
+void slot_loadConfig()
+void slot_internalEditorRadioButtonChanged(bool enabled)
+void slot_externalEditorCommandTextChanged(QString text)
+void slot_externalEditorBrowseButtonClicked(bool checked)
+void slot_gmcpBroadcastCheckBoxChanged(int state)
+void slot_gmcpIntervalSpinBoxChanged(int value)
}
Configuration o-- CanvasSettings
Configuration o-- Hotkeys
Configuration o-- CommsSettings
Configuration o-- MumeClockSettings
Configuration o-- Configuration_General
CanvasSettings o-- Advanced
CanvasSettings o-- VisibilityFilter
Proxy ..> MumeClock : uses
Proxy ..> GameObserver : uses
Proxy ..> MumeXmlParser : uses
Proxy ..> UserTelnet : interacts
UserTelnetOutputs <|.. Proxy : implements virt_onGmcpModuleEnabled
UserTelnet ..> UserTelnetOutputs : uses callbacks
UserTelnet ..> GmcpModule : receiveGmcpModule
UserTelnet ..> GmcpMessage : sendSupportedGmcpModules
MumeXmlParser ..> GameObserver : emits sig_rawGameText via Proxy
MumeClockSettings ..> MumeClock : configures
MumeProtocolPage ..> Configuration : reads and writes mumeClock
MumeProtocolPage ..> Proxy : indirectly restarts broadcaster via change callbacks
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 - here's some feedback:
- There is quite a bit of unconditional qDebug logging added in the GMCP and clock-broadcast paths (e.g. in UserTelnet, Proxy::startClockBroadcaster/broadcastClockInfo); consider reducing verbosity or gating these behind an existing debug flag to avoid noisy logs in normal use.
- The season-change notification logic in MumeClock is duplicated across parseMumeTime, parseWeather, and parseClockTime; consider extracting a small helper (e.g. updateAndEmitSeasonChange(moment)) to centralize this behavior and reduce the risk of future inconsistencies.
- This PR introduces a number of configuration additions (textures, hotkeys, comms, visibility filters) that are orthogonal to GMCP/clock synchronization; if possible, consider splitting these into a separate change to keep the GMCP/clock work more focused and easier to review.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There is quite a bit of unconditional qDebug logging added in the GMCP and clock-broadcast paths (e.g. in UserTelnet, Proxy::startClockBroadcaster/broadcastClockInfo); consider reducing verbosity or gating these behind an existing debug flag to avoid noisy logs in normal use.
- The season-change notification logic in MumeClock is duplicated across parseMumeTime, parseWeather, and parseClockTime; consider extracting a small helper (e.g. updateAndEmitSeasonChange(moment)) to centralize this behavior and reduce the risk of future inconsistencies.
- This PR introduces a number of configuration additions (textures, hotkeys, comms, visibility filters) that are orthogonal to GMCP/clock synchronization; if possible, consider splitting these into a separate change to keep the GMCP/clock work more focused and easier to review.
## Individual Comments
### Comment 1
<location> `src/proxy/proxy.cpp:915-924` </location>
<code_context>
+void Proxy::startClockBroadcaster()
</code_context>
<issue_to_address>
**issue (bug_risk):** Validate GMCP clock interval to avoid zero/negative or extremely small timer periods.
`gmcpBroadcastInterval` is used directly in `QTimer::setInterval`. If the config (now or in the future) allows 0/negative or very small values, this could produce undefined behaviour or an excessively fast timer. Please clamp the value to a sensible min (e.g. 100 ms or 2500 ms) and consider a max to prevent accidental flooding or effectively disabled timers.
</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
+915
to
+924
| void Proxy::startClockBroadcaster() | ||
| { | ||
| const auto &config = getConfig(); | ||
|
|
||
| qDebug() << "=== startClockBroadcaster called ==="; | ||
| qDebug() << " gmcpBroadcast config:" << config.mumeClock.gmcpBroadcast.get(); | ||
| qDebug() << " MUME_TIME enabled:" << isUserGmcpModuleEnabled(GmcpModuleTypeEnum::MUME_TIME); | ||
|
|
||
| // Only start if GMCP broadcasting is enabled and client supports MUME.Time module | ||
| if (!config.mumeClock.gmcpBroadcast.get() || !isUserGmcpModuleEnabled(GmcpModuleTypeEnum::MUME_TIME)) { |
There was a problem hiding this comment.
issue (bug_risk): Validate GMCP clock interval to avoid zero/negative or extremely small timer periods.
gmcpBroadcastInterval is used directly in QTimer::setInterval. If the config (now or in the future) allows 0/negative or very small values, this could produce undefined behaviour or an excessively fast timer. Please clamp the value to a sensible min (e.g. 100 ms or 2500 ms) and consider a max to prevent accidental flooding or effectively disabled timers.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
GMCP Improvements:
Clock Broadcasting:
GameObserver Integration:
Documentation:
This PR establishes the foundation for real-time GMCP features and enables better synchronization between MMapper and clients.
Summary by Sourcery
Enhance GMCP-based time synchronization and introduce richer UI/configuration options, including hotkey and communications settings.
New Features:
Enhancements: