Add Visibility Filters for selective map element display #428
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
Visibility Filter System: - New VisibilityFilterWidget for controlling map element visibility - Selective show/hide for all infomark types (Generic, Herb, River, Place, etc.) - Connection visibility toggle - Dockable filter panel in main window Features: - Per-marker-class visibility controls (10 types) - Show All / Hide All quick actions - Connection display toggle - Real-time map updates when filters change - Persistent filter state in configuration Infomark Types: - Generic, Herb, River, Place, Mob, Comment, Road, Object, Action, Locality - Each type individually toggleable - Color-coded by marker class Configuration Infrastructure: - VisibilityFilter configuration struct with NamedConfig - isVisible() and setVisible() helper methods - Change callbacks for live map updates - Stored per-marker-class in config Integration: - Infomarks.cpp checks visibility before rendering - Connections.cpp respects connection visibility setting - MainWindow dock integration - No dependencies on other PRs This allows users to declutter the map by hiding marker types they're not currently interested in, improving map readability.
Reviewer's GuideAdds a configurable visibility filter system for map infomarks and connections, exposes it via a new dockable Visibility Filter panel, and wires configuration, rendering, and hotkey infrastructure so visibility changes update the map in real time and persist across sessions. Sequence diagram for toggling marker visibility via the Visibility Filter panelsequenceDiagram
actor User
participant VisibilityFilterWidget
participant Configuration as Config_VisibilityFilter
participant MainWindow
participant MapCanvas
participant Infomarks as MapCanvas_drawInfomark
User->>VisibilityFilterWidget: toggle Generic checkbox
VisibilityFilterWidget->>Config_VisibilityFilter: setVisible(InfomarkClassEnum_GENERIC, checked)
VisibilityFilterWidget-->>VisibilityFilterWidget: emit sig_visibilityChanged
VisibilityFilterWidget->>MainWindow: sig_visibilityChanged
MainWindow->>MapCanvas: infomarksChanged()
loop subsequent frame rendering
MapCanvas->>MapCanvas_drawInfomark: drawInfomark(batch, marker, offset)
MapCanvas_drawInfomark->>Config_VisibilityFilter: isVisible(marker.getClass())
alt marker class visible
MapCanvas_drawInfomark-->>MapCanvas_drawInfomark: render marker geometry
else marker class hidden
MapCanvas_drawInfomark-->>MapCanvas_drawInfomark: return without rendering
end
end
Sequence diagram for toggling connection visibilitysequenceDiagram
actor User
participant VisibilityFilterWidget
participant VisibilityFilter as Config_VisibilityFilter
participant MainWindow
participant MapCanvas
participant ConnectionMeshes
User->>VisibilityFilterWidget: toggle Connections checkbox
VisibilityFilterWidget->>Config_VisibilityFilter: setConnectionsVisible(checked)
VisibilityFilterWidget-->>VisibilityFilterWidget: emit sig_connectionsVisibilityChanged
VisibilityFilterWidget->>MainWindow: sig_connectionsVisibilityChanged
MainWindow->>MapCanvas: update()
loop when MapCanvas repaints
MapCanvas->>ConnectionMeshes: render(thisLayer, focusedLayer)
ConnectionMeshes->>Config_VisibilityFilter: isConnectionsVisible()
alt connections visible
ConnectionMeshes-->>ConnectionMeshes: draw batched connection geometry
else connections hidden
ConnectionMeshes-->>ConnectionMeshes: return early (skip rendering)
end
end
Class diagram for visibility filter configuration and UIclassDiagram
class Configuration {
+CanvasSettings canvas
+Hotkeys hotkeys
+CommsSettings comms
+MumeClockSettings mumeClock
}
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)
+VisibilityFilter()
}
class VisibilityFilterWidget {
+QCheckBox* m_genericCheckBox
+QCheckBox* m_herbCheckBox
+QCheckBox* m_riverCheckBox
+QCheckBox* m_placeCheckBox
+QCheckBox* m_mobCheckBox
+QCheckBox* m_commentCheckBox
+QCheckBox* m_roadCheckBox
+QCheckBox* m_objectCheckBox
+QCheckBox* m_actionCheckBox
+QCheckBox* m_localityCheckBox
+QCheckBox* m_connectionsCheckBox
+QPushButton* m_showAllButton
+QPushButton* m_hideAllButton
+ChangeMonitor_Lifetime m_changeMonitorLifetime
+VisibilityFilterWidget(QWidget* parent)
+void setupUI()
+void connectSignals()
+void setupChangeCallbacks()
+void updateCheckboxStates()
<<signal>> void sig_visibilityChanged()
<<signal>> void sig_connectionsVisibilityChanged()
}
class MapCanvas {
+void drawInfomark(InfomarksBatch batch, Infomark marker, Coordinate offset)
+void infomarksChanged()
+void update()
+void slot_reloadTextures()
+void slot_onSeasonChanged()
}
class ConnectionMeshes {
+void render(int thisLayer, int focusedLayer) const
}
class MainWindow {
+VisibilityFilterWidget* m_visibilityFilterWidget
+QDockWidget* m_dockDialogVisibleMarkers
+void createActions()
+void setupMenuBar()
+void wireConnections()
+void applyHotkeys()
+void registerGlobalShortcuts()
}
class Configuration_Hotkeys {
+NamedConfig_QString modeSelectMarkers
+NamedConfig_QString modeCreateMarker
+NamedConfig_QString panelRoom
+NamedConfig_QString panelAdventure
+NamedConfig_QString panelComms
+NamedConfig_QString viewRadialTransparency
+NamedConfig_QString panelLog
+NamedConfig_QString fileOpen
+NamedConfig_QString fileSave
+NamedConfig_QString fileReload
+NamedConfig_QString fileQuit
+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 panelClient
+NamedConfig_QString panelGroup
+NamedConfig_QString panelDescription
+NamedConfig_QString panelAdventure
+NamedConfig_QString panelComms
+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
+Configuration_Hotkeys()
}
class Configuration_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
}
class MumeClockSettings {
+int64_t startEpoch
+bool display
+NamedConfig_bool gmcpBroadcast
+NamedConfig_int gmcpBroadcastInterval
+MumeClockSettings()
}
Configuration --> CanvasSettings : has
Configuration --> Configuration_Hotkeys : has
Configuration --> Configuration_CommsSettings : has
Configuration --> MumeClockSettings : has
CanvasSettings --> Advanced : has
CanvasSettings --> VisibilityFilter : has
VisibilityFilterWidget ..> VisibilityFilter : uses
MainWindow --> VisibilityFilterWidget : owns
MainWindow --> MapCanvas : uses
MapCanvas ..> Configuration : reads
ConnectionMeshes ..> Configuration : reads
Configuration_Hotkeys ..> QAction : applied_via_MainWindow
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:
- The configuration layer now includes and depends on
map/infomark.hjust to useInfomarkClassEnum; consider inverting this dependency (e.g., move the visibility helpers closer toInfomarkor use a small enum-to-index helper) to avoid config → map coupling and reduce recompilation impact. VisibilityFilter::isVisible/setVisibleuse exhaustiveswitches without adefault, but silently fall through for unknown enum values; it may be safer to add adefaultthat either asserts/logs or clearly defines behavior when newInfomarkClassEnumvalues are added.MainWindow::applyHotkeysprints detailed debug output for every shortcut change, which might be noisy in normal use; consider wrapping theseqDebug()calls in a debug flag or compile-time guard so they can be disabled outside troubleshooting.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The configuration layer now includes and depends on `map/infomark.h` just to use `InfomarkClassEnum`; consider inverting this dependency (e.g., move the visibility helpers closer to `Infomark` or use a small enum-to-index helper) to avoid config → map coupling and reduce recompilation impact.
- `VisibilityFilter::isVisible`/`setVisible` use exhaustive `switch`es without a `default`, but silently fall through for unknown enum values; it may be safer to add a `default` that either asserts/logs or clearly defines behavior when new `InfomarkClassEnum` values are added.
- `MainWindow::applyHotkeys` prints detailed debug output for every shortcut change, which might be noisy in normal use; consider wrapping these `qDebug()` calls in a debug flag or compile-time guard so they can be disabled outside troubleshooting.
## Individual Comments
### Comment 1
<location> `src/configuration/configuration.cpp:56-65` </location>
<code_context>
}
}
+NODISCARD TextureSetEnum intToTextureSet(int value)
+{
+ switch (value) {
+ case 0:
+ return TextureSetEnum::CLASSIC;
+ case 1:
+ return TextureSetEnum::MODERN;
+ case 2:
+ return TextureSetEnum::CUSTOM;
+ default:
+ return TextureSetEnum::MODERN; // Default to Modern
+ }
+}
+
+NODISCARD int textureSetToInt(TextureSetEnum value)
+{
+ return static_cast<int>(value);
+}
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Persisting texture set as magic integers risks mismatch if TextureSetEnum changes
The `intToTextureSet` / `textureSetToInt` pair assumes `TextureSetEnum` is defined as `0/1/2` in that order. If the enum order/values change or a new value is inserted, existing configs will deserialize to the wrong texture set. To avoid this, either:
- serialize a stable identifier (e.g. explicit constants or string keys) and map from that, or
- have `intToTextureSet` compare against `static_cast<int>(TextureSetEnum::...)` instead of hard‑coded `0/1/2`.
This keeps deserialization aligned with future enum changes.
</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
+56
to
+65
| NODISCARD TextureSetEnum intToTextureSet(int value) | ||
| { | ||
| switch (value) { | ||
| case 0: | ||
| return TextureSetEnum::CLASSIC; | ||
| case 1: | ||
| return TextureSetEnum::MODERN; | ||
| case 2: | ||
| return TextureSetEnum::CUSTOM; | ||
| default: |
There was a problem hiding this comment.
issue (bug_risk): Persisting texture set as magic integers risks mismatch if TextureSetEnum changes
The intToTextureSet / textureSetToInt pair assumes TextureSetEnum is defined as 0/1/2 in that order. If the enum order/values change or a new value is inserted, existing configs will deserialize to the wrong texture set. To avoid this, either:
- serialize a stable identifier (e.g. explicit constants or string keys) and map from that, or
- have
intToTextureSetcompare againststatic_cast<int>(TextureSetEnum::...)instead of hard‑coded0/1/2.
This keeps deserialization aligned with future enum changes.
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.
Visibility Filter System:
Features:
Infomark Types:
Configuration Infrastructure:
Integration:
This allows users to declutter the map by hiding marker types they're not currently interested in, improving map readability.
Summary by Sourcery
Introduce configurable map visibility controls, enhanced graphics options, and new communications/hotkey infrastructure in the main window and configuration system.
New Features:
Enhancements:
Build:
Chores: