From 11b9331133208ea70097dc805ebae8e1397dc58e Mon Sep 17 00:00:00 2001 From: Stephen Lyons Date: Sat, 15 Aug 2015 22:15:39 +0100 Subject: [PATCH] Backport: merge ten commits room "entrance" code from "release_30" to "development" branches, Commit 01 of 10 originally entitled: "fix for keeping reverse area exit map in sync with exit creation, adding, and deletion" Conflicts resolved in: src/TRoom.h Commit 02 of 10 originally entitled: "bug fix for entranceMap having reversed key and value" Conflicts resolved in: src/TRoomDB.cpp Commit 03 of 10 originally entitled: "removal of entranceMap cleanup" Conflicts resolved in: src/TRoomDB.cpp Commit 04 of 10 originally entitled: "BugFix: prevent crash in dlgRoomExits::initExits() on missing exit" Code deficiencies that remove rooms without properly updating connected rooms were causing segmentation faults because this method - perhaps foolishly - expected a room to exist when another room had an exit Id to the room given. This commit corrects any faulty normal exits now by resetting them to the no exit -1 value. Commit 05 of 10 originally entitled: "Fixup: prevent unneeded TRoom::setExit() calls from dlgRoomExits class" Each individual call to TRoom::setExit((int)exitRoomId,(int)directionCode) from dlgRoomExit::save() creates additional entries in TRoom::entranceMap even for non-exit directions. To reduce (but unfortunately not eliminate) the number of duplicates change the save() code to only use setExit() when a difference between the current and saved exit room numbers is found. Commit 06 of 10 originally entitled: "Fixup: clear TRoomDB::entranceMap on map clearance" Obvious but was missing. Commit 07 of 10 originally entitled: "Fixup: add getAllRoomEntrances() to Lua command set" Though suitable for release code this was added to help with debugging. This was what enabled me to spot the problems that the previous pair of commits ameliorates. This currently only reports the rooms that have exit(s) that lead to the given room Id. It is anticipated that there will be a future revision to report the particular direction(s) from the given room(s) are the one(s) that lead to the room, using a second argument that will be a boolean true to trigger that behavior (the absence of, or a false, second argument would then cause the result that this code produces.) Conflicts resolved in: src/TLuaInterpreter.cpp src/TLuaInterpreter.h Commit 08 of 10 originally entitled: "Fixup: add debugging output to TRoomDB::updateEntranceMap(TRoom *)" Set a break point in the method and change the static bool showDebug to dis-/en-able output... Conflicts resolved in: src/TRoomDB.cpp Commit 09 of 10 originally entitled: "FixEnhance: fix entranceMap maintenance, bulk room deletion & map loading" Previously we were not removing entries from the entranceMap involving the value (a room that the room Id that was a key had an entrance FROM) when a route was changed. There is a performance cost in ensuring the data is kept correctly - there may be a modest gain by storing the entrance data within each TRoom class instance rather than a central database in TRoomDB... Deletion of multiple rooms and map loading can be done more efficiently if we skip some redundant steps. Also added/revised some timing code to measure things. Conflicts resolved in: src/TRoomDB.cpp src/TRoomDB.h Commit 10 of 10 originally entitled: "BugFix: Some previous coding errors" * T2DMap::slot_setArea(): used a uint where I should have used an int as a method I called can return a -1 in some cases. * TArea::getAreaExitRoomData(): a qWarning() in a debugging line I had used the wrong type (%1,%2,...) of format string argument characters when I should of used (%i or %s)... * (bool)TArea::mIsDirty: was put in wrong block of lines in header Conflicts resolved in: src/TArea.h Further conflicts resolved which were brought about by later re-basing before posting code out to world: src/dlgRoomExits.cpp Signed-off-by: Stephen Lyons --- src/T2DMap.cpp | 11 +- src/TArea.cpp | 20 +++- src/TArea.h | 5 +- src/TLuaInterpreter.cpp | 52 ++++++++- src/TLuaInterpreter.h | 1 + src/TMap.cpp | 16 +-- src/TRoom.cpp | 55 ++++++--- src/TRoom.h | 2 +- src/TRoomDB.cpp | 244 ++++++++++++++++++++++++++++++++++------ src/TRoomDB.h | 14 ++- src/dlgRoomExits.cpp | 96 ++++++++++++---- 11 files changed, 408 insertions(+), 108 deletions(-) diff --git a/src/T2DMap.cpp b/src/T2DMap.cpp index 12f9980bee8..1b48869408d 100644 --- a/src/T2DMap.cpp +++ b/src/T2DMap.cpp @@ -3002,11 +3002,10 @@ void T2DMap::slot_setImage() void T2DMap::slot_deleteRoom() { + mpMap->mpRoomDB->removeRoom( mMultiSelectionList ); + // mMultiSelectionList gets cleared as rooms are removed by + // TRoomDB::removeRoom() so no need to clear it here! mMultiRect = QRect(0,0,0,0); - for( int j=0; jmpRoomDB->removeRoom( mMultiSelectionList[j] ); - } mMultiSelectionListWidget.clear(); mMultiSelectionListWidget.hide(); repaint(); @@ -3300,8 +3299,8 @@ void T2DMap::slot_setArea() int newAreaId = arealist_combobox->itemData( arealist_combobox->currentIndex() ).toInt(); mMultiRect = QRect(0,0,0,0); - uint maxRoomIndex = mMultiSelectionList.size() - 1; - for( int j = 0; j <= maxRoomIndex; j++ ) { + int maxRoomIndex = mMultiSelectionList.size() - 1; + for( unsigned int j = 0; j <= maxRoomIndex; j++ ) { if( j < maxRoomIndex ) { mpMap->setRoomArea( mMultiSelectionList.at(j), newAreaId, true ); } diff --git a/src/TArea.cpp b/src/TArea.cpp index 417844f8843..4e7a39b5267 100644 --- a/src/TArea.cpp +++ b/src/TArea.cpp @@ -29,7 +29,7 @@ #include "pre_guard.h" #include #include -#include +#include #include "post_guard.h" // Previous direction #defines here did not match the DIR_ defines in TRoom.h, @@ -254,6 +254,9 @@ void TArea::determineAreaExitsOfRoom( int id ) void TArea::determineAreaExits() { + QElapsedTimer timer; + timer.start(); + exits.clear(); for( int i=0; igetRoom(rooms[i]); @@ -337,6 +340,7 @@ void TArea::determineAreaExits() } } //qDebug()<<"exits:"<getAreaID(this) << "took:" << timer.nsecsElapsed() * 1.0e-9 << "sec."; } void TArea::fast_calcSpan( int id ) @@ -517,12 +521,18 @@ void TArea::calcSpan() void TArea::removeRoom( int room ) { - QTime time; - time.start(); + static double cumulativeMean = 0.0; + static quint64 runCount = 0 ; + QElapsedTimer timer; + timer.start(); // TRoom * pR = mpRoomDB->getRoom( room ); rooms.removeOne( room ); exits.remove( room ); - qDebug()<<"Area removal took"<x; // int y = pR->y*-1; // int z = pR->z; @@ -575,7 +585,7 @@ const QMultiMap > TArea::getAreaExitRoomData() const case DIR_OUT: exitData.first = QString("out"); break; case DIR_OTHER: roomsWithOtherAreaSpecialExits.insert(itAreaExit.key()); break; default: - qWarning("TArea::getAreaExitRoomData() Warning: unrecognised exit code %1 found for exit from room %2 to room %3.", + qWarning("TArea::getAreaExitRoomData() Warning: unrecognised exit code %i found for exit from room %i to room %i.", itAreaExit.value().second, itAreaExit.key(), itAreaExit.value().first ); } if( ! exitData.first.isEmpty() ) { diff --git a/src/TArea.h b/src/TArea.h index 0bdadbef488..ac5855c72e6 100644 --- a/src/TArea.h +++ b/src/TArea.h @@ -56,7 +56,6 @@ class TArea QList getCollisionNodes(); QList getRoomsByPosition(int x, int y, int z); QMap > > koordinatenSystem(); - bool mIsDirty; QList rooms; // rooms of this area @@ -78,7 +77,9 @@ class TArea bool gridMode; bool isZone; int zoneAreaRef; - TRoomDB* mpRoomDB; + TRoomDB * mpRoomDB; + bool mIsDirty; + private: TArea() { qFatal("FATAL: illegal default constructor use of TArea()"); }; diff --git a/src/TLuaInterpreter.cpp b/src/TLuaInterpreter.cpp index ad4c39ee35d..8295347413f 100644 --- a/src/TLuaInterpreter.cpp +++ b/src/TLuaInterpreter.cpp @@ -3917,6 +3917,54 @@ int TLuaInterpreter::getRoomExits( lua_State *L ) return 0; } +// Given a room Id number, returns a lua list (monotonically increasing keys +// starting at 1) with (sorted) values being room Id numbers that have exit(s) +// that enter the given room (even one way routes). +// TODO: Provide exit details: +int TLuaInterpreter::getAllRoomEntrances( lua_State *L ) +{ + int roomId; + if( ! lua_isnumber( L, 1 ) ) { + lua_pushstring( L, tr( "getAllRoomEntrances: bad argument #1 type (room Id, as number expected, got %1)." ).arg( luaL_typename(L, 1)).toUtf8().constData()); + lua_error( L ); + } + else { + roomId = lua_tonumber( L, 1 ); + } + + Host * pHost = TLuaInterpreter::luaInterpreterMap[L]; + if( ! pHost ) { + lua_pushnil( L ); + lua_pushstring( L, tr( "getAllRoomEntrances: NULL Host pointer - something is wrong!" ).toUtf8().constData() ); + return 2; + } + else if( ! pHost->mpMap || ! pHost->mpMap->mpRoomDB ) { + lua_pushnil( L ); + lua_pushstring( L, tr( "getAllRoomEntrances: no map present or loaded!" ).toUtf8().constData() ); + return 2; + } + else { + TRoom * pR = pHost->mpMap->mpRoomDB->getRoom(roomId); + if( ! pR ) { + lua_pushnil( L ); + lua_pushstring( L, tr( "getAllRoomEntrances: bad argument #1 value (number %1 is not a valid room Id)." ).arg(roomId).toUtf8().constData() ); + return 2; + } + lua_newtable(L); + QList entrances = pHost->mpMap->mpRoomDB->getEntranceHash().values( roomId ); + // Could use a .toSet().toList() to remove duplicates values + if( entrances.count() > 1 ) { + std::sort( entrances.begin(), entrances.end() ); + } + for( uint i = 0; i < entrances.size(); i++ ) { + lua_pushnumber( L, i+1 ); + lua_pushnumber( L, entrances.at( i ) ); + lua_settable(L, -3); + } + return 1; + } +} + // EITHER searchRoom( roomId ): // Returns the room name for the given roomId number, or errors out if no such // room exists. @@ -8197,7 +8245,6 @@ int TLuaInterpreter::addSpecialExit( lua_State * L ) { pR_from->setSpecialExit( id_to, _dir ); pR_from->setSpecialExitLock( id_to, _dir, false ); - pHost->mpMap->mMapGraphNeedsUpdate = true; } return 0; } @@ -8232,7 +8279,6 @@ int TLuaInterpreter::removeSpecialExit( lua_State * L ) if( pR ) { pR->setSpecialExit( -1, _dir ); - pHost->mpMap->mMapGraphNeedsUpdate = true; } return 0; } @@ -8335,7 +8381,6 @@ int TLuaInterpreter::clearSpecialExits( lua_State * L ) if( pR ) { pR->clearSpecialExits(); - pHost->mpMap->mMapGraphNeedsUpdate = true; } return 0; } @@ -11546,6 +11591,7 @@ void TLuaInterpreter::initLuaGlobals() lua_register( pGlobalLua, "openWebPage", TLuaInterpreter::openWebPage); lua_register( pGlobalLua, "getRoomUserDataKeys", TLuaInterpreter::getRoomUserDataKeys ); lua_register( pGlobalLua, "getAllRoomUserData", TLuaInterpreter::getAllRoomUserData ); + lua_register( pGlobalLua, "getAllRoomEntrances", TLuaInterpreter::getAllRoomEntrances ); diff --git a/src/TLuaInterpreter.h b/src/TLuaInterpreter.h index 0b1e59b2146..e25af184079 100644 --- a/src/TLuaInterpreter.h +++ b/src/TLuaInterpreter.h @@ -384,6 +384,7 @@ Q_OBJECT static int openWebPage( lua_State * L ); static int getRoomUserDataKeys( lua_State * L ); static int getAllRoomUserData( lua_State * L ); + static int getAllRoomEntrances( lua_State * L ); std::list mCaptureGroupList; diff --git a/src/TMap.cpp b/src/TMap.cpp index 46ccca16ae6..5c5ba7b04c0 100644 --- a/src/TMap.cpp +++ b/src/TMap.cpp @@ -275,6 +275,7 @@ int TMap::createNewRoomID() bool TMap::setExit( int from, int to, int dir ) { + // FIXME: This along with TRoom->setExit need to be unified to a controller. TRoom * pR = mpRoomDB->getRoom( from ); TRoom * pR_to = mpRoomDB->getRoom( to ); @@ -337,13 +338,14 @@ bool TMap::setExit( int from, int to, int dir ) return false; } pA->determineAreaExitsOfRoom(pR->getId()); + mpRoomDB->updateEntranceMap(pR); return ret; } void TMap::init( Host * pH ) { // init areas - QTime _time; + QElapsedTimer _time; _time.start(); if( version < 14 ) { @@ -399,7 +401,7 @@ void TMap::init( Host * pH ) } } } - qDebug("TMap::init() Initialize run time:%i milli-seconds.", _time.elapsed() ); + qDebug() << "TMap::init() Initialize run time:" << _time.nsecsElapsed() * 1.0e-9 << "sec."; } @@ -1032,14 +1034,14 @@ bool TMap::serialize( QDataStream & ofs ) ofs << pR->getExitWeights(); ofs << pR->doors; } - return true; } bool TMap::restore(QString location) { qDebug()<<"restoring map of profile:"<getName()<<" url:"<getUrl(); - QTime _time; _time.start(); + QElapsedTimer _time; + _time.start(); QString folder; QStringList entries; qDebug()<<"RESTORING MAP"; @@ -1192,10 +1194,8 @@ bool TMap::restore(QString location) int i; ifs >> i; TRoom * pT = new TRoom(mpRoomDB); - mpRoomDB->restoreSingleRoom( ifs, i, pT ); pT->restore( ifs, i, version ); - - + mpRoomDB->restoreSingleRoom( ifs, i, pT ); } customEnvColors[257] = mpHost->mRed_2; customEnvColors[258] = mpHost->mGreen_2; @@ -1213,7 +1213,7 @@ bool TMap::restore(QString location) customEnvColors[270] = mpHost->mLightCyan_2; customEnvColors[271] = mpHost->mLightWhite_2; customEnvColors[272] = mpHost->mLightBlack_2; - qDebug()<<"LOADED rooms:"<size()<<" loading time:"<<_time.elapsed(); + qDebug() << "TMap::restore() Loaded" << mpRoomDB->size() << "rooms in:" << _time.nsecsElapsed()* 1.0e-9 << "sec."; if( canRestore ) { return true; diff --git a/src/TRoom.cpp b/src/TRoom.cpp index b16b2e42155..622a05a1b04 100644 --- a/src/TRoom.cpp +++ b/src/TRoom.cpp @@ -31,7 +31,7 @@ #include #include #include -#include +#include #include "post_guard.h" @@ -70,10 +70,16 @@ TRoom::TRoom(TRoomDB * pRDB) TRoom::~TRoom() { - QTime timer; + static double cumulativeMean = 0.0; + static quint64 runCount = 0 ; + QElapsedTimer timer; timer.start(); mpRoomDB->__removeRoom( id ); - qDebug()<<"room destructor took"<addRoom( id ); dirtyAreas.insert( pA ); - pA->mIsDirty; + pA->mIsDirty = true; if( ! isToDeferAreaRelatedRecalculations ) { QSetIterator itpArea = dirtyAreas; @@ -243,21 +249,24 @@ bool TRoom::setArea( int areaID, bool isToDeferAreaRelatedRecalculations ) bool TRoom::setExit( int to, int direction ) { + // FIXME: This along with TRoom->setExit need to be unified to a controller. switch(direction){ - case DIR_NORTH: north = to; return true; break; - case DIR_NORTHEAST: northeast = to; return true; break; - case DIR_NORTHWEST: northwest = to; return true; break; - case DIR_EAST: east = to; return true; break; - case DIR_WEST: west = to; return true; break; - case DIR_SOUTH: south = to; return true; break; - case DIR_SOUTHEAST: southeast = to; return true; break; - case DIR_SOUTHWEST: southwest = to; return true; break; - case DIR_UP: up = to; return true; break; - case DIR_DOWN: down = to; return true; break; - case DIR_IN: in = to; return true; break; - case DIR_OUT: out = to; return true; - } - return false; + case DIR_NORTH: north = to; break; + case DIR_NORTHEAST: northeast = to; break; + case DIR_NORTHWEST: northwest = to; break; + case DIR_EAST: east = to; break; + case DIR_WEST: west = to; break; + case DIR_SOUTH: south = to; break; + case DIR_SOUTHEAST: southeast = to; break; + case DIR_SOUTHWEST: southwest = to; break; + case DIR_UP: up = to; break; + case DIR_DOWN: down = to; break; + case DIR_IN: in = to; break; + case DIR_OUT: out = to; break; + default: return false; + } + mpRoomDB->updateEntranceMap(this); + return true; } bool TRoom::hasExit( int direction ) @@ -549,7 +558,15 @@ void TRoom::setSpecialExit( int to, const QString& cmd ) pA->determineAreaExitsOfRoom( id ); // This updates the (TArea *)->exits map even for exit REMOVALS } + mpRoomDB->updateEntranceMap(this); + mpRoomDB->mpMap->mMapGraphNeedsUpdate = true; +} +void TRoom::clearSpecialExits() +{ + other.clear(); + mpRoomDB->updateEntranceMap(this); + mpRoomDB->mpMap->mMapGraphNeedsUpdate = true; } void TRoom::removeAllSpecialExitsToRoom( int _id ) @@ -569,6 +586,8 @@ void TRoom::removeAllSpecialExitsToRoom( int _id ) { pA->determineAreaExitsOfRoom( id ); } + mpRoomDB->updateEntranceMap(this); + mpRoomDB->mpMap->mMapGraphNeedsUpdate = true; } void TRoom::calcRoomDimensions() diff --git a/src/TRoom.h b/src/TRoom.h index 0b56552212c..f6c05da4c26 100644 --- a/src/TRoom.h +++ b/src/TRoom.h @@ -69,7 +69,7 @@ class TRoom bool hasSpecialExitLock( int, const QString& ); void removeAllSpecialExitsToRoom(int _id ); void setSpecialExit( int to, const QString& cmd ); - void clearSpecialExits() { other.clear(); } + void clearSpecialExits(); const QMultiMap & getOtherMap() const { return other; } const QMap & getExitWeights() const { return exitWeights; } void setExitWeight(const QString& cmd, int w ); diff --git a/src/TRoomDB.cpp b/src/TRoomDB.cpp index 6640cc38cc4..5bea242a37a 100644 --- a/src/TRoomDB.cpp +++ b/src/TRoomDB.cpp @@ -30,13 +30,14 @@ #include "pre_guard.h" #include #include -#include +#include #include "post_guard.h" TRoomDB::TRoomDB( TMap * pMap ) : mpMap( pMap ) , mUnnamedAreaName( qApp->translate( "TRoomDB", "Unnamed Area" ) ) +, mpTempRoomDeletionList( 0 ) { } @@ -57,10 +58,7 @@ bool TRoomDB::addRoom( int id ) { rooms[id] = new TRoom( this ); rooms[id]->setId(id); - QHash exits = rooms[id]->getExits(); - QList toExits = exits.keys(); - for (int i = 0; i < toExits.size(); i++) - reverseExitMap.insert(id, toExits[i]); + // there is no point to update the entranceMap here, as the room has no exit information return true; } else @@ -74,12 +72,13 @@ bool TRoomDB::addRoom( int id ) } } -bool TRoomDB::addRoom( int id, TRoom * pR ) +bool TRoomDB::addRoom( int id, TRoom * pR, bool isMapLoading ) { if( !rooms.contains( id ) && id > 0 && pR ) { rooms[id] = pR; pR->setId(id); + updateEntranceMap(pR, isMapLoading); return true; } else @@ -88,14 +87,133 @@ bool TRoomDB::addRoom( int id, TRoom * pR ) } } +void TRoomDB::deleteValuesFromEntranceMap( int value ) +{ + QList keyList = entranceMap.keys(); + QList valueList = entranceMap.values(); + QList deleteEntries; + uint index = valueList.indexOf( value ); + while ( index != -1 ) { + deleteEntries.append( index ); + index = valueList.indexOf( value, index + 1 ); + } + for (int i = deleteEntries.size() - 1; i >= 0; --i ) { + entranceMap.remove( keyList.at(deleteEntries.at(i)), valueList.at(deleteEntries.at(i)) ); + } +} + +void TRoomDB::deleteValuesFromEntranceMap( QSet & valueSet ) +{ + QElapsedTimer timer; + timer.start(); + QList keyList = entranceMap.keys(); + QList valueList = entranceMap.values(); + QList deleteEntries; + foreach(int roomId, valueSet) { + int index = valueList.indexOf( roomId ); + while (index >= 0 ) { + deleteEntries.append( index ); + index = valueList.indexOf( roomId, index + 1 ); + } + } + for (uint i = 0; i < deleteEntries.size(); ++i) { + entranceMap.remove( keyList.at(deleteEntries.at(i)), valueList.at(deleteEntries.at(i)) ); + } + qDebug() << "TRoomDB::deleteValuesFromEntranceMap() with a list of:" << valueSet.size() << "items, run time:" << timer.nsecsElapsed() * 1.0e-9 << "sec."; +} + +void TRoomDB::updateEntranceMap(int id) +{ + TRoom * pR = getRoom(id); + updateEntranceMap(pR); +} + +void TRoomDB::updateEntranceMap(TRoom * pR, bool isMapLoading) +{ + static bool showDebug = false; // Enable this at runtime (set a breakpoint on it) for debugging! + + // entranceMap maps the room to rooms it has a viable exit to. So if room b and c both have + // an exit to room a, upon deleting room a we want a map that allows us to find + // room b and c efficiently. + // So we create a mapping like: {room_a: room_b, room_a: room_c}. This allows us to delete + // rooms and know which other rooms are impacted by this change in a single lookup. + if( pR ) { + int id = pR->getId(); + QHash exits = pR->getExits(); + QList toExits = exits.keys(); + QString values; + // to update this we need to iterate the entire entranceMap and remove invalid + // connections. I'm not sure if this is efficient for every update, and given + // that we check for rooms existance when the map is used, we'll deal with + // possible spurious exits for now. + // entranceMap.remove(id); // <== not what is wanted + // We need to remove all values == id NOT keys == id, so try and do that + // now. SlySven + + if( ! isMapLoading ) { + deleteValuesFromEntranceMap( id ); // When LOADING a map, will never need to do this + } + for(unsigned int i = 0; i < toExits.size(); i++) { + if( showDebug ) { + values.append( QStringLiteral("%1,").arg(toExits.at(i)) ); + } + entranceMap.insert(toExits.at(i), id); + } + if( showDebug ) { + if( ! values.isEmpty() ) { + values.chop(1); + } + if( values.isEmpty() ) { + qDebug( "TRoomDB::updateEntranceMap(TRoom * pR) called for room with Id:%i, it is not an Entrance for any Rooms.", id ); + } + else { + qDebug( "TRoomDB::updateEntranceMap(TRoom * pR) called for room with Id:%i, it is an Entrance for Room(s): %s.", id, values.toLatin1().constData() ); + } + } + } +} + // this is call by TRoom destructor only bool TRoomDB::__removeRoom( int id ) { - TRoom* pR = getRoom(id); + static QMultiHash _entranceMap; // Make it persistant - for multiple room deletions + static bool isBulkDelete = false; + // Gets set / reset by mpTempRoomDeletionList being non-null, used to setup + // _entranceMap the first time around for multi-room deletions + + TRoom * pR = getRoom(id); + // This will FAIL during map deletion as TRoomDB::rooms has already been + // zapped, so can use to skip everything... if (pR) { + if( mpTempRoomDeletionList && mpTempRoomDeletionList->size() > 1 ) { // We are deleting multiple rooms + if( ! isBulkDelete ) { + _entranceMap = entranceMap; + _entranceMap.detach(); // MUST take a deep copy of the data + isBulkDelete = true; // But only do it the first time for a bulk delete + } + } + else { // We are deleting a single room + if( isBulkDelete ) { // Last time was a bulk delete but it isn't one now + isBulkDelete = false; + } + _entranceMap.clear(); + _entranceMap = entranceMap; // Refresh our local copy + _entranceMap.detach(); // MUST take a deep copy of the data + } + // FIXME: make a proper exit controller so we don't need to do all these if statements - QMultiHash::iterator i = reverseExitMap.find(id); - while (i != reverseExitMap.end() && i.key() == id) { + // Remove the links from the rooms entering this room + QMultiHash::const_iterator i = _entranceMap.find(id); + // The removeAllSpecialExitsToRoom below modifies the entranceMap - and + // it is unsafe to modify (use copy operations on) something that an STL + // iterator is active on - see "Implicit sharing iterator problem" in + // "Container Class | Qt 5.x Core" - this is now avoid by taking a deep + // copy and iterating through that instead whilst modifying the original + while (i != entranceMap.end() && i.key() == id) { + if( i.value() == id || mpTempRoomDeletionList && mpTempRoomDeletionList->size() > 1 && mpTempRoomDeletionList->contains( i.value() ) ) { + ++i; + continue; // Bypass rooms we know are also to be deleted + } TRoom* r = getRoom(i.value()); if (r) { if (r->getNorth() == id) @@ -139,7 +257,10 @@ bool TRoomDB::__removeRoom( int id ) TArea * pA = getArea( areaID ); if (pA) pA->removeRoom(id); - reverseExitMap.remove(id); + if( ( ! mpTempRoomDeletionList ) || mpTempRoomDeletionList->size() == 1 ) { // if NOT deleting multiple rooms + entranceMap.remove(id); // Only removes matching keys + deleteValuesFromEntranceMap(id); // Needed to remove matching values + } // Because we clear the graph in initGraph which will be called // if mMapGraphNeedsUpdate is true -- we don't need to // remove the vertex using clear_vertex and remove_vertex here @@ -165,19 +286,49 @@ bool TRoomDB::removeRoom( int id ) return false; } +void TRoomDB::removeRoom( QList & ids ) +{ + QElapsedTimer timer; + timer.start(); + QSet deletedRoomIds; + mpTempRoomDeletionList = &ids; // Will activate "bulk room deletion" code + // When used by TLuaInterpreter::deleteArea() + // via removeArea(int) the list of rooms to + // delete - as suppplied by the reference + // type argument IS NOT CONSTANT - it is + // ALTERED by TArea::removeRoom( int room ) + // for each room that is removed + quint64 roomcount = mpTempRoomDeletionList->size(); + while( ! mpTempRoomDeletionList->isEmpty() ) { + int deleteRoomId = mpTempRoomDeletionList->first(); + TRoom * pR = getRoom( deleteRoomId ); + if( pR ) { + deletedRoomIds.insert( deleteRoomId ); + delete pR; + } + } + foreach(int deleteRoomId, deletedRoomIds ) { + entranceMap.remove( deleteRoomId ); // This has been deferred from __removeRoom() + } + deleteValuesFromEntranceMap( deletedRoomIds ); + mpTempRoomDeletionList->clear(); + mpTempRoomDeletionList=0; + qDebug() << "TRoomDB::removeRoom(QList) run time for" << roomcount << "rooms:" << timer.nsecsElapsed() * 1.0e-9 << "sec."; +} + bool TRoomDB::removeArea( int id ) { if( areas.contains( id ) ) { - TArea * pA = areas.value(id); - QList rl; - for( int i=0; i< pA->rooms.size(); i++ ) { - rl.push_back( pA->rooms.at(i) ); - } - for( int i=rl.size()-1; i >= 0; i-- ) { - removeRoom( rl.at(i) ); + TArea * pA = areas.value( id ); + if( ! rooms.isEmpty() ) { + removeRoom( pA->rooms ); // During map deletion rooms will already + // have been cleared so this would not + // be wanted to be done in that case. } - areaNamesMap.remove( id ); - areas.remove( id ); + areaNamesMap.remove( id ); // During map deletion areaNamesMap will + // already have been cleared !!! + areas.remove( id ); // This means areas.clear() is not needed during map + // deletion mpMap->mMapGraphNeedsUpdate = true; return true; @@ -192,16 +343,30 @@ bool TRoomDB::removeArea( int id ) bool TRoomDB::removeArea( QString name ) { - if( areaNamesMap.values().contains( name ) ) - { - return removeArea( areaNamesMap.key( name ) ); + if( areaNamesMap.values().contains( name ) ) { + return removeArea( areaNamesMap.key( name ) ); // i.e. call the removeArea(int) method + } + else { + return false; } - return false; } void TRoomDB::removeArea( TArea * pA ) { - removeArea( getAreaID(pA) ); + if( ! pA ) { + qWarning( "TRoomDB::removeArea(TArea *) Warning - attempt to remove an area with a NULL TArea pointer!" ); + return; + } + + int areaId = areas.key(pA, 0); + if( areaId == areas.key(pA, -1) ) { + // By testing twice with different default keys to return if value NOT + // found, we can be certain we have an actual valid value + removeArea( areaId ); + } + else { + qWarning( "TRoomDB::removeArea(TArea *) Warning - attempt to remove an area NOT in TRoomDB::areas!" ); + } } int TRoomDB::getAreaID( TArea * pA ) @@ -211,7 +376,8 @@ int TRoomDB::getAreaID( TArea * pA ) void TRoomDB::buildAreas() { - QTime _time; _time.start(); + QElapsedTimer timer; + timer.start(); QHashIterator it( rooms ); while( it.hasNext() ) { @@ -237,7 +403,7 @@ void TRoomDB::buildAreas() areas[id] = new TArea( mpMap, this ); } } - qDebug()<<"BUILD AREAS run time:"<<_time.elapsed(); + qDebug() << "TRoomDB::buildAreas() run time:" << timer.nsecsElapsed() * 1.0e-9 << "sec."; } @@ -386,7 +552,8 @@ QList TRoomDB::getAreaIDList() void TRoomDB::auditRooms() { - QTime t; t.start(); + QElapsedTimer timer; + timer.start(); // rooms konsolidieren QHashIterator itRooms( rooms ); while( itRooms.hasNext() ) @@ -396,7 +563,7 @@ void TRoomDB::auditRooms() pR->auditExits(); } - qDebug()<<"audit map: runtime:"< rPtrL = getRoomPtrList(); - rooms.clear(); + rooms.clear(); // Prevents any further use of TRoomDB::getRoom(int) !!! + entranceMap.clear(); areaNamesMap.clear(); hashTable.clear(); - for(int i=0; i areaList = getAreaPtrList(); - for( int i=0; i & ); bool removeArea( int id ); bool removeArea( QString name ); void removeArea( TArea * ); @@ -60,13 +61,17 @@ class TRoomDB QList getRoomIDList(); QList getAreaIDList(); const QMap & getAreaNamesMap() const { return areaNamesMap; } - + void updateEntranceMap(TRoom *, bool isMapLoading = false ); + void updateEntranceMap(int); + const QMultiHash & getEntranceHash() const { return entranceMap; } + void deleteValuesFromEntranceMap( int ); + void deleteValuesFromEntranceMap( QSet & ); void buildAreas(); void clearMapDB(); void initAreasForOldMaps(); void auditRooms(); - bool addRoom(int id, TRoom *pR); + bool addRoom(int id, TRoom *pR, bool isMapLoading = false); int getAreaID(TArea * pA); void restoreAreaMap( QDataStream & ); void restoreSingleArea( QDataStream &, int, TArea * ); @@ -81,11 +86,12 @@ class TRoomDB bool __removeRoom( int id ); QHash rooms; - QMultiHash reverseExitMap; + QMultiHash entranceMap; QMap areas; QMap areaNamesMap; TMap * mpMap; QString mUnnamedAreaName; + QList * mpTempRoomDeletionList; // Used during bulk room deletion friend class TRoom;//friend TRoom::~TRoom(); //friend class TMap;//bool TMap::restore(QString location); diff --git a/src/dlgRoomExits.cpp b/src/dlgRoomExits.cpp index a2a7546d50c..ee9267a4e1e 100644 --- a/src/dlgRoomExits.cpp +++ b/src/dlgRoomExits.cpp @@ -302,7 +302,9 @@ void dlgRoomExits::save() if (nw->isEnabled() && nw->text().size() > 0 && mpHost->mpMap->mpRoomDB->getRoom(nw->text().toInt()) != 0 ) { // There IS a valid exit on the dialogue in this direction - pR->setExit( nw->text().toInt(), DIR_NORTHWEST ); // So store it + if( originalExits.value( DIR_NORTHWEST )->destination != nw->text().toInt() ) { + pR->setExit( nw->text().toInt(), DIR_NORTHWEST ); // Destination is different - so store it + } if (pR->hasExitStub(DIR_NORTHWEST)) // And ensure that stub exit is cleared if set pR->setExitStub(DIR_NORTHWEST, false); if (weight_nw->value()) // And store any weighing specifed @@ -310,7 +312,9 @@ void dlgRoomExits::save() else pR->setExitWeight( QStringLiteral("nw"), 0); } else { // No valid exit on the dialogue - pR->setExit( -1, DIR_NORTHWEST ); // So ensure the value for no exit is stored + if( originalExits.value( DIR_NORTHWEST )->destination > 0 ) { + pR->setExit( -1, DIR_NORTHWEST ); // Destination has been deleted So ensure the value for no exit is stored + } if (stub_nw->isChecked() != pR->hasExitStub(DIR_NORTHWEST)) // Does the stub exit setting differ from what is stored pR->setExitStub(DIR_NORTHWEST, stub_nw->isChecked()); // So change stored idea to match @@ -322,7 +326,9 @@ void dlgRoomExits::save() } if (n->isEnabled() && n->text().size() > 0 && mpHost->mpMap->mpRoomDB->getRoom(n->text().toInt()) != 0 ) { - pR->setExit( n->text().toInt(), DIR_NORTH ); + if( originalExits.value( DIR_NORTH )->destination != n->text().toInt() ) { + pR->setExit( n->text().toInt(), DIR_NORTH ); + } if (pR->hasExitStub(DIR_NORTH)) pR->setExitStub(DIR_NORTH, false); if (weight_n->value()) @@ -330,7 +336,9 @@ void dlgRoomExits::save() else pR->setExitWeight( QStringLiteral("n"), 0); } else { - pR->setExit( -1, DIR_NORTH ); + if( originalExits.value( DIR_NORTH )->destination > 0 ) { + pR->setExit( -1, DIR_NORTH ); + } if (stub_n->isChecked() != pR->hasExitStub(DIR_NORTH)) pR->setExitStub(DIR_NORTH, stub_n->isChecked()); pR->setExitWeight( QStringLiteral("n"), 0); @@ -341,7 +349,9 @@ void dlgRoomExits::save() } if (ne->isEnabled() && ne->text().size() > 0 && mpHost->mpMap->mpRoomDB->getRoom(ne->text().toInt()) != 0 ) { - pR->setExit( ne->text().toInt(), DIR_NORTHEAST ); + if( originalExits.value( DIR_NORTHEAST )->destination != ne->text().toInt() ) { + pR->setExit( ne->text().toInt(), DIR_NORTHEAST ); + } if (pR->hasExitStub(DIR_NORTHEAST)) pR->setExitStub(DIR_NORTHEAST, false); if (weight_ne->value()) @@ -349,7 +359,9 @@ void dlgRoomExits::save() else pR->setExitWeight( QStringLiteral("ne"), 0); } else { - pR->setExit( -1, DIR_NORTHEAST ); + if( originalExits.value( DIR_NORTHEAST )->destination > 0 ) { + pR->setExit( -1, DIR_NORTHEAST ); + } if (stub_ne->isChecked() != pR->hasExitStub(DIR_NORTHEAST)) pR->setExitStub(DIR_NORTHEAST, stub_ne->isChecked()); pR->setExitWeight( QStringLiteral("ne"), 0); @@ -360,7 +372,9 @@ void dlgRoomExits::save() } if (up->isEnabled() && up->text().size() > 0 && mpHost->mpMap->mpRoomDB->getRoom(up->text().toInt()) != 0 ) { - pR->setExit( up->text().toInt(), DIR_UP ); + if( originalExits.value( DIR_UP )->destination != up->text().toInt() ) { + pR->setExit( up->text().toInt(), DIR_UP ); + } if (pR->hasExitStub(DIR_UP)) pR->setExitStub(DIR_UP, false); if (weight_up->value()) @@ -368,7 +382,9 @@ void dlgRoomExits::save() else pR->setExitWeight( QStringLiteral("up"), 0); } else { - pR->setExit( -1, DIR_UP ); + if( originalExits.value( DIR_UP )->destination > 0 ) { + pR->setExit( -1, DIR_UP ); + } if (stub_up->isChecked() != pR->hasExitStub(DIR_UP)) pR->setExitStub(DIR_UP, stub_up->isChecked()); pR->setExitWeight( QStringLiteral("up"), 0); @@ -379,7 +395,9 @@ void dlgRoomExits::save() } if (w->isEnabled() && w->text().size() > 0 && mpHost->mpMap->mpRoomDB->getRoom(w->text().toInt()) != 0 ) { - pR->setExit( w->text().toInt(), DIR_WEST ); + if( originalExits.value( DIR_WEST )->destination != w->text().toInt() ) { + pR->setExit( w->text().toInt(), DIR_WEST ); + } if (pR->hasExitStub(DIR_WEST)) pR->setExitStub(DIR_WEST, false); if (weight_w->value()) @@ -387,7 +405,9 @@ void dlgRoomExits::save() else pR->setExitWeight( QStringLiteral("w"), 0); } else { - pR->setExit( -1, DIR_WEST ); + if( originalExits.value( DIR_WEST )->destination > 0 ) { + pR->setExit( -1, DIR_WEST ); + } if (stub_w->isChecked() != pR->hasExitStub(DIR_WEST)) pR->setExitStub(DIR_WEST, stub_w->isChecked()); pR->setExitWeight( QStringLiteral("w"), 0); @@ -398,7 +418,9 @@ void dlgRoomExits::save() } if (e->isEnabled() && e->text().size() > 0 && mpHost->mpMap->mpRoomDB->getRoom(e->text().toInt()) != 0 ) { - pR->setExit( e->text().toInt(), DIR_EAST ); + if( originalExits.value( DIR_EAST )->destination != e->text().toInt() ) { + pR->setExit( e->text().toInt(), DIR_EAST ); + } if (pR->hasExitStub(DIR_EAST)) pR->setExitStub(DIR_EAST, false); if (weight_e->value()) @@ -406,7 +428,9 @@ void dlgRoomExits::save() else pR->setExitWeight( QStringLiteral("e"), 0); } else { - pR->setExit( -1, DIR_EAST ); + if( originalExits.value( DIR_EAST )->destination > 0 ) { + pR->setExit( -1, DIR_EAST ); + } if (stub_e->isChecked() != pR->hasExitStub(DIR_EAST)) pR->setExitStub(DIR_EAST, stub_e->isChecked()); pR->setExitWeight( QStringLiteral("e"), 0); @@ -417,7 +441,9 @@ void dlgRoomExits::save() } if (down->isEnabled() && down->text().size() > 0 && mpHost->mpMap->mpRoomDB->getRoom(down->text().toInt()) != 0 ) { - pR->setExit( down->text().toInt(), DIR_DOWN ); + if( originalExits.value( DIR_DOWN )->destination != down->text().toInt() ) { + pR->setExit( down->text().toInt(), DIR_DOWN ); + } if (pR->hasExitStub(DIR_DOWN)) pR->setExitStub(DIR_DOWN, false); if (weight_down->value()) @@ -425,7 +451,9 @@ void dlgRoomExits::save() else pR->setExitWeight( QStringLiteral("down"), 0); } else { - pR->setExit( -1, DIR_DOWN ); + if( originalExits.value( DIR_DOWN )->destination > 0 ) { + pR->setExit( -1, DIR_DOWN ); + } if (stub_down->isChecked() != pR->hasExitStub(DIR_DOWN)) pR->setExitStub(DIR_DOWN, stub_down->isChecked()); pR->setExitWeight( QStringLiteral("down"), 0); @@ -436,7 +464,9 @@ void dlgRoomExits::save() } if (sw->isEnabled() && sw->text().size() > 0 && mpHost->mpMap->mpRoomDB->getRoom(sw->text().toInt()) != 0 ) { - pR->setExit( sw->text().toInt(), DIR_SOUTHWEST ); + if( originalExits.value( DIR_SOUTHWEST )->destination != sw->text().toInt() ) { + pR->setExit( sw->text().toInt(), DIR_SOUTHWEST ); + } if (pR->hasExitStub(DIR_SOUTHWEST)) pR->setExitStub(DIR_SOUTHWEST, false); if (weight_sw->value()) @@ -444,7 +474,9 @@ void dlgRoomExits::save() else pR->setExitWeight( QStringLiteral("sw"), 0); } else { - pR->setExit( -1, DIR_SOUTHWEST ); + if( originalExits.value( DIR_SOUTHWEST )->destination > 0 ) { + pR->setExit( -1, DIR_SOUTHWEST ); + } if (stub_sw->isChecked() != pR->hasExitStub(DIR_SOUTHWEST)) pR->setExitStub(DIR_SOUTHWEST, stub_sw->isChecked()); pR->setExitWeight( QStringLiteral("sw"), 0); @@ -455,7 +487,9 @@ void dlgRoomExits::save() } if (s->isEnabled() && s->text().size() > 0 && mpHost->mpMap->mpRoomDB->getRoom(s->text().toInt()) != 0 ) { - pR->setExit( s->text().toInt(), DIR_SOUTH ); + if( originalExits.value( DIR_SOUTH )->destination != s->text().toInt() ) { + pR->setExit( s->text().toInt(), DIR_SOUTH ); + } if (pR->hasExitStub(DIR_SOUTH)) pR->setExitStub(DIR_SOUTH, false); if (weight_s->value()) @@ -463,7 +497,9 @@ void dlgRoomExits::save() else pR->setExitWeight( QStringLiteral("s"), 0); } else { - pR->setExit( -1, DIR_SOUTH ); + if( originalExits.value( DIR_SOUTH )->destination > 0 ) { + pR->setExit( -1, DIR_SOUTH ); + } if (stub_s->isChecked() != pR->hasExitStub(DIR_SOUTH)) pR->setExitStub(DIR_SOUTH, stub_s->isChecked()); pR->setExitWeight( QStringLiteral("s"), 0); @@ -474,7 +510,9 @@ void dlgRoomExits::save() } if (se->isEnabled() && se->text().size() > 0 && mpHost->mpMap->mpRoomDB->getRoom(se->text().toInt()) != 0 ) { - pR->setExit( se->text().toInt(), DIR_SOUTHEAST ); + if( originalExits.value( DIR_SOUTHEAST )->destination != se->text().toInt() ) { + pR->setExit( se->text().toInt(), DIR_SOUTHEAST ); + } if (pR->hasExitStub(DIR_SOUTHEAST)) pR->setExitStub(DIR_SOUTHEAST, false); if (weight_se->value()) @@ -482,7 +520,9 @@ void dlgRoomExits::save() else pR->setExitWeight( QStringLiteral("se"), 0); } else { - pR->setExit( -1, DIR_SOUTHEAST ); + if( originalExits.value( DIR_SOUTHWEST )->destination > 0 ) { + pR->setExit( -1, DIR_SOUTHEAST ); + } if (stub_se->isChecked() != pR->hasExitStub(DIR_SOUTHEAST)) pR->setExitStub(DIR_SOUTHEAST, stub_se->isChecked()); pR->setExitWeight( QStringLiteral("se"), 0); @@ -493,7 +533,9 @@ void dlgRoomExits::save() } if (in->isEnabled() && in->text().size() > 0 && mpHost->mpMap->mpRoomDB->getRoom(in->text().toInt()) != 0 ) { - pR->setExit( in->text().toInt(), DIR_IN ); + if( originalExits.value( DIR_IN )->destination != in->text().toInt() ) { + pR->setExit( in->text().toInt(), DIR_IN ); + } if (pR->hasExitStub(DIR_IN)) pR->setExitStub(DIR_IN, false); if (weight_in->value()) @@ -501,7 +543,9 @@ void dlgRoomExits::save() else pR->setExitWeight( QStringLiteral("in"), 0); } else { - pR->setExit( -1, DIR_IN ); + if( originalExits.value( DIR_IN )->destination > 0 ) { + pR->setExit( -1, DIR_IN ); + } if (stub_in->isChecked() != pR->hasExitStub(DIR_IN)) pR->setExitStub(DIR_IN, stub_in->isChecked()); pR->setExitWeight( QStringLiteral("in"), 0); @@ -512,7 +556,9 @@ void dlgRoomExits::save() } if (out->isEnabled() && out->text().size() > 0 && mpHost->mpMap->mpRoomDB->getRoom(out->text().toInt()) != 0 ) { - pR->setExit( out->text().toInt(), DIR_OUT ); + if( originalExits.value( DIR_OUT )->destination != out->text().toInt() ) { + pR->setExit( out->text().toInt(), DIR_OUT ); + } if (pR->hasExitStub(DIR_OUT)) pR->setExitStub(DIR_OUT, false); if (weight_out->value()) @@ -520,7 +566,9 @@ void dlgRoomExits::save() else pR->setExitWeight( QStringLiteral("out"), 0); } else { - pR->setExit( -1, DIR_OUT ); + if( originalExits.value( DIR_OUT )->destination > 0 ) { + pR->setExit( -1, DIR_OUT ); + } if (stub_out->isChecked() != pR->hasExitStub(DIR_OUT)) pR->setExitStub(DIR_OUT, stub_out->isChecked()); pR->setExitWeight( QStringLiteral("out"), 0);