From 98b7effca96969f14d554d5c5a1211d1edda2b24 Mon Sep 17 00:00:00 2001 From: Vadim Peretokin Date: Mon, 22 Apr 2024 18:04:08 +0200 Subject: [PATCH 1/5] Add: automatically update map on all changes via API (#7187) #### Brief overview of PR changes/additions Automatically update map on all map changes via API while not losing performance. #### Motivation for adding to Mudlet Better user experience. #### Other info (issues closed, discussion etc) Alleviates the need for calling https://wiki.mudlet.org/w/Manual:Mapper_Functions#updateMap and fixes the inconsistency where some functions were updating the map automatically and others haven't. --------- Co-authored-by: Vadim Peretokin Co-authored-by: Kebap --- src/TLuaInterpreter.cpp | 10 --- src/TLuaInterpreterMapper.cpp | 145 ++++++++++++++++------------------ src/TMap.cpp | 33 +++++--- src/dlgPackageExporter.cpp | 3 +- 4 files changed, 92 insertions(+), 99 deletions(-) diff --git a/src/TLuaInterpreter.cpp b/src/TLuaInterpreter.cpp index 01c92e2acbf..ddf8969d972 100644 --- a/src/TLuaInterpreter.cpp +++ b/src/TLuaInterpreter.cpp @@ -811,16 +811,6 @@ int TLuaInterpreter::loadReplay(lua_State* L) } } -// Documentation: https://wiki.mudlet.org/w/Manual:Lua_Functions#updateMap -int TLuaInterpreter::updateMap(lua_State* L) -{ - const Host& host = getHostFromLua(L); - if (host.mpMap) { - host.mpMap->update(); - } - return 0; -} - // Documentation: https://wiki.mudlet.org/w/Manual:Lua_Functions#cut int TLuaInterpreter::cut(lua_State* L) { diff --git a/src/TLuaInterpreterMapper.cpp b/src/TLuaInterpreterMapper.cpp index f85dac490da..eb6170c02ba 100644 --- a/src/TLuaInterpreterMapper.cpp +++ b/src/TLuaInterpreterMapper.cpp @@ -27,6 +27,8 @@ // mapper-specific functions of TLuaInterpreter, split out separately // for convenience and to keep TLuaInterpreter.cpp size reasonable +// any call that that modifies the map visually needs to call host.mpMap->update(); + #include "TLuaInterpreter.h" #include "EAction.h" @@ -264,6 +266,7 @@ int TLuaInterpreter::deleteMapLabel(lua_State* L) int labelID = getVerifiedInt(L, __func__, 2, "labelID"); Host& host = getHostFromLua(L); host.mpMap->deleteMapLabel(area, labelID); + host.mpMap->update(); return 0; } // Documentation: https://wiki.mudlet.org/w/Manual:Lua_Functions#addAreaName @@ -286,10 +289,7 @@ int TLuaInterpreter::addAreaName(lua_State* L) // Note that adding an area name implicitly creates an underlying TArea instance lua_pushnumber(L, host.mpMap->mpRoomDB->addArea(name)); host.mpMap->setUnsaved(__func__); - - if (host.mpMap->mpMapper) { - host.mpMap->mpMapper->updateAreaComboBox(); - } + host.mpMap->update(); return 1; } @@ -500,12 +500,7 @@ int TLuaInterpreter::addCustomLine(lua_State* L) pR->calcRoomDimensions(); host.mpMap->setUnsaved(__func__); - - // Better refresh the 2D map to show the new line: - if (host.mpMap->mpMapper && host.mpMap->mpMapper->mp2dMap) { - host.mpMap->mpMapper->mp2dMap->mNewMoveAction = true; - host.mpMap->mpMapper->mp2dMap->update(); - } + host.mpMap->update(); lua_pushboolean(L, true); return 1; @@ -581,6 +576,7 @@ int TLuaInterpreter::addRoom(lua_State* L) if (added) { host.mpMap->setRoomArea(id, -1, false); host.mpMap->setUnsaved(__func__); + host.mpMap->update(); host.mpMap->mMapGraphNeedsUpdate = true; } return 1; @@ -608,6 +604,7 @@ int TLuaInterpreter::addSpecialExit(lua_State* L) } pR_from->setSpecialExit(toRoomID, dir); + host.mpMap->update(); lua_pushboolean(L, true); return 1; } @@ -702,19 +699,20 @@ int TLuaInterpreter::clearAreaUserDataItem(lua_State* L) // Documentation: https://wiki.mudlet.org/w/Manual:Lua_Functions#clearMapSelection int TLuaInterpreter::clearMapSelection(lua_State* L) { - Host* pHost = &getHostFromLua(L); - if (!pHost || !pHost->mpMap || !pHost->mpMap->mpMapper || !pHost->mpMap->mpMapper->mp2dMap) { + const Host& host = getHostFromLua(L); + if (!host.mpMap || !host.mpMap->mpMapper || !host.mpMap->mpMapper->mp2dMap) { return warnArgumentValue(L, __func__, "no map present or loaded"); } - if (pHost->mpMap->mpMapper->mp2dMap->mMultiSelection) { + if (host.mpMap->mpMapper->mp2dMap->mMultiSelection) { return warnArgumentValue(L, __func__, "rooms are being selected right now and cannot be stopped at this point"); } - if (pHost->mpMap->mpMapper->mp2dMap->mMultiSelectionSet.isEmpty()) { + if (host.mpMap->mpMapper->mp2dMap->mMultiSelectionSet.isEmpty()) { lua_pushboolean(L, false); } else { - pHost->mpMap->mpMapper->mp2dMap->clearSelection(); + host.mpMap->mpMapper->mp2dMap->clearSelection(); lua_pushboolean(L, true); } + host.mpMap->update(); return 1; } @@ -772,6 +770,7 @@ int TLuaInterpreter::clearRoomUserData(lua_State* L) } else { lua_pushboolean(L, false); } + host.mpMap->update(); return 1; } @@ -802,6 +801,7 @@ int TLuaInterpreter::clearRoomUserDataItem(lua_State* L) } else { lua_pushboolean(L, false); } + host.mpMap->update(); return 1; } @@ -814,10 +814,10 @@ int TLuaInterpreter::clearSpecialExits(lua_State* L) if (pR) { pR->clearSpecialExits(); } + host.mpMap->update(); return 0; } - - +// Documentation: https://wiki.mudlet.org/w/Manual:Lua_Functions#closeMapWidget int TLuaInterpreter::closeMapWidget(lua_State* L) { Host& host = getHostFromLua(L); @@ -926,7 +926,7 @@ int TLuaInterpreter::connectExitStub(lua_State* L) } host.mpMap->mMapGraphNeedsUpdate = true; - // equivalent to a call to updateMap(L): + host.mpMap->update(); lua_pushboolean(L, true); return 1; @@ -981,6 +981,7 @@ int TLuaInterpreter::createMapLabel(lua_State* L) const Host& host = getHostFromLua(L); lua_pushinteger(L, host.mpMap->createMapLabel(area, text, posx, posy, posz, QColor(fgr, fgg, fgb, foregroundTransparency), QColor(bgr, bgg, bgb, backgroundTransparency), showOnTop, noScaling, temporary, zoom, fontSize, fontName)); + host.mpMap->update(); return 1; } @@ -1004,6 +1005,7 @@ int TLuaInterpreter::createMapImageLabel(lua_State* L) const Host& host = getHostFromLua(L); lua_pushinteger(L, host.mpMap->createMapImageLabel(area, imagePathFileName, posx, posy, posz, width, height, zoom, showOnTop, temporary)); + host.mpMap->update(); return 1; } @@ -1114,13 +1116,14 @@ int TLuaInterpreter::deleteArea(lua_State* L) host.mpMap->mpMapper->updateAreaComboBox(); } host.mpMap->setUnsaved(__func__); + host.mpMap->update(); host.mpMap->mMapGraphNeedsUpdate = true; } lua_pushboolean(L, result); return 1; } - +// Documentation: https://wiki.mudlet.org/w/Manual:Lua_Functions#deleteMap int TLuaInterpreter::deleteMap(lua_State* L) { const Host& host = getHostFromLua(L); @@ -1134,8 +1137,7 @@ int TLuaInterpreter::deleteMap(lua_State* L) host.mpMap->mapClear(); - // Also cause any displayed map to reset: - updateMap(L); + host.mpMap->update(); lua_pushboolean(L, true); return 1; @@ -1151,6 +1153,7 @@ int TLuaInterpreter::deleteRoom(lua_State* L) const Host& host = getHostFromLua(L); lua_pushboolean(L, host.mpMap->mpRoomDB->removeRoom(id)); host.mpMap->setUnsaved(__func__); + host.mpMap->update(); return 1; } @@ -1163,6 +1166,7 @@ int TLuaInterpreter::disableMapInfo(lua_State* L) return warnArgumentValue(L, __func__, qsl("map info '%1' does not exist").arg(name)); } + host.mpMap->update(); lua_pushboolean(L, true); return 1; } @@ -1175,6 +1179,8 @@ int TLuaInterpreter::enableMapInfo(lua_State* L) if (!host.mpMap->mMapInfoContributorManager->enableContributor(name)) { return warnArgumentValue(L, __func__, qsl("map info '%1' does not exist").arg(name)); } + + host.mpMap->update(); lua_pushboolean(L, true); return 1; } @@ -2299,6 +2305,7 @@ int TLuaInterpreter::gotoRoom(lua_State* L) } host.startSpeedWalk(); lua_pushboolean(L, true); + host.mpMap->update(); return 1; } @@ -2370,11 +2377,7 @@ int TLuaInterpreter::highlightRoom(lua_State* L) pR->highlightColor2 = bg; pR->highlightRadius = radius; - if (host.mpMap->mpMapper) { - if (host.mpMap->mpMapper->mp2dMap) { - host.mpMap->mpMapper->mp2dMap->update(); - } - } + host.mpMap->update(); lua_pushboolean(L, true); } else { lua_pushboolean(L, false); @@ -2390,6 +2393,8 @@ int TLuaInterpreter::killMapInfo(lua_State* L) if (!host.mpMap->mMapInfoContributorManager->removeContributor(name)) { return warnArgumentValue(L, __func__, qsl("map info '%1' does not exist").arg(name)); } + + host.mpMap->update(); lua_pushboolean(L, true); return 1; } @@ -2474,6 +2479,7 @@ int TLuaInterpreter::lockExit(lua_State* L) if (pR) { pR->setExitLock(dir, b); host.mpMap->setUnsaved(__func__); + host.mpMap->update(); host.mpMap->mMapGraphNeedsUpdate = true; } return 0; @@ -2489,6 +2495,7 @@ int TLuaInterpreter::lockRoom(lua_State* L) if (pR) { pR->isLocked = b; host.mpMap->setUnsaved(__func__); + host.mpMap->update(); host.mpMap->mMapGraphNeedsUpdate = true; lua_pushboolean(L, true); } else { @@ -2520,6 +2527,7 @@ int TLuaInterpreter::lockSpecialExit(lua_State* L) lua_pushboolean(L, true); host.mpMap->setUnsaved(__func__); + host.mpMap->update(); host.mpMap->mMapGraphNeedsUpdate = true; return 1; } @@ -2620,6 +2628,7 @@ int TLuaInterpreter::registerMapInfo(lua_State* L) return MapInfoProperties{ isBold, isItalic, text, color }; }); + host.mpMap->update(); lua_pushboolean(L, true); return 1; } @@ -2655,11 +2664,8 @@ int TLuaInterpreter::removeCustomLine(lua_State* L) // Need to update the TRoom {min|max}_{x|y} settings as they are used during // the painting process: pR->calcRoomDimensions(); - // Better refresh the 2D map to show the new line: - if (host.mpMap->mpMapper && host.mpMap->mpMapper->mp2dMap) { - host.mpMap->mpMapper->mp2dMap->mNewMoveAction = true; - host.mpMap->mpMapper->mp2dMap->update(); - } + host.mpMap->update(); + lua_pushboolean(L, true); return 1; } @@ -2746,6 +2752,7 @@ int TLuaInterpreter::removeSpecialExit(lua_State* L) "the special exit name/command '%1' does not exist in exit roomID %2").arg(dir, QString::number(fromRoomID))); } pR->setSpecialExit(-1, dir); + host.mpMap->update(); lua_pushboolean(L, true); return 1; } @@ -2764,17 +2771,7 @@ int TLuaInterpreter::resetRoomArea(lua_State* L) } const bool result = host.mpMap->setRoomArea(id, -1, false); if (result) { - // As a successful result WILL change the area a room is in then the map - // should be updated. The GUI code that modifies room(s) areas already - // includes such a call to update the mapper. - if (host.mpMap->mpMapper) { - host.mpMap->mpMapper->mp2dMap->update(); - } -#if defined(INCLUDE_3DMAPPER) - if (host.mpMap->mpM) { - host.mpMap->mpM->update(); - } -#endif + host.mpMap->update(); } lua_pushboolean(L, result); return 1; @@ -3197,12 +3194,12 @@ int TLuaInterpreter::setAreaName(lua_State* L) const bool result = host.mpMap->mpRoomDB->setAreaName(id, newName); if (result) { host.mpMap->setUnsaved(__func__); + host.mpMap->update(); if (host.mpMap->mpMapper) { host.mpMap->mpMapper->updateAreaComboBox(); if (isCurrentAreaRenamed) { host.mpMap->mpMapper->comboBox_showArea->setCurrentText(newName); } - updateMap(L); } } lua_pushboolean(L, result); @@ -3230,6 +3227,7 @@ int TLuaInterpreter::setAreaUserData(lua_State* L) } pA->mUserData[key] = value; host.mpMap->setUnsaved(__func__); + host.mpMap->update(); lua_pushboolean(L, true); return 1; } @@ -3245,6 +3243,7 @@ int TLuaInterpreter::setCustomEnvColor(lua_State* L) const Host& host = getHostFromLua(L); host.mpMap->mCustomEnvColors[id] = QColor(r, g, b, alpha); host.mpMap->setUnsaved(__func__); + host.mpMap->update(); return 0; } @@ -3313,9 +3312,7 @@ int TLuaInterpreter::setDoor(lua_State* L) const bool result = pR->setDoor(exitCmd, doorStatus); if (result) { host.mpMap->setUnsaved(__func__); - if (host.mpMap->mpMapper && host.mpMap->mpMapper->mp2dMap) { - host.mpMap->mpMapper->mp2dMap->update(); - } + host.mpMap->update(); } lua_pushboolean(L, result); return 1; @@ -3336,6 +3333,7 @@ int TLuaInterpreter::setExit(lua_State* L) const Host& host = getHostFromLua(L); lua_pushboolean(L, host.mpMap->setExit(from, to, dir)); host.mpMap->mMapGraphNeedsUpdate = true; + host.mpMap->update(); return 1; } @@ -3366,6 +3364,7 @@ int TLuaInterpreter::setExitStub(lua_State* L) return lua_error(L); } pR->setExitStub(dir, status); + host.mpMap->update(); return 0; } @@ -3398,6 +3397,7 @@ int TLuaInterpreter::setExitWeight(lua_State* L) pR->setExitWeight(direction, weight); lua_pushboolean(L, true); + host.mpMap->update(); return 1; } @@ -3414,15 +3414,7 @@ int TLuaInterpreter::setGridMode(lua_State* L) } else { pA->gridMode = gridMode; pA->calcSpan(); - if (host.mpMap->mpMapper) { - if (host.mpMap->mpMapper->mp2dMap) { - // Not needed IMHO - Slysven - // host.mpMap->mpMapper->mp2dMap->init(); - // cout << "NEW GRID MAP: init" << endl; - // But this is: - host.mpMap->mpMapper->update(); - } - } + host.mpMap->update(); } host.mpMap->setUnsaved(__func__); lua_pushboolean(L, true); @@ -3468,6 +3460,7 @@ int TLuaInterpreter::setMapZoom(lua_State* L) } lua_pushboolean(L, true); + host.mpMap->update(); return 1; } @@ -3518,17 +3511,7 @@ int TLuaInterpreter::setRoomArea(lua_State* L) // appear in the TRoomDB::areaNamesMap... const bool result = host.mpMap->setRoomArea(id, areaId, false); if (result) { - // As a successful result WILL change the area a room is in then the map - // should be updated. The GUI code that modifies room(s) areas already - // includes such a call to update the mapper. - if (host.mpMap->mpMapper) { - host.mpMap->mpMapper->mp2dMap->update(); - } -#if defined(INCLUDE_3DMAPPER) - if (host.mpMap->mpM) { - host.mpMap->mpM->update(); - } -#endif + host.mpMap->update(); } lua_pushboolean(L, result); return 1; @@ -3555,6 +3538,7 @@ int TLuaInterpreter::setRoomChar(lua_State* L) pR->mSymbol = symbol.normalized(QString::NormalizationForm_C, QChar::Unicode_10_0); } host.mpMap->setUnsaved(__func__); + host.mpMap->update(); lua_pushboolean(L, true); return 1; } @@ -3586,10 +3570,8 @@ int TLuaInterpreter::setRoomCharColor(lua_State* L) } pR->mSymbolColor = QColor(r, g, b); - if (host.mpMap->mpMapper && host.mpMap->mpMapper->mp2dMap) { - host.mpMap->mpMapper->mp2dMap->update(); - } host.mpMap->setUnsaved(__func__); + host.mpMap->update(); lua_pushboolean(L, true); return 1; } @@ -3603,6 +3585,7 @@ int TLuaInterpreter::setRoomCoordinates(lua_State* L) const int z = getVerifiedInt(L, __func__, 4, "z"); const Host& host = getHostFromLua(L); lua_pushboolean(L, host.mpMap->setRoomCoordinates(id, x, y, z)); + host.mpMap->update(); return 1; } @@ -3619,6 +3602,7 @@ int TLuaInterpreter::setRoomEnv(lua_State* L) } pR->environment = env; host.mpMap->setUnsaved(__func__); + host.mpMap->update(); lua_pushboolean(L, true); return 1; } @@ -3657,7 +3641,7 @@ int TLuaInterpreter::setRoomName(lua_State* L) } pR->name = name; host.mpMap->setUnsaved(__func__); - updateMap(L); + host.mpMap->update(); lua_pushboolean(L, true); return 1; } @@ -3681,6 +3665,7 @@ int TLuaInterpreter::setRoomUserData(lua_State* L) } pR->userData[key] = value; host.mpMap->setUnsaved(__func__); + host.mpMap->update(); lua_pushboolean(L, true); return 1; } @@ -3699,6 +3684,7 @@ int TLuaInterpreter::setRoomWeight(lua_State* L) pR->setWeight(w); host.mpMap->setUnsaved(__func__); + host.mpMap->update(); host.mpMap->mMapGraphNeedsUpdate = true; lua_pushboolean(L, true); return 1; @@ -3713,11 +3699,7 @@ int TLuaInterpreter::unHighlightRoom(lua_State* L) TRoom* pR = host.mpMap->mpRoomDB->getRoom(id); if (pR) { pR->highlight = false; - if (host.mpMap) { - if (host.mpMap->mpMapper) { - host.mpMap->mpMapper->mp2dMap->update(); - } - } + host.mpMap->update(); lua_pushboolean(L, true); } else { lua_pushboolean(L, false); @@ -3738,11 +3720,18 @@ int TLuaInterpreter::unsetRoomCharColor(lua_State* L) // Reset it to the default (and invalid) QColor: pR->mSymbolColor = {}; - if (host.mpMap->mpMapper && host.mpMap->mpMapper->mp2dMap) { - host.mpMap->mpMapper->mp2dMap->update(); - } host.mpMap->setUnsaved(__func__); + host.mpMap->update(); lua_pushboolean(L, true); return 1; } +// Documentation: https://wiki.mudlet.org/w/Manual:Lua_Functions#updateMap +int TLuaInterpreter::updateMap(lua_State* L) +{ + const Host& host = getHostFromLua(L); + if (host.mpMap) { + host.mpMap->update(); + } + return 0; +} diff --git a/src/TMap.cpp b/src/TMap.cpp index 3615764914d..b291ae44d04 100644 --- a/src/TMap.cpp +++ b/src/TMap.cpp @@ -3405,21 +3405,34 @@ bool TMap::incrementJsonProgressDialog(const bool isExportNotImport, const bool return mpProgressDialog->wasCanceled(); } +/** + * Update the the 2D and 3D map visually. + * + * It ensures debouncing internally to ensure that bulk calls are efficient. + */ void TMap::update() { + static bool debounce; + if (!debounce) { + debounce = true; + QTimer::singleShot(0, this, [this]() { + debounce = false; + #if defined(INCLUDE_3DMAPPER) - if (mpM) { - mpM->update(); - } + if (mpM) { + mpM->update(); + } #endif - if (mpMapper) { - mpMapper->checkBox_showRoomNames->setVisible(getRoomNamesPresent()); - mpMapper->checkBox_showRoomNames->setChecked(getRoomNamesShown()); + if (mpMapper) { + mpMapper->checkBox_showRoomNames->setVisible(getRoomNamesPresent()); + mpMapper->checkBox_showRoomNames->setChecked(getRoomNamesShown()); - if (mpMapper->mp2dMap) { - mpMapper->mp2dMap->mNewMoveAction = true; - mpMapper->mp2dMap->update(); - } + if (mpMapper->mp2dMap) { + mpMapper->mp2dMap->mNewMoveAction = true; + mpMapper->mp2dMap->update(); + } + } + }); } } diff --git a/src/dlgPackageExporter.cpp b/src/dlgPackageExporter.cpp index 6931de709b5..d756520b477 100644 --- a/src/dlgPackageExporter.cpp +++ b/src/dlgPackageExporter.cpp @@ -1464,6 +1464,8 @@ void dlgPackageExporter::slot_recountItems(QTreeWidgetItem *item) if (!debounce) { debounce = true; QTimer::singleShot(0, this, [this]() { + debounce = false; + const int itemsToExport = countCheckedItems(); if (itemsToExport) { //: This is the text shown at the top of a groupbox when there is %n (one or more) items to export in the Package exporter dialogue; the initial (and when there is no items selected) is a separate text. @@ -1472,7 +1474,6 @@ void dlgPackageExporter::slot_recountItems(QTreeWidgetItem *item) //: This is the text shown at the top of a groupbox initially and when there is NO items to export in the Package exporter dialogue. mpSelectionText->setTitle(tr("Select what to export")); } - debounce = false; }); } } From 77ba03c49ff44a3bec743350d1578aee94e78a60 Mon Sep 17 00:00:00 2001 From: Vadim Peretokin Date: Thu, 25 Apr 2024 17:25:07 +0200 Subject: [PATCH 2/5] Fix a crash when double-clicking on a word to select it (#7123) #### Brief overview of PR changes/additions Fix a crash as shown by the stacktrace https://pastebin.com/GHbzFZNs #### Motivation for adding to Mudlet Bugfix #### Other info (issues closed, discussion etc) Fix written with the help of chatgpt, here is the conversation for anyone else who'd like to copy this: https://chat.openai.com/share/cf97d3d8-e419-4878-9a2b-87409195340a --------- Co-authored-by: Kebap Co-authored-by: Vadim Peretokin --- src/TTextEdit.cpp | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/src/TTextEdit.cpp b/src/TTextEdit.cpp index 8f7edbf6ba4..d3209d57d86 100644 --- a/src/TTextEdit.cpp +++ b/src/TTextEdit.cpp @@ -1104,10 +1104,20 @@ void TTextEdit::expandSelectionToWords() { int yind = mPA.y(); int xind = mPA.x(); - for (; xind >= 0; --xind) { - if (mpBuffer->lineBuffer.at(yind).at(xind) == QChar::Space - || mpHost->mDoubleClickIgnore.contains(mpBuffer->lineBuffer.at(yind).at(xind))) { - break; + + // Check if yind is within the valid range of lineBuffer + if (yind >= 0 && yind < static_cast(mpBuffer->lineBuffer.size())) { + for (; xind >= 0; --xind) { + // Ensure xind is within the valid range for the current line + if (xind < static_cast(mpBuffer->lineBuffer.at(yind).size())) { + const QChar currentChar = mpBuffer->lineBuffer.at(yind).at(xind); + if (currentChar == QChar::Space + || mpHost->mDoubleClickIgnore.contains(currentChar)) { + break; + } + } else { + break; // xind is out of bounds, break the loop + } } } mDragStart.setX(xind + 1); @@ -1115,16 +1125,23 @@ void TTextEdit::expandSelectionToWords() yind = mPB.y(); xind = mPB.x(); - for (; xind < static_cast(mpBuffer->lineBuffer.at(yind).size()); ++xind) { - if (mpBuffer->lineBuffer.at(yind).at(xind) == QChar::Space - || mpHost->mDoubleClickIgnore.contains(mpBuffer->lineBuffer.at(yind).at(xind))) { - break; + + // Repeat the check for yind and xind for the second part + if (yind >= 0 && yind < static_cast(mpBuffer->lineBuffer.size())) { + for (; xind < static_cast(mpBuffer->lineBuffer.at(yind).size()); ++xind) { + const QChar currentChar = mpBuffer->lineBuffer.at(yind).at(xind); + if (currentChar == QChar::Space + || mpHost->mDoubleClickIgnore.contains(currentChar)) { + break; + } } } mDragSelectionEnd.setX(xind - 1); mPB.setX(xind - 1); } + + void TTextEdit::expandSelectionToLine(int y) { if (!(y < mpBuffer->lineBuffer.size())) { From 350404b16996cb12b8e746516a66539ea8f6516a Mon Sep 17 00:00:00 2001 From: Stephen Lyons Date: Fri, 26 Apr 2024 02:40:37 +0100 Subject: [PATCH 3/5] Infrastructure: refactor and simplify profile closing (#7203) #### Brief overview of PR changes/additions An attempt to improve the process of closing multiple profiles, including handling the case where a closure of the whole Mudlet application is cancelled part way through by the end-user selecting the "Cancel" option when asked if they want to save a profile or not. #### Motivation for adding to Mudlet When multi-playing carrying out the above action (cancelling part way through) can leave widgets and dialogues for profiles open even though the underlying `Host` instance has gone away. This can easily lead to fatal segmentation faults when those widgets or dialogues try to reference something that is no longer present. Additionally the code prior to this PR had multiple execution paths when "closing" a profile leading to some of them not doing all the things they needed to - hence the issues just listed. This PR tries to rationalise the execution path and reduce the amount of duplication so that it is easier to follow and amend in the future. #### Other info (issues closed, discussion etc) A `closeEvent(QEvent*)` for the `TMainConsole` class has been created to override the `TConsole` one as it is the only console that has to be concerned with saving the profile. The way the code was structured meant that the destruction of `TDockWidget`s would sometimes hit a `"TConsole::closeEvent(QCloseEvent*) INFO - closing a UserWindow but the TDockWidget pointer was not found to be removed...";` message - this was because the relevant `TDockWidget` had already been removed from the `(QMap) TMainConsole::mDockWidgetMap` before the `TConsole::closeEvent(...)` was executed. The `(bool) Host::mIsGoingDown` flag was unnecessary and seemed to duplicate the `(bool) Host::mIsClosingDown` one - OTOH I did need a new `(bool) Host::mForcedClose` flag to signal that the main application was being forcibly closed and that everything should be saved whether the user had set the "auto-save on exit option" or not. The `(bool) TConsole::mUserAgreedToCloseConsole` flag has been renamed and moved to be `(bool) TMainConsole::mEnableClose` instead. The actions to close down widgets and dialogues associated with a profile have been moved to the new `(void) Host::closeChildren` method which is called by ~~both the `(void) Host::forceClose()` method - which cannot be aborted and~~ the `(bool) Host::requestClose()` which can be aborted, and returns `false` if that *has* happened. ***Edit: the forceClose() method now just sets a flag which forces the requestClose() to not ask questions and just save things...*** The `HostManager::deleteHost(const QString&)` return value was never used so I changed it to be a `void` method. Note that this is the method that is actually responsible for destroying the `Host` instance for each profile. The `(static void) TDebug::removeHost(Host*)` method needs the name of the profile that is "going away" but because it is called from the `Host` destructor I ran into a copy of seg. faults from: * calling `Host::getName()` from there * calling `TBuffer::log(...)` indirectly from there because that dereferences `Host::mpConsole` to check if the `TBuffer` is the one for the main console and there wasn't a valid `Host` at that point. The process of iterating through all the open profiles to close them down in `(void) mudlet::closeEvent(QCloseEvent*)` is more tricky than it seems because the `for (auto pHost : mHostManager)` loop blows up if `mHostManager` is modified in anyway during the iteration. Instead I found I had to note down the profiles that have been closed and only destroy them after the "Do you wanna save this profile?" loop has been gone through for as many as get closed. Because the package exporter widget is parented to the widget that is effectively the `QMainWindow` for the mudlet instance rather than the `TMainConsole` for it's profile, it was not getting closed/destroyed should it have been open when the profile it was associated with get closed but the Mudlet application stay running. As the `Host` class is NOT derived from a `QWidget` the dialogue could not be made a child of it; instead I've arranged for the `QObject::destroyed` signal of the `Host` to cancel and close it instead. --------- Signed-off-by: Stephen Lyons Co-authored-by: Kebap Co-authored-by: Vadim Peretokin Co-authored-by: Mike Conley --- src/Host.cpp | 164 +++++++++++++++++++++---------- src/Host.h | 16 ++- src/HostManager.cpp | 14 +-- src/HostManager.h | 4 +- src/TBuffer.cpp | 5 + src/TConsole.cpp | 128 ++++-------------------- src/TConsole.h | 1 - src/TDebug.cpp | 14 +-- src/TDebug.h | 4 +- src/TLuaInterpreter.cpp | 2 +- src/TMainConsole.cpp | 110 +++++++++++++++++++++ src/TMainConsole.h | 4 +- src/ctelnet.cpp | 2 +- src/dlgPackageExporter.cpp | 7 ++ src/mudlet.cpp | 194 +++++++++++-------------------------- src/mudlet.h | 1 + 16 files changed, 346 insertions(+), 324 deletions(-) diff --git a/src/Host.cpp b/src/Host.cpp index 1c8a1ea1275..c170369e92b 100644 --- a/src/Host.cpp +++ b/src/Host.cpp @@ -236,7 +236,6 @@ Host::Host(int port, const QString& hostname, const QString& login, const QStrin , mSslIgnoreAll(false) , mUseProxy(false) , mProxyPort(0) -, mIsGoingDown(false) , mIsProfileLoadingSequence(false) , mNoAntiAlias(false) , mpEditorDialog(nullptr) @@ -342,7 +341,6 @@ Host::Host(int port, const QString& hostname, const QString& login, const QStrin , mGifTracker() , mHostID(id) , mHostName(hostname) -, mIsClosingDown(false) , mLogin(login) , mPass(pass) , mPort(port) @@ -364,7 +362,7 @@ Host::Host(int port, const QString& hostname, const QString& login, const QStrin , mDebugShowAllProblemCodepoints(false) , mCompactInputLine(false) { - TDebug::addHost(this); + TDebug::addHost(this, mHostName); // The "autolog" sentinel file controls whether logging the game's text as // plain text or HTML is immediately resumed on profile loading. Do not @@ -477,11 +475,114 @@ Host::~Host() if (mpDockableMapWidget) { mpDockableMapWidget->deleteLater(); } - mIsGoingDown = true; - mIsClosingDown = true; mErrorLogStream.flush(); mErrorLogFile.close(); - TDebug::removeHost(this); + // Since this is a destructor, it's risky to rely on member variables within the destructor itself. + // To avoid this, we can pass the profile name as an argument instead of accessing it + // directly as a member variable. This ensures the destructor doesn't depend on the + // object's state being valid. + + TDebug::removeHost(this, mHostName); +} + +void Host::forceClose() +{ + if (!mpConsole) { + // The main console has gone away so we must already be dying + return; + } + + mForcedClose = true; + postMessage(tr("[ ALERT ] - This profile will now save and close.")); + // Ensure the above is displayed before proceeding... + qApp->processEvents(); + + // Because we have set the mForcedClose flag above a requestClose() + // afterwards WILL succeed +} + +// Returns true if we are closing down +bool Host::requestClose() +{ + if (!mpConsole) { + // The main console has gone away so we must already be dying + return true; + } + + // This call ends up at the (void) TMainConsole::closeEvent(...) and causes + // the close() method called here to return a true if the event was + // accepted: + if (!mpConsole->close()) { + // Nope the user doesn't want this to close - and it won't have set its + // mEnableClose flag: + return false; + } + + // The above will have initiated a save of the profile (and its map) if it + // got a true returned from the TMainConsole::close() call. + + // Get rid of any dialogs we might have open: + closeChildren(); + + // This time this will succeed as mEnableClose is set: + mpConsole->close(); + return true; +} + +void Host::closeChildren() +{ + mIsClosingDown = true; + const auto hostToolBarMap = getActionUnit()->getToolBarList(); + // disconnect before removing objects from memory as sysDisconnectionEvent needs that stuff. + if (mSslTsl) { + mTelnet.abortConnection(); + } else { + mTelnet.disconnectIt(); + } + + stopAllTriggers(); + + if (mpEditorDialog) { + mpEditorDialog->setAttribute(Qt::WA_DeleteOnClose); + mpEditorDialog->close(); + mpEditorDialog = nullptr; + } + + for (const QString& consoleName : mpConsole->mSubConsoleMap.keys()) { + // Only user-windows will be in this map: + auto pD = mpConsole->mDockWidgetMap.value(consoleName); + // All User TConsole's will be in this map: + auto pC = mpConsole->mSubConsoleMap.value(consoleName); + if (pD) { + // This undocks the widget + mudlet::self()->removeDockWidget(pD); + } + + // This will remove both what pD and pC point at from their respective + // QMaps: + pC->close(); + } + + if (mpNotePad) { + mpNotePad->save(); + mpNotePad->setAttribute(Qt::WA_DeleteOnClose); + mpNotePad->close(); + mpNotePad = nullptr; + } + + for (TToolBar* pTB : hostToolBarMap) { + if (pTB) { + pTB->setAttribute(Qt::WA_DeleteOnClose); + pTB->deleteLater(); + } + } + + // close IRC client window if it is open. + if (mpDlgIRC) { + mpDlgIRC->setAttribute(Qt::WA_DeleteOnClose); + mpDlgIRC->deleteLater(); + mpDlgIRC = nullptr; + } } void Host::loadMap() @@ -827,7 +928,7 @@ std::tuple Host::saveProfile(const QString& saveFolder, } if (mIsProfileLoadingSequence) { - //If we're inside of profile loading sequence modules might not be loaded yet, thus we can accidetnally clear their contents + //If we're inside of profile loading sequence modules might not be loaded yet, thus we can accidentally clear their contents return {false, filename_xml, qsl("profile loading is in progress")}; } @@ -842,8 +943,9 @@ std::tuple Host::saveProfile(const QString& saveFolder, if (saveFolder.isEmpty() && saveName.isEmpty()) { // This is likely to be the save as the profile is closed - qDebug().noquote().nospace() << "Host::saveProfile(...) INFO - called with no saveFolder or saveName arguments " - "so assuming it is a end of session save and the TCommandLines' histories need saving..."; + qDebug().noquote().nospace() << "Host::saveProfile(...) INFO - called with no saveFolder or saveName arguments for profile '" + << mHostName + << "' so assuming it is an end of session save and the TCommandLines' histories need saving..."; emit signal_saveCommandLinesHistory(); } @@ -1675,16 +1777,6 @@ bool Host::killTrigger(const QString& name) return mTriggerUnit.killTrigger(name); } -void Host::closingDown() -{ - mIsClosingDown = true; -} - -bool Host::isClosingDown() -{ - return mIsClosingDown; -} - std::pair Host::installPackage(const QString& fileName, int module) { // As the pointer to dialog is only used now WITHIN this method and this @@ -3206,40 +3298,6 @@ void Host::hideMudletsVariables() } } -void Host::close() -{ - // disconnect before removing objects from memory as sysDisconnectionEvent needs that stuff. - if (mSslTsl) { - mTelnet.abortConnection(); - } else { - mTelnet.disconnectIt(); - } - - // close script editor - if (mpEditorDialog) { - mpEditorDialog->setAttribute(Qt::WA_DeleteOnClose); - mpEditorDialog->close(); - mpEditorDialog = nullptr; - } - // close notepad - if (mpNotePad) { - mpNotePad->save(); - mpNotePad->setAttribute(Qt::WA_DeleteOnClose); - mpNotePad->close(); - mpNotePad = nullptr; - } - // close IRC client window - if (mpDlgIRC) { - mpDlgIRC->setAttribute(Qt::WA_DeleteOnClose); - mpDlgIRC->deleteLater(); - mpDlgIRC = nullptr; - } - if (mpConsole) { - mpConsole->close(); - mpConsole = nullptr; - } -} - bool Host::createBuffer(const QString& name) { if (!mpConsole) { diff --git a/src/Host.h b/src/Host.h index cd909cea9a4..acafdf11224 100644 --- a/src/Host.h +++ b/src/Host.h @@ -212,8 +212,11 @@ class Host : public QObject bool getLargeAreaExitArrows() const { return mLargeAreaExitArrows; } void setLargeAreaExitArrows(const bool); - void closingDown(); - bool isClosingDown(); + void forceClose(); + bool isClosingDown() const { return mIsClosingDown; } + bool isClosingForced() const { return mForcedClose; } + bool requestClose(); + unsigned int assemblePath(); bool checkForMappingScript(); bool checkForCustomSpeedwalk(); @@ -355,7 +358,6 @@ class Host : public QObject void setCompactInputLine(const bool state); bool getCompactInputLine() const { return mCompactInputLine; } QPointer findConsole(QString name); - void close(); QPair getLines(const QString& windowName, const int lineFrom, const int lineTo); std::pair openWindow(const QString& name, bool loadLayout, bool autoDock, const QString& area); @@ -474,7 +476,6 @@ class Host : public QObject QString mProxyUsername; QString mProxyPassword; - bool mIsGoingDown; // Used to force the test compilation of the scripts for TActions ("Buttons") // that are pushdown buttons that run when they are "pushed down" during // loading even though the buttons start out with themselves NOT being @@ -746,6 +747,7 @@ private slots: void autoSaveMap(); QString sanitizePackageName(const QString packageName) const; TCommandLine* activeCommandLine(); + void closeChildren(); QFont mDisplayFont; @@ -769,7 +771,7 @@ private slots: int mHostID; QString mHostName; QString mDiscordGameName; // Discord self-reported game name - bool mIsClosingDown; + bool mIsClosingDown = false; QString mLine; QString mLogin; @@ -910,6 +912,10 @@ private slots: // Whether to display each item's ID number in the editor: bool mShowIDsInEditor = false; + + // Set when the mudlet singleton demands that we close - used to force an + // attempt to save the profile and map - without asking: + bool mForcedClose = false; }; Q_DECLARE_OPERATORS_FOR_FLAGS(Host::DiscordOptionFlags) diff --git a/src/HostManager.cpp b/src/HostManager.cpp index a7196bf4c54..f17f484226b 100644 --- a/src/HostManager.cpp +++ b/src/HostManager.cpp @@ -25,16 +25,18 @@ #include "dlgMapper.h" #include "mudlet.h" -bool HostManager::deleteHost(const QString& hostname) +void HostManager::deleteHost(const QString& hostname) { - // make sure this is really a new host + // make sure this is really an existing host if (!mHostPool.contains(hostname)) { qDebug() << "HostManager::deleteHost(" << hostname.toUtf8().constData() << ") ERROR: not a member of host pool... aborting!"; - return false; - } else { - const int ret = mHostPool.remove(hostname); - return ret; + return; } + + // As this pulls the QSharedPointer that hostname identifies out of the pool + // the Host goes out of scope when execution leaves this method and thus + // gets destroyed: + mHostPool.remove(hostname); } bool HostManager::addHost(const QString& hostname, const QString& port, const QString& login, const QString& pass) diff --git a/src/HostManager.h b/src/HostManager.h index e42c5f5c135..fa5f63c7a55 100644 --- a/src/HostManager.h +++ b/src/HostManager.h @@ -52,12 +52,12 @@ class HostManager public: - HostManager() = default; /* : mpActiveHost() - Not needed */ + HostManager() = default; Host* getHost(const QString& hostname); bool addHost(const QString& name, const QString& port, const QString& login, const QString& pass); int getHostCount(); - bool deleteHost(const QString&); + void deleteHost(const QString&); void postIrcMessage(const QString&, const QString&, const QString&); void postInterHostEvent(const Host*, const TEvent&, const bool = false); void changeAllHostColour(const Host*); diff --git a/src/TBuffer.cpp b/src/TBuffer.cpp index 507f62eb8b8..85cde48d254 100644 --- a/src/TBuffer.cpp +++ b/src/TBuffer.cpp @@ -2750,8 +2750,13 @@ inline int TBuffer::wrap(int startLine) return insertedLines > 0 ? insertedLines : 0; } +// This only works on the Main Console for a profile void TBuffer::log(int fromLine, int toLine) { + if (mpHost.isNull()) { + return; + } + TBuffer* pB = &mpHost->mpConsole->buffer; if (pB != this || !mpHost->mpConsole->mLogToLogFile) { return; diff --git a/src/TConsole.cpp b/src/TConsole.cpp index 7b1ccc6a787..7e4b32f952f 100644 --- a/src/TConsole.cpp +++ b/src/TConsole.cpp @@ -722,14 +722,14 @@ void TConsole::closeEvent(QCloseEvent* event) if (mudlet::self()->isGoingDown() || mpHost->isClosingDown()) { event->accept(); return; - } else { - hide(); - mudlet::smpDebugArea->setVisible(false); - mudlet::smDebugMode = false; - mudlet::self()->refreshTabBar(); - event->ignore(); - return; } + + hide(); + mudlet::smpDebugArea->setVisible(false); + mudlet::smDebugMode = false; + mudlet::self()->refreshTabBar(); + event->ignore(); + return; } if (mType & (SubConsole|Buffer)) { @@ -745,11 +745,11 @@ void TConsole::closeEvent(QCloseEvent* event) event->accept(); return; - } else { - hide(); - event->ignore(); - return; } + + hide(); + event->ignore(); + return; } if (mType == UserWindow) { @@ -763,112 +763,26 @@ void TConsole::closeEvent(QCloseEvent* event) mUpperPane->close(); mLowerPane->close(); } - if (!pD) { + if (pD) { + pD->setAttribute(Qt::WA_DeleteOnClose); + pD->deleteLater(); + } else { qDebug() << "TConsole::closeEvent(QCloseEvent*) INFO - closing a UserWindow but the TDockWidget pointer was not found to be removed..."; } + // This will also cause the QWidget to be automatically hidden: event->accept(); return; - } else { - hide(); - event->ignore(); - return; } - } - - TEvent conCloseEvent{}; - conCloseEvent.mArgumentList.append(qsl("sysExitEvent")); - conCloseEvent.mArgumentTypeList.append(ARGUMENT_TYPE_STRING); - mpHost->raiseEvent(conCloseEvent); - if (mpHost->mFORCE_SAVE_ON_EXIT) { - mudlet::self()->saveWindowLayout(); - mpHost->modulesToWrite.clear(); - mpHost->saveProfile(); - - if (mpHost->mpMap && mpHost->mpMap->mpRoomDB) { - // There is a map loaded - but it *could* have no rooms at all! - const QDir dir_map; - const QString directory_map = mudlet::getMudletPath(mudlet::profileMapsPath, mProfileName); - const QString filename_map = mudlet::getMudletPath(mudlet::profileDateTimeStampedMapPathFileName, mProfileName, QDateTime::currentDateTime().toString("yyyy-MM-dd#HH-mm-ss")); - if (!dir_map.exists(directory_map)) { - dir_map.mkpath(directory_map); - } - QSaveFile file(filename_map); - if (file.open(QIODevice::WriteOnly)) { - QDataStream out(&file); - if (mudlet::scmRunTimeQtVersion >= QVersionNumber(5, 13, 0)) { - out.setVersion(mudlet::scmQDataStreamFormat_5_12); - } - // FIXME: https://github.com/Mudlet/Mudlet/issues/6316 - unchecked return value - we are not handling a failure to save the map! - mpHost->mpMap->serialize(out); - if (!file.commit()) { - qDebug() << "TConsole::closeEvent: error saving map: " << file.errorString(); - } - } - } - mpHost->waitForProfileSave(); - event->accept(); + hide(); + event->ignore(); return; } - if (!mUserAgreedToCloseConsole) { - ASK: - const int choice = QMessageBox::question(this, tr("Save profile?"), tr("Do you want to save the profile %1?").arg(mProfileName), QMessageBox::Yes | QMessageBox::No | QMessageBox::Cancel); - if (choice == QMessageBox::Cancel) { - event->setAccepted(false); - event->ignore(); - return; - } - if (choice == QMessageBox::Yes) { - mudlet::self()->saveWindowLayout(); - - mpHost->modulesToWrite.clear(); - auto [ok, filename, error] = mpHost->saveProfile(); - - if (!ok) { - QMessageBox::critical(this, tr("Couldn't save profile"), tr("Sorry, couldn't save your profile - got the following error: %1").arg(error)); - goto ASK; - } else if (mpHost->mpMap && mpHost->mpMap->mpRoomDB) { - // There is a map loaded - but it *could* have no rooms at all! - const QDir dir_map; - const QString directory_map = mudlet::getMudletPath(mudlet::profileMapsPath, mProfileName); - const QString filename_map = mudlet::getMudletPath(mudlet::profileDateTimeStampedMapPathFileName, mProfileName, QDateTime::currentDateTime().toString(qsl("yyyy-MM-dd#HH-mm-ss"))); - if (!dir_map.exists(directory_map)) { - dir_map.mkpath(directory_map); - } - QSaveFile file(filename_map); - if (file.open(QIODevice::WriteOnly)) { - QDataStream out(&file); - if (mudlet::scmRunTimeQtVersion >= QVersionNumber(5, 13, 0)) { - out.setVersion(mudlet::scmQDataStreamFormat_5_12); - } - // FIXME: https://github.com/Mudlet/Mudlet/issues/6316 - unchecked return value - we are not handling a failure to save the map! - mpHost->mpMap->serialize(out); - if (!file.commit()) { - qDebug() << "TConsole::closeEvent: error saving map: " << file.errorString(); - } - } - } - mpHost->waitForProfileSave(); - event->accept(); - return; - - } else if (choice == QMessageBox::No) { - mudlet::self()->saveWindowLayout(); - - event->accept(); - return; - } else { - if (!mudlet::self()->isGoingDown()) { - QMessageBox::warning(this, "Aborting exit", "Session exit aborted on user request."); - event->ignore(); - return; - } else { - event->accept(); - return; - } - } + if (mType == MainConsole) { + // The event should have been handled by the override in the TMainConsole + Q_ASSERT_X(false, "TConsole::closeEvent()", "Close event not handled by TMainConsole override."); } } diff --git a/src/TConsole.h b/src/TConsole.h index b6b54f358e5..7b05eea63d8 100644 --- a/src/TConsole.h +++ b/src/TConsole.h @@ -286,7 +286,6 @@ class TConsole : public QWidget bool mIsPromptLine = false; QToolButton* logButton = nullptr; QToolButton* timeStampButton = nullptr; - bool mUserAgreedToCloseConsole = false; QLineEdit* mpBufferSearchBox = nullptr; QAction* mpAction_searchCaseSensitive = nullptr; QToolButton* mpBufferSearchUp = nullptr; diff --git a/src/TDebug.cpp b/src/TDebug.cpp index 01bd9d4768f..7de30dfab78 100644 --- a/src/TDebug.cpp +++ b/src/TDebug.cpp @@ -163,7 +163,7 @@ void TDebug::changeHostName(const Host* pHost, const QString& newName) } } -/* static */ void TDebug::addHost(Host* pHost) +/* static */ void TDebug::addHost(Host* pHost, const QString hostName) { if (!initialised) { smAvailableIdentifiers << qsl("[A] ") << qsl("[B] ") << qsl("[C] ") << qsl("[D] ") << qsl("[E] ") @@ -179,12 +179,6 @@ void TDebug::changeHostName(const Host* pHost, const QString& newName) return; } - QString hostName = pHost->getName(); - // Take a deep-copy to prevent RVO of the Host::getName() method from - // stopping deleting the 'Host::mHostName` when the profile is destroyed - // - so this copy can persist independently of the original when the latter - // goes away: - hostName.detach(); QPair newIdentifier; if (TDebug::smAvailableIdentifiers.isEmpty()) { // Run out of identifiers - use fall-back one: @@ -209,7 +203,7 @@ void TDebug::changeHostName(const Host* pHost, const QString& newName) } } -/* static */ void TDebug::removeHost(Host* pHost) +/* static */ void TDebug::removeHost(Host* pHost, const QString hostName) { auto identifier = TDebug::smIdentifierMap.take(pHost); // Check for the use of non-profile specific tags: @@ -218,7 +212,7 @@ void TDebug::changeHostName(const Host* pHost, const QString& newName) smAvailableIdentifiers.enqueue(identifier.second); } TDebug localMessage(Qt::darkGray, Qt::white); - localMessage << qsl("Profile '%1' ended.\n").arg(pHost->getName()) >> nullptr; + localMessage << qsl("Profile '%1' ended.\n").arg(hostName) >> nullptr; TDebug tableMessage(Qt::white, Qt::black); tableMessage << TDebug::displayNewTable() >> nullptr; } @@ -273,7 +267,7 @@ void TDebug::changeHostName(const Host* pHost, const QString& newName) // for it - this will also cause a pair of new TDebug messages to // be created and processed prior to the call to this method being // completed: - addHost(pHost); + addHost(pHost, pHost->getName()); } // By now smIdentifierMap WILL contain something for pHost - but use an // the "fault" mark (a bang/exclaimation point) if something is really diff --git a/src/TDebug.h b/src/TDebug.h index 3cde2bb3d94..053b90432a1 100644 --- a/src/TDebug.h +++ b/src/TDebug.h @@ -86,8 +86,8 @@ class TDebug explicit TDebug(const QColor&, const QColor&); ~TDebug() = default; - static void addHost(Host*); - static void removeHost(Host*); + static void addHost(Host*, const QString); // Might need to NOLINT this to prevent a warning about not using a reference + static void removeHost(Host*, const QString); // Might need to NOLINT this to prevent a warning about not using a reference static void changeHostName(const Host*, const QString&); static void flushMessageQueue(); static QString getTag(Host*); diff --git a/src/TLuaInterpreter.cpp b/src/TLuaInterpreter.cpp index ddf8969d972..b091c6cdb85 100644 --- a/src/TLuaInterpreter.cpp +++ b/src/TLuaInterpreter.cpp @@ -1310,7 +1310,7 @@ int TLuaInterpreter::setModulePriority(lua_State* L) int TLuaInterpreter::closeMudlet(lua_State* L) { Q_UNUSED(L) - mudlet::self()->forceClose(); + mudlet::self()->armForceClose(); return 0; } diff --git a/src/TMainConsole.cpp b/src/TMainConsole.cpp index 064420b6c3a..ab933adcebd 100644 --- a/src/TMainConsole.cpp +++ b/src/TMainConsole.cpp @@ -1532,3 +1532,113 @@ void TMainConsole::showStatistics() mpHost->mpConsole->raise(); } + +void TMainConsole::closeEvent(QCloseEvent* event) +{ + qDebug().nospace().noquote() << "TMainConsole::closeEvent(...) INFO - received by \"" << mpHost->getName() << "\"."; + TEvent conCloseEvent{}; + conCloseEvent.mArgumentList.append(qsl("sysExitEvent")); + conCloseEvent.mArgumentTypeList.append(ARGUMENT_TYPE_STRING); + mpHost->raiseEvent(conCloseEvent); + + if (mpHost->mFORCE_SAVE_ON_EXIT || mpHost->isClosingForced()) { + mudlet::self()->saveWindowLayout(); + mpHost->modulesToWrite.clear(); + // We are not checking the status result from here! + mpHost->saveProfile(); + + if (mpHost->mpMap && mpHost->mpMap->mpRoomDB) { + // There is a map loaded - but it *could* have no rooms at all! + const QDir dir_map; + const QString directory_map = mudlet::getMudletPath(mudlet::profileMapsPath, mProfileName); + const QString filename_map = mudlet::getMudletPath(mudlet::profileDateTimeStampedMapPathFileName, mProfileName, QDateTime::currentDateTime().toString("yyyy-MM-dd#HH-mm-ss")); + if (!dir_map.exists(directory_map)) { + dir_map.mkpath(directory_map); + } + QSaveFile file(filename_map); + if (file.open(QIODevice::WriteOnly)) { + QDataStream out(&file); + if (mudlet::scmRunTimeQtVersion >= QVersionNumber(5, 13, 0)) { + out.setVersion(mudlet::scmQDataStreamFormat_5_12); + } + // FIXME: https://github.com/Mudlet/Mudlet/issues/6316 - unchecked return value - we are not handling a failure to save the map! + mpHost->mpMap->serialize(out); + if (!file.commit()) { + qWarning() << "TMainConsole::closeEvent(...) WARNING - error saving map: " << file.errorString(); + } + } + } + mpHost->waitForProfileSave(); + event->accept(); + return; + } + + if (!mEnableClose) { + ASK: + const int choice = QMessageBox::question(this, tr("Save profile?"), tr("Do you want to save the profile %1?").arg(mProfileName), QMessageBox::Yes | QMessageBox::No | QMessageBox::Cancel); + if (choice == QMessageBox::Cancel) { + event->ignore(); + return; + } + + if (choice == QMessageBox::Yes) { + mudlet::self()->saveWindowLayout(); + + mpHost->modulesToWrite.clear(); + auto [ok, filename, error] = mpHost->saveProfile(); + + if (!ok) { + QMessageBox::critical(this, + tr("Could not save profile"), + tr("Sorry, could not save your profile as \"%1\" - got the following error: \"%2\".") + .arg(filename, error), + QMessageBox::Retry); + goto ASK; + } + + if (mpHost->mpMap && mpHost->mpMap->mpRoomDB) { + // There is a map loaded - but it *could* have no rooms at all! + const QDir dir_map; + const QString directory_map = mudlet::getMudletPath(mudlet::profileMapsPath, mProfileName); + const QString filename_map = mudlet::getMudletPath(mudlet::profileDateTimeStampedMapPathFileName, mProfileName, QDateTime::currentDateTime().toString(qsl("yyyy-MM-dd#HH-mm-ss"))); + if (!dir_map.exists(directory_map)) { + dir_map.mkpath(directory_map); + } + + QSaveFile file(filename_map); + if (file.open(QIODevice::WriteOnly)) { + QDataStream out(&file); + if (mudlet::scmRunTimeQtVersion >= QVersionNumber(5, 13, 0)) { + out.setVersion(mudlet::scmQDataStreamFormat_5_12); + } + // FIXME: https://github.com/Mudlet/Mudlet/issues/6316 - unchecked return value - we are not handling a failure to save the map! + mpHost->mpMap->serialize(out); + if (!file.commit()) { + qDebug() << "TConsole::closeEvent: error saving map: " << file.errorString(); + } + } + } + + mpHost->waitForProfileSave(); + mEnableClose = true; + event->accept(); + return; + } + + if (choice == QMessageBox::No) { + mudlet::self()->saveWindowLayout(); + mEnableClose = true; + event->accept(); + return; + } + + if (!mudlet::self()->isGoingDown()) { + QMessageBox::warning(this, "Aborting exit", "Session exit aborted on user request."); + event->ignore(); + return; + } + + mEnableClose = true; + event->accept(); + } +} diff --git a/src/TMainConsole.h b/src/TMainConsole.h index 2c126e3fb47..0acf819a269 100644 --- a/src/TMainConsole.h +++ b/src/TMainConsole.h @@ -48,6 +48,7 @@ class TMainConsole : public TConsole void resizeEvent(QResizeEvent* event) override; void resetMainConsole(); + void closeEvent(QCloseEvent*) override; TConsole* createMiniConsole(const QString& windowname, const QString& name, int x, int y, int width, int height); TScrollBox* createScrollBox(const QString& windowname, const QString& name, int x, int y, int width, int height); bool raiseWindow(const QString& name); @@ -137,7 +138,7 @@ public slots: private: // Was public in Host class but made private there and cloned to here // (for main TConsole) to prevent it being changed without going through the - // process to load in the the changed dictionary: + // process to load in the changed dictionary: QString mSpellDic; // Cloned from Host @@ -161,6 +162,7 @@ public slots: // for the ".aff" file - this member is for the per profile option only as // the shared one is held by the mudlet singleton class: QSet mWordSet_profile; + bool mEnableClose = false; }; #endif // MUDLET_TMAINCONSOLE_H diff --git a/src/ctelnet.cpp b/src/ctelnet.cpp index d785d0e6c8f..96e4de0bdc9 100644 --- a/src/ctelnet.cpp +++ b/src/ctelnet.cpp @@ -508,7 +508,7 @@ void cTelnet::slot_socketDisconnected() mNeedDecompression = false; reset(); - if (!mpHost->mIsGoingDown) { + if (!mpHost->isClosingDown()) { postMessage(spacer); #if !defined(QT_NO_SSL) diff --git a/src/dlgPackageExporter.cpp b/src/dlgPackageExporter.cpp index d756520b477..af630f9fe54 100644 --- a/src/dlgPackageExporter.cpp +++ b/src/dlgPackageExporter.cpp @@ -133,6 +133,13 @@ dlgPackageExporter::dlgPackageExporter(QWidget *parent, Host* pHost) //: Title of the window. The %1 will be replaced by the current profile's name setWindowTitle(tr("Package Exporter - %1").arg(mpHost->getName())); + + // Ensure this dialog goes away if the Host (profile) is closed while we are + // open - as this is parented to the mudlet instance rather than the Host + // of the profile it would otherwise be left around if only the profile was + // being closed: + connect(mpHost, &QObject::destroyed, this, &dlgPackageExporter::slot_cancelExport); + connect(mpHost, &QObject::destroyed, this, &dlgPackageExporter::close); } dlgPackageExporter::~dlgPackageExporter() diff --git a/src/mudlet.cpp b/src/mudlet.cpp index 6c5fe0b3b66..d34bbc2890f 100644 --- a/src/mudlet.cpp +++ b/src/mudlet.cpp @@ -1383,87 +1383,31 @@ void mudlet::slot_closeCurrentProfile() void mudlet::slot_closeProfileRequested(int tab) { const QString name = mpTabBar->tabData(tab).toString(); - closeHost(name); -} - -void mudlet::closeHost(const QString& name) -{ Host* pH = mHostManager.getHost(name); if (!pH) { return; } - std::list> const hostToolBarMap = pH->getActionUnit()->getToolBarList(); - QMap& dockWindowMap = pH->mpConsole->mDockWidgetMap; - QMap& hostConsoleMap = pH->mpConsole->mSubConsoleMap; - - if (!pH->mpConsole->close()) { + if (!pH->requestClose()) { return; } - pH->mpConsole->mUserAgreedToCloseConsole = true; - pH->closingDown(); - - // disconnect before removing objects from memory as sysDisconnectionEvent needs that stuff. - if (pH->mSslTsl) { - pH->mTelnet.abortConnection(); - } else { - pH->mTelnet.disconnectIt(); - } - - pH->stopAllTriggers(); - pH->mpEditorDialog->setAttribute(Qt::WA_DeleteOnClose); - pH->mpEditorDialog->close(); - pH->mpEditorDialog = nullptr; - - for (auto consoleName : hostConsoleMap.keys()) { - if (dockWindowMap.contains(consoleName)) { - dockWindowMap[consoleName]->setAttribute(Qt::WA_DeleteOnClose); - dockWindowMap[consoleName]->close(); - removeDockWidget(dockWindowMap[consoleName]); - dockWindowMap.remove(consoleName); - } - - hostConsoleMap[consoleName]->close(); - hostConsoleMap.remove(consoleName); - } - - if (pH->mpNotePad) { - pH->mpNotePad->save(); - pH->mpNotePad->setAttribute(Qt::WA_DeleteOnClose); - pH->mpNotePad->close(); - pH->mpNotePad = nullptr; - } - - for (TToolBar* pTB : hostToolBarMap) { - if (pTB) { - pTB->setAttribute(Qt::WA_DeleteOnClose); - pTB->deleteLater(); - } - } - - // close IRC client window if it is open. - if (pH->mpDlgIRC) { - pH->mpDlgIRC->setAttribute(Qt::WA_DeleteOnClose); - pH->mpDlgIRC->deleteLater(); - } + closeHost(name); +} +// This removes the Host (profile) from this class's QMainWindow and related +// structures: +void mudlet::closeHost(const QString& name) +{ + Host* pH = mHostManager.getHost(name); migrateDebugConsole(pH); - pH->mpConsole->close(); - mpTabBar->removeTab(name); - // PLACEMARKER: Host destruction (1) - from close button on tab bar - // Unfortunately the spaghetti nature of the code means that the profile - // is also (maybe) saved (or not) in the TConsole::close() call prior to - // here but because that is optional we cannot only force a "save" - // operation in the profile preferences dialog for the Host specific - // details BEFORE the save (so any changes make it into the save) - - // instead we just have to accept that any profile changes will not be - // saved if the preferences dialog is not closed before the profile is... + // PLACEMARKER: Host destruction (1) - from all sources int hostCount = mHostManager.getHostCount(); emit signal_hostDestroyed(pH, --hostCount); - mHostManager.deleteHost(pH->getName()); + // This is what kills the Host instance: + mHostManager.deleteHost(name); emit signal_adjustAccessibleNames(); updateMultiViewControls(); } @@ -1803,34 +1747,53 @@ Host* mudlet::getActiveHost() { if (mpCurrentActiveHost && mpCurrentActiveHost->mpConsole) { return mpCurrentActiveHost; - } else { - return nullptr; } + + return nullptr; } +// Received when the OS/DE/WM tells Mudlet to close (or we force the close +// ourselves): void mudlet::closeEvent(QCloseEvent* event) { - QVector closingHosts; + qDebug() << "mudlet::closeEvent(...) INFO - called!"; + QStringList hostsToDestroy; + bool abortClose = false; + // Due to the way that Hosts are stored we cannot do a closeHost(hostName) + // within the following loop as it fatally messes with what mHostManager + // contains - this is STL iterator stuff! for (auto pHost : mHostManager) { - const auto console = pHost->mpConsole; - if (!console) { + if (pHost->requestClose()) { + // If we get here then the user has agreed to close it and the + // profile has been saved - if required - or both have happened + // automatically - and the main console has been told to close: + + hostsToDestroy.append(pHost->getName()); continue; } - if (!console->close()) { - // close out any profiles that we have agreed to close so far - for (const auto& hostName : qAsConst(closingHosts)) { - closeHost(hostName); - } - event->ignore(); - return; - } else { - console->mUserAgreedToCloseConsole = true; - closingHosts.append(pHost->getName()); - } + // This profile is not to be closed or the user has cancelled the close, + // in either case the application close cannot proceed - so give up, + // but we cannot just ignore() the event and return as there may be + // previously closed profiles to clean up: + abortClose = true; + // Stop the iteration + break; + } + + // Clean up the profiles that are being closed + for (auto const& hostName : hostsToDestroy) { + closeHost(hostName); } + // Now we bail out if the close is cancelled: + if (abortClose) { + event->ignore(); + return; + } + + // Since we are here the close is to be completed: writeSettings(); goingDown(); @@ -1839,19 +1802,9 @@ void mudlet::closeEvent(QCloseEvent* event) smpDebugArea->close(); } - for (auto pHost : mHostManager) { - pHost->close(); - } - // hide main Mudlet window once we're sure the 'do you want to save the profile?' won't come up hide(); - for (auto pHost : mHostManager) { - if (pHost->currentlySavingProfile()) { - pHost->waitForProfileSave(); - } - } - // pass the event on so dblsqd can perform an update // if automatic updates have been disabled event->accept(); @@ -1860,50 +1813,10 @@ void mudlet::closeEvent(QCloseEvent* event) void mudlet::forceClose() { for (auto pHost : mHostManager) { - auto console = pHost->mpConsole; - if (!console) { - continue; - } - pHost->saveProfile(); - console->mUserAgreedToCloseConsole = true; - - if (pHost->mSslTsl) { - pHost->mTelnet.abortConnection(); - } else { - pHost->mTelnet.disconnectIt(); - } - - // close script-editor - if (pHost->mpEditorDialog) { - pHost->mpEditorDialog->setAttribute(Qt::WA_DeleteOnClose); - pHost->mpEditorDialog->close(); - } - - if (pHost->mpNotePad) { - pHost->mpNotePad->save(); - pHost->mpNotePad->setAttribute(Qt::WA_DeleteOnClose); - pHost->mpNotePad->close(); - pHost->mpNotePad = nullptr; - } - - if (pHost->mpDlgIRC) { - pHost->mpDlgIRC->close(); - } - - console->close(); - } - - // hide main Mudlet window once we're sure the 'do you want to save the profile?' won't come up - hide(); - - for (auto pHost : mHostManager) { - if (pHost->currentlySavingProfile()) { - pHost->waitForProfileSave(); - } + pHost->forceClose(); } - writeSettings(); - + // This will fire the closeEvent(...) close(); } @@ -4947,3 +4860,14 @@ void mudlet::onlyShowProfiles(const QStringList& predefinedProfiles) return QImage(qsl(":/splash/Mudlet_splashscreen_main.png")); #endif // INCLUDE_VARIABLE_SPLASH_SCREEN } + +// The Lua interpreter cannot call mudlet::forceClose() directly as the latter +// will destroy the former before a direct call has completed which has bad +// effects (like the Lua API resetProfile() once did). Instead arrange for it +// to be done on the next Qt event loop iteration: +void mudlet::armForceClose() +{ + QTimer::singleShot(0, this, [this]() { + forceClose(); + }); +} diff --git a/src/mudlet.h b/src/mudlet.h index 06768d89134..065c316310f 100644 --- a/src/mudlet.h +++ b/src/mudlet.h @@ -323,6 +323,7 @@ class mudlet : public QMainWindow, public Ui::main_window void doAutoLogin(const QString&); void enableToolbarButtons(); void forceClose(); + void armForceClose(); Host* getActiveHost(); QStringList getAvailableFonts(); QList getAvailableTranslationCodes() const { return mTranslationsMap.keys(); } From 8927d9efc26b1efea997132f3c880da7180447e2 Mon Sep 17 00:00:00 2001 From: Vadim Peretokin Date: Sat, 27 Apr 2024 16:15:36 +0200 Subject: [PATCH 4/5] Infrastructure: Don't install outdated node 14 for danger (#7216) #### Brief overview of PR changes/additions Don't install outdated node 14 for danger #### Motivation for adding to Mudlet Fix danger to danger again #### Other info (issues closed, discussion etc) Wasn't able to test via `push:` trigger as that caused some issues, so hopefully this fix works as-is. --- .github/workflows/dangerjs.yml | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/.github/workflows/dangerjs.yml b/.github/workflows/dangerjs.yml index d28f2d622cb..5d9a3d3bd8c 100644 --- a/.github/workflows/dangerjs.yml +++ b/.github/workflows/dangerjs.yml @@ -14,11 +14,7 @@ jobs: ref: ${{github.event.pull_request.head.ref}} repository: ${{github.event.pull_request.head.repo.full_name}} fetch-depth: 0 - - name: Use Node.js - uses: actions/setup-node@v4 - with: - node-version: "14.x" - - name: Install yarn dependencies + - name: Install yarn dependencies and run danger run: | yarn add danger --dev yarn danger ci From 047b3c543f437ddaf13b1d6b85b6a8c48cbee4bd Mon Sep 17 00:00:00 2001 From: Vadim Peretokin Date: Sat, 27 Apr 2024 16:15:53 +0200 Subject: [PATCH 5/5] Fix: edbee crash in Qt6 (#7214) #### Brief overview of PR changes/additions edbee crash in Qt6 #### Motivation for adding to Mudlet Bugfix #### Other info (issues closed, discussion etc) The hide() method was being called right within during widget creation which was causing the crash. Closes https://github.com/Mudlet/Mudlet/issues/7209 Co-authored-by: Vadim Peretokin --- 3rdparty/edbee-lib | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/3rdparty/edbee-lib b/3rdparty/edbee-lib index 4ca16c24547..dac04313497 160000 --- a/3rdparty/edbee-lib +++ b/3rdparty/edbee-lib @@ -1 +1 @@ -Subproject commit 4ca16c245472ba6c5e4a9b6e8e459e2700ea9dc0 +Subproject commit dac043134972dc6bb269271ce0362b9e95a9e6de