diff --git a/src/T2DMap.cpp b/src/T2DMap.cpp index e06c5ad44d6..ceddb1a8522 100644 --- a/src/T2DMap.cpp +++ b/src/T2DMap.cpp @@ -3711,23 +3711,27 @@ void T2DMap::slot_toggleMapViewOnly() void T2DMap::populateUserContextMenus(QMenu& menu) { QMap userMenus; - QMapIterator menuIterator(mUserMenus); - while (menuIterator.hasNext()) { - menuIterator.next(); - const QStringList menuInfo = menuIterator.value(); + // First pass: create all menus in insertion order + for (const QString& uniqueName : mUserMenusOrder) { + if (!mUserMenus.contains(uniqueName)) { + continue; // Skip if menu was removed + } + const QStringList menuInfo = mUserMenus.value(uniqueName); const QString displayName = menuInfo.value(1); auto* userMenu = new QMenu(displayName, &menu); - userMenus.insert(menuIterator.key(), userMenu); + userMenus.insert(uniqueName, userMenu); } - menuIterator.toFront(); - while (menuIterator.hasNext()) { - menuIterator.next(); - const QStringList menuInfo = menuIterator.value(); + // Second pass: add menus to their parents in insertion order + for (const QString& uniqueName : mUserMenusOrder) { + if (!mUserMenus.contains(uniqueName)) { + continue; // Skip if menu was removed + } + const QStringList menuInfo = mUserMenus.value(uniqueName); const QString menuParent = menuInfo.value(0); - if (auto* childMenu = userMenus.value(menuIterator.key(), nullptr)) { + if (auto* childMenu = userMenus.value(uniqueName, nullptr)) { if (menuParent.isEmpty()) { menu.addMenu(childMenu); } else if (auto* parentMenu = userMenus.value(menuParent, nullptr)) { diff --git a/src/T2DMap.h b/src/T2DMap.h index d735544e466..5cd833f436e 100644 --- a/src/T2DMap.h +++ b/src/T2DMap.h @@ -217,6 +217,8 @@ class T2DMap : public QWidget // unique name, List:parent name ("" if null), display name QMap mUserMenus; + // Tracks insertion order of menu unique names (preserves creation order) + QList mUserMenusOrder; bool mRoomBeingMoved = false; QPointF mRoomMoveLastMapPoint; diff --git a/src/TLuaInterpreterMapper.cpp b/src/TLuaInterpreterMapper.cpp index 078fc6e833a..bd8a28dfc15 100644 --- a/src/TLuaInterpreterMapper.cpp +++ b/src/TLuaInterpreterMapper.cpp @@ -562,7 +562,12 @@ int TLuaInterpreter::addMapMenu(lua_State* L) if (host.mpMap) { if (host.mpMap->mpMapper) { if (host.mpMap->mpMapper->mp2dMap) { - host.mpMap->mpMapper->mp2dMap->mUserMenus.insert(uniqueName, menuList); + auto* mp2dMap = host.mpMap->mpMapper->mp2dMap; + mp2dMap->mUserMenus.insert(uniqueName, menuList); + // Track insertion order - only add if not already present + if (!mp2dMap->mUserMenusOrder.contains(uniqueName)) { + mp2dMap->mUserMenusOrder.append(uniqueName); + } } } } @@ -1844,18 +1849,40 @@ int TLuaInterpreter::getMapMenus(lua_State* L) return warnArgumentValue(L, __func__, "you haven't opened a map yet"); } + bool returnDetailed = false; + if (lua_gettop(L) > 0) { + returnDetailed = getVerifiedBool(L, __func__, 1, "return detailed information", true); + } + + auto* mp2dMap = host.mpMap->mpMapper->mp2dMap; lua_newtable(L); - QMapIterator it(host.mpMap->mpMapper->mp2dMap->mUserMenus); - while (it.hasNext()) { - it.next(); - QString parent, display; - QStringList menuInfo = it.value(); - parent = menuInfo[0]; - display = menuInfo[1]; - qDebug() << it.key() << parent << display; - lua_pushstring(L, display.toUtf8().constData()); - lua_pushstring(L, parent.isEmpty() ? "top-level" : parent.toUtf8().constData()); - lua_settable(L, -3); + + // Iterate in insertion order + int index = 1; + for (const QString& uniqueName : mp2dMap->mUserMenusOrder) { + if (!mp2dMap->mUserMenus.contains(uniqueName)) { + continue; // Skip if menu was removed but order list wasn't cleaned up + } + const QStringList& menuInfo = mp2dMap->mUserMenus[uniqueName]; + const QString& parent = menuInfo[0]; + const QString& display = menuInfo[1]; + + if (returnDetailed) { + // Build an integer-indexed array so consumers can use ipairs() and get the correct order + lua_createtable(L, 0, 3); + lua_pushstring(L, uniqueName.toUtf8().constData()); + lua_setfield(L, -2, "uniquename"); + lua_pushstring(L, display.toUtf8().constData()); + lua_setfield(L, -2, "name"); + lua_pushstring(L, parent.isEmpty() ? "top-level" : parent.toUtf8().constData()); + lua_setfield(L, -2, "parent"); + lua_rawseti(L, -2, index++); + } else { + // Return original format for backward compatibility + lua_pushstring(L, display.toUtf8().constData()); + lua_pushstring(L, parent.isEmpty() ? "top-level" : parent.toUtf8().constData()); + lua_settable(L, -3); + } } return 1; @@ -2899,36 +2926,41 @@ int TLuaInterpreter::removeMapMenu(lua_State* L) if (host.mpMap) { if (host.mpMap->mpMapper) { if (host.mpMap->mpMapper->mp2dMap) { - host.mpMap->mpMapper->mp2dMap->mUserMenus.remove(uniqueName); + auto* mp2dMap = host.mpMap->mpMapper->mp2dMap; + mp2dMap->mUserMenus.remove(uniqueName); //remove all entries with this as parent QStringList removeList; removeList.append(uniqueName); bool newElement = true; while (newElement) { newElement = false; - QMapIterator it(host.mpMap->mpMapper->mp2dMap->mUserMenus); + QMapIterator it(mp2dMap->mUserMenus); while (it.hasNext()) { it.next(); QStringList menuInfo = it.value(); const QString parent = menuInfo[0]; if (removeList.contains(parent)) { - host.mpMap->mpMapper->mp2dMap->mUserMenus.remove(it.key()); + mp2dMap->mUserMenus.remove(it.key()); if (it.key() != "" && !removeList.contains(it.key())) { - host.mpMap->mpMapper->mp2dMap->mUserMenus.remove(it.key()); + mp2dMap->mUserMenus.remove(it.key()); removeList.append(it.key()); newElement = true; } } } } - QMapIterator it2(host.mpMap->mpMapper->mp2dMap->mUserActions); + QMapIterator it2(mp2dMap->mUserActions); while (it2.hasNext()) { it2.next(); const QString actParent = it2.value()[1]; if (removeList.contains(actParent)) { - host.mpMap->mpMapper->mp2dMap->mUserActions.remove(it2.key()); + mp2dMap->mUserActions.remove(it2.key()); } } + // Remove all deleted menus from the order list + for (const QString& menuToRemove : removeList) { + mp2dMap->mUserMenusOrder.removeAll(menuToRemove); + } } } } diff --git a/src/mudlet-lua/tests/MapMenu_spec.lua b/src/mudlet-lua/tests/MapMenu_spec.lua new file mode 100644 index 00000000000..bd85befe7ff --- /dev/null +++ b/src/mudlet-lua/tests/MapMenu_spec.lua @@ -0,0 +1,136 @@ +describe("Tests map menu functions", function() + + setup(function() + -- createMapper initializes mp2dMap which is required by addMapMenu/getMapMenus + createMapper(0, 0, 400, 300) + end) + + describe("Tests getMapMenus detailed mode", function() + + before_each(function() + addMapMenu("_test_detail_A", "", "Detail Menu A") + addMapMenu("_test_detail_B", "_test_detail_A", "Detail Menu B") + end) + + after_each(function() + removeMapMenu("_test_detail_A") + removeMapMenu("_test_detail_B") + end) + + it("should return an integer-indexed array usable with ipairs", function() + local menus = getMapMenus(true) + assert.is_table(menus) + assert.is_not_nil(menus[1]) + assert.is_nil(menus[0]) + end) + + it("should include uniquename, name, and parent fields for each entry", function() + local menus = getMapMenus(true) + local found + for _, m in ipairs(menus) do + if m.uniquename == "_test_detail_A" then + found = m + break + end + end + assert.is_not_nil(found, "test menu _test_detail_A not found in results") + assert.are.equal("_test_detail_A", found.uniquename) + assert.are.equal("Detail Menu A", found.name) + assert.are.equal("top-level", found.parent) + end) + + it("should report the parent uniquename for child menus", function() + local menus = getMapMenus(true) + local found + for _, m in ipairs(menus) do + if m.uniquename == "_test_detail_B" then + found = m + break + end + end + assert.is_not_nil(found, "child menu _test_detail_B not found in results") + assert.are.equal("_test_detail_A", found.parent) + end) + + it("should report 'top-level' for menus with no parent", function() + local menus = getMapMenus(true) + for _, m in ipairs(menus) do + if m.uniquename == "_test_detail_A" then + assert.are.equal("top-level", m.parent) + return + end + end + assert.is_true(false, "_test_detail_A not found in results") + end) + + end) + + describe("Tests getMapMenus order preservation", function() + + after_each(function() + removeMapMenu("_test_order_X") + removeMapMenu("_test_order_Y") + removeMapMenu("_test_order_Z") + end) + + it("should return menus in insertion order", function() + addMapMenu("_test_order_X", "", "Order X") + addMapMenu("_test_order_Y", "", "Order Y") + addMapMenu("_test_order_Z", "", "Order Z") + + local menus = getMapMenus(true) + local posX, posY, posZ + for i, m in ipairs(menus) do + if m.uniquename == "_test_order_X" then posX = i end + if m.uniquename == "_test_order_Y" then posY = i end + if m.uniquename == "_test_order_Z" then posZ = i end + end + assert.is_not_nil(posX, "_test_order_X not found") + assert.is_not_nil(posY, "_test_order_Y not found") + assert.is_not_nil(posZ, "_test_order_Z not found") + assert.is_true(posX < posY, "X should precede Y in insertion order") + assert.is_true(posY < posZ, "Y should precede Z in insertion order") + end) + + it("should not produce a duplicate entry when a menu is re-added", function() + addMapMenu("_test_order_X", "", "Order X") + addMapMenu("_test_order_Y", "", "Order Y") + addMapMenu("_test_order_X", "", "Order X Updated") + + local menus = getMapMenus(true) + local count = 0 + for _, m in ipairs(menus) do + if m.uniquename == "_test_order_X" then + count = count + 1 + end + end + assert.are.equal(1, count) + end) + + end) + + describe("Tests getMapMenus non-detailed mode", function() + + before_each(function() + addMapMenu("_test_basic_A", "", "Basic Menu A") + end) + + after_each(function() + removeMapMenu("_test_basic_A") + end) + + it("should return a table keyed by display name with parent as value", function() + local menus = getMapMenus() + assert.is_table(menus) + assert.are.equal("top-level", menus["Basic Menu A"]) + end) + + it("should behave identically when called with explicit false", function() + local menus = getMapMenus(false) + assert.is_table(menus) + assert.are.equal("top-level", menus["Basic Menu A"]) + end) + + end) + +end)