Improve: speed up bulk room creation #7184
Conversation
|
Hey there! Thanks for helping Mudlet improve. 🌟 Test versionsYou can directly test the changes here:
No need to install anything - just unzip and run. |
vadi2
changed the title
vadi2
added
the
needs documentation
| if (lua_gettop(L) >= 2) { | ||
| areaID = getVerifiedInt(L, __func__, 2, "areaID"); | ||
| } | ||
| // defer area calculations as all new rooms are initialised at 0,0,0 anyway |
There was a problem hiding this comment.
🤔 If we are extending room creation to put them in the wanted area directly why not go the whole hog and accept the coordinates to position them in the area as well...? Fair enough - I am not sure what the Lua API would look like for that!
There was a problem hiding this comment.
Adding room coordinates isn't slow, so leaving it be for now
| while (lua_next(L, 1) != 0) { | ||
| const int id = getVerifiedInt(L, __func__, -1, "roomID"); | ||
| if (!host.mpMap->mpRoomDB->getRoomIDList().contains(id)) { | ||
| return warnArgumentValue(L, __func__, csmInvalidRoomID.arg(id)); |
There was a problem hiding this comment.
Would it not be reasonable to identify the index (starting at 1 obviously - it is a Lua table) of the invalid room id that prompted the error - or do you consider it sufficient to report only the bogus value?
There was a problem hiding this comment.
I think just thr wrong value is enough
| if (!host.mpMap->mpRoomDB->getRoomIDList().contains(id)) { | ||
| return warnArgumentValue(L, __func__, csmInvalidRoomID.arg(id)); | ||
| } | ||
| roomIds.append(id); |
There was a problem hiding this comment.
std::set rather than a std::vector which prohibits duplicates (and is sorted as a side effect)...
| @@ -277,18 +277,18 @@ inline QDebug operator<<(QDebug debug, const TRoom* room) | |||
| } | |||
There was a problem hiding this comment.
<aside>How come you never put in code to report the custom exit line colour, whether it ends in an arrow-head and the line style?</aside>
| } | ||
| if (id <= 0) { | ||
| QString error = qsl("addRoom: illegal room id=%1. roomID must be > 0").arg(id); |
There was a problem hiding this comment.
<aside>🚨 I18n - this text is presented to the error console in the editor and other such texts displayed in this way are put through the translation system (twice in other cases - which is also suspect - the second one prepends "[MAP ERROR:] with no space after it in TMap::logError(...)!)</aside>
| QString error = qsl("addRoom: illegal room id=%1. roomID must be > 0").arg(id); | ||
| mpMap->logError(error); | ||
| } | ||
| return false; |
There was a problem hiding this comment.
We return false if the room is not created - but we do not report here if the room already exists AND that is why this method failed. Perhaps we review the usage of this method and determine whether the reporting of failures need to be unified/be made more rational?
Brief overview of PR changes/additions
Speed up bulk room creation so people can make million-map rooms quickly:
Motivation for adding to Mudlet
https://discord.com/channels/283581582550237184/1215109133150199889/1217359192835358741
Other info (issues closed, discussion etc)
Somewhere along the line we forgot that https://wiki.mudlet.org/w/Manual:Mapper_Functions#updateMap needs to be called from scripts to update the map for this very reason - performance