Skip to content

Feat: Allow adding and deleting areas via GUI#16

Open
lychee-lynx wants to merge 4 commits into
swe-productivity:developmentfrom
lychee-lynx:mapper_area
Open

Feat: Allow adding and deleting areas via GUI#16
lychee-lynx wants to merge 4 commits into
swe-productivity:developmentfrom
lychee-lynx:mapper_area

Conversation

@lychee-lynx
Copy link
Copy Markdown

Brief overview of PR changes/additions

  • Adds architecture to easily allow adding and deleting new areas via GUI.
  • Includes some checks to not accidentally delete default area.

Motivation for adding to Mudlet

  • Make it easier to handle areas similar to rooms.

Other info (issues closed, discussion etc)

Attempts to close #8

@lychee-lynx lychee-lynx changed the title Feature: allow adding and deleting areas via GUI Feat: allow adding and deleting areas via GUI Jan 30, 2026
@lychee-lynx lychee-lynx changed the title Feat: allow adding and deleting areas via GUI Feat: Allow adding and deleting areas via GUI Jan 30, 2026
@vadi2
Copy link
Copy Markdown
Collaborator

vadi2 commented Mar 20, 2026

Thanks for working on this! Having GUI area management is a nice usability improvement. A few things I noticed:

Bug: map changes won't be saved. After adding or removing an area, mpMap->setUnsaved(func) needs to be called (similar to how the Lua addAreaName and deleteArea do it in TLuaInterpreterMapper.cpp). Without this, users can add/delete areas via the GUI, close Mudlet, and silently lose all their changes.

Bug: map display won't update after changes. The Lua equivalents also call mpMap->updateArea(id) after adding/removing areas (TLuaInterpreterMapper.cpp:1235 and :295), which triggers 2D/3D map refreshes and emits signal_areaChanged. Without this the map view won't update.

Should allow deleting areas that contain rooms. Currently the dialog refuses to delete any area that has rooms, telling the user to move or delete them first. That's a lot of manual work. The Lua deleteArea already handles this - TRoomDB::removeArea() deletes the rooms along with the area. The GUI should allow it too, just with a confirmation that mentions the room count (e.g. "This will also delete 42 rooms. Continue?").

Replace message boxes with inline feedback. The QMessageBox pop-ups for errors ("area already exists", "cannot delete default area") are disruptive. Consider inline UI feedback instead - e.g. a status label in the dialog, or disabling the remove button when the default area is selected. Pop-up confirmations for destructive actions (deleting an area with rooms) are fine, but error/validation feedback should be inline.

refreshAreas() is overly complex. It walks the parent widget hierarchy with findChildren<dlgMapper*>(), but since you already have mpMap, you can just use mpMap->mpMapper->updateAreaComboBox() directly - that's the same pointer the Lua API uses.

UI uses absolute positioning. The .ui file sets hardcoded x/y/width/height geometry for widgets instead of using Qt layouts (QVBoxLayout, QHBoxLayout). This means the dialog won't resize, won't adapt to different DPI/font sizes, and will break with longer translated strings. Please use layouts instead.

Unused includes. dlgConfigureAreas.cpp includes QElapsedTimer, QMenu, QPainter, QProgressDialog, and QListWidget, none of which are used. The coding standards ask to minimize includes.

Missing newline at end of dlgConfigureAreas.h. The project requires all files to end with a newline character.

Default area check could be simpler. checkDefaultArea() compares by name string, but the default area is always ID -1. Checking areaId == -1 is simpler and more robust (the default area name varies by translation). Also, currentAreaId() returns -1 when nothing is selected, which collides with the default area's actual ID - the areaId <= 0 guard in slot_removeArea happens to catch both but the overlap is confusing.

Name trimming inconsistency. slot_addArea() checks name.trimmed().isEmpty() but passes the untrimmed name to addArea(), so area names with leading/trailing spaces would be accepted. The Lua equivalent addAreaName trims the name before using it (TLuaInterpreterMapper.cpp:272).

@lychee-lynx
Copy link
Copy Markdown
Author

Thanks for the detailed review, I have addressed most of the feedback, please let me know your thoughts on this revision.
Also do let me know if you want me to rebase the commits instead of having them as fixes.

@lychee-lynx
Copy link
Copy Markdown
Author

Hi @vadi2, could you please review these changes.

@lychee-lynx
Copy link
Copy Markdown
Author

Hi @vadi2, gentle ping on this whenever you get a chance. I’ve addressed most of the earlier feedback and would appreciate another review when convenient. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature request: Mapper - Configure areas via GUI

2 participants