Refactor: store special exits and their lock status separately by SlySven · Pull Request #4526 · Mudlet/Mudlet

SlySven · 2020-12-24T17:06:09Z

This will enable some simplification of code.

During development it became clear that the Lua API getSpecialExits(...) function was a bit defective and did not behave as documented in the Wiki: it was only showing one special exit at random that led to a particular exit room in the (admittedly unlikely) event of there being more than one. It also used the special exit name/command as a key in a sub-table with the special exit lock status of that exit as a "0" or "1" string value. Edit: which does not correspond to the current documentation!

This PR repairs the above function by adding an optional boolean argument that:

if omitted or false, replicates the previous behaviour but if there is more than one special exit to the same room it always picks one with the lowest exit weight that is unlocked or if there is none it picks one with the lowest weight that is locked. This will be compatible with old scripts.
if true, returns ALL the exits in the sub-table that lead to the particular room id that is the key in the main table, again those exit commands are the keys with a value being a "0" or "1" depending on whether the exit is unlocked or locked respectively.

For the record, the original implementation of special exits was introduced in commit: e0ba28d and that was supported by the addition of map format version 6. Locking of Special Exits was added in somewhere between: 19f8563 and: 070912e (which revised the map format to 11).

Also:

use a couple of const QStrings as templates in the dlgRoomExit.cpp file to remove 95 duplicated QStringLiterals from the read-only code segment of the compile object file.
add const where relevant to some TRoom methods.
work harder to ensure than when a special exit is deleted from a TRoom then elements that were related to it are also cleaned up.
prepare to save the new TRoom data structures in the next Mudlet map file format (21) when it is enabled. In the meantime a workaround to convert the in-game data to the current format is utilised for all current map formats Mudlet can currently use. This will impact a little on the save/loading speeds but that is the cost of simplifying the code that works with special exits elsewhere in the application.
revise and extend the error handling for the room special exit functions generally so that they confirm to our throwing an error on argument type issue (and reporting the faulty argument) and returning nil plus an errormessage for a run-time value problem.

Signed-off-by: Stephen Lyons slysven@virginmedia.com

This will enable some simplification of code. During development it became clear that the Lua API `getSpecialExits(...)` function was a bit defective and did not behave as documented in the Wiki: it was only showing one special exit at random that led to a particular exit room in the (admittedly unlikely) event of there being more than one. It also used the special exit name/command as a key in a sub-table with the special exit lock status of that exit as a "0" or "1" string value. This PR repairs the above function by adding an optional boolean argument that: * if omitted or false, replicates the previous behaviour but if there is more than one special exit to the same room it always picks one with the lowest exit weight that is unlocked or if there is none it picks one with the lowest weight that is locked. This will be compatible with old scripts. * if true, returns ALL the exits in the sub-table that lead to the particular room id that is the key in the main table, again those exit commands are the keys with a value being a "0" or "1" depending on whether the exit is unlocked or locked respectively. For the record, the original implementation of special exits was introduced in commit: e0ba28d and that was supported by the addition of map format version 6. Locking of Special Exits was added in somewhere between: 19f8563 and: 070912e (which revised the map format to 11). Also: * use a couple of `const QString`s as templates in the `dlgRoomExit.cpp` file to remove 95 duplicated `QStringLiterals` from the read-only code segment of the compile object file. * add `const` where relevant to some `TRoom` methods. * work harder to ensure than when a special exit is deleted from a `TRoom` then elements that were related to it are also cleaned up. * prepare to save the new `TRoom` data structures in the next Mudlet map file format (21) when it is enabled. In the meantime a workaround to convert the in-game data to the current format is utilised for all current map formats Mudlet can currently use. This will impact a little on the save/loading speeds but that is the cost of simplifying the code that works with special exits elsewhere in the application. * revise and extend the error handling for the room special exit functions generally so that they confirm to our throwing an error on argument type issue (and reporting the faulty argument) and returning `nil` plus an error message for a run-time value problem. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>

add-deployment-links · 2020-12-24T17:06:57Z

Hey there! Thanks for helping Mudlet improve. 🌟

Test versions

You can directly test the changes here:

linux: https://make.mudlet.org/snapshots/4ad758/Mudlet-4.10.1-testing-pr4526-73ec6175-linux-x64.AppImage.tar
osx: (temporarily out of order)
windows: https://make.mudlet.org/snapshots/f884ae/Mudlet-4.10.1-testing-pr4526-73ec6175-windows.zip

No need to install anything - just unzip and run.
Let us know if it works well, and if it doesn't, please give details.

Cause a build failure but only in some (Clang) compilers: "Error: /Users/runner/work/Mudlet/Mudlet/src/TLuaInterpreter.cpp:5382:97: error: cannot pass object of non-trivial type 'QString' through variadic function; call will abort at runtime [-Wnon-pod-varargs] in: lua_pushfstring(L, "the special exit name/command \"%s\" does not exist in room id %d", dir, fromRoomID); Signed-off-by: Stephen Lyons <slysven@virginmedia.com>

…eparately

vadi2

Great improvements 👍

vadi2 · 2020-12-28T13:06:29Z

src/T2DMap.cpp

-                    qWarning(R"(T2DMap::slot_customLineProperties() - WARNING: missing command "%s" from custom lines for room id %i)",
-                             qPrintable(exit), room->getId());
-                }
+                qWarning().noquote().nospace() << "T2DMap::slot_customLineProperties() WARNING - missing command \"" << exit << "\" from custom lines for room id " << room->getId();


I am not sure what this error message actually means

Haven't looked at the surrounding code but I'd expect that to mean that one or more of the customExitLine(|Style|Color|EndsInArrow) QMaps has a key that is NOT an exit - from what I recall of looking through the code it didn't always clear out those details when an exit in a particular direction was removed,,,,

Ah ok, can you clarify the message to be better?

Changed to:

qWarning().noquote().nospace() << "T2DMap::slot_customLineProperties() WARNING - missing no exit \"" << exit << "\" to be associated with a custom exit line with that designation in room id " << room->getId();

vadi2 · 2020-12-28T13:08:46Z

src/TLuaInterpreter.cpp

+
+    bool showAllExits = false;
+    if (lua_gettop(L) > 1) {
+        if (!lua_isboolean(L, 2)) {


I think it is better to be consistent in how we deal with bad functions: create a getSpecialExits1 rather than complicating the code, and usage, of the original.

It turns out that due to insufficient testing data I didn't find out that the output being displayed (from using lua display(getSpecialExits()) was not as wrong as I thought - because I had chosen 1, 2, 3 as three different special exit room ids the list format output from display(...) which did NOT show the keys was correct. The only defect was that for multiple special exits with the same destination it just listed ONE of them at random. So the fix - to show (one of) the best (unlocked and lowest exit weighting) one is entirely backwards compatible when the existing function is used as is...

All that needs to be done is to fix the detail of the example code and to add a warning that:

"In the corner case where there is more than one special exit from this room to the same destination room, only one room is shown chosen at random prior to Mudlet 4.y.z - after that point only the best, in terms of being unlocked and of lowest exit weighting (or one of the best if there is a tie) special exits to that room is listed unless the optional true argument is given which will cause all of them to be listed."

OK, that sounds simple.

vadi2 · 2020-12-28T13:09:24Z

src/TRoom.cpp

 }

-void TRoom::setId(int _id)
+void TRoom::setId(const int roomId)


Signed-off-by: Stephen Lyons <slysven@virginmedia.com>

SlySven · 2020-12-31T22:41:20Z

📚 Wiki updated to match: https://wiki.mudlet.org/w/Manual:Mapper_Functions#getSpecialExits .

SlySven · 2021-01-02T03:13:49Z

Although "This branch has no conflicts with the base branch" it will clobber #4546 - but that is only a draft at this point so I can sort the clash out shortly...

Since Mudlet#4526 was merged into the development branch 4 days ago reading of maps will LOCK ALL special exits and for really old formats (prior to 11) will chop the first letter of the special exit name/command each time the file is loaded... This was caused by a copy-paste area in the code the regenerated the older style of storing the special exits and their locked status that was used prior to that PR. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>

Since Mudlet#4526 was merged into the development branch 4 days ago reading of maps will prepend a '0' before the first letter of the special exit name/command for every special exit that is NOT locked each time the file is loaded... This was caused by a copy-paste area in the code the regenerated the older style of storing the special exits and their locked status that was used prior to that PR. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>

Since Mudlet#4526 was merged into the development branch 4 days ago reading of maps will prepend a '0' before the first letter of the special exit name/command for every special exit that is NOT locked each time the file is loaded... This was caused by a copy-paste error in the code the regenerated the older style of storing the special exits and their locked status that was used prior to that PR. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>

Since Mudlet#4526 was merged into the development branch 4 days ago reading of maps will prepend a '0' before the first letter of the special exit name/command for every special exit that is NOT locked each time the file is loaded... This was caused by a copy-paste error in the code that converted the older style of storing the special exits and their locked status that was used prior to that PR. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>

Since Mudlet#4526 was merged into the development branch 4 days ago reading of old maps (formats *before* *8*) built from Mudlet prior to 8e0f9db (merged on 2011/01/24) will chop the first character from the name/command of every special exit... This was caused by a copy-paste error in the code that converted the older style of storing the special exits and their locked status that was used prior to that PR. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>

…ad (#4574) Since #4526 was merged into the development branch 4 days ago reading of old maps (formats before 8) built from Mudlet prior to 8e0f9db (merged on 2011/01/24) will chop the first character from the name/command of every special exit... This was caused by a copy-paste error in the code that converted the older style of storing the special exits and their locked status that was used prior to that PR. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>

@ryanstadther

This is an error that slipped in as a result of Mudlet#4526 and resulted in the exit room id being stored as the special exit name in the BGL graph that is built for the A* search algorithm to work on. Consquently if that route was used in a path the special exit command produced for that step was garbage which also tripped off warnings about invalid characters being sent to the MUD as an exit direction. Thanks to @ryanstadther for bringing this to my attention! Signed-off-by: Stephen Lyons <slysven@virginmedia.com>

@ryanstadther

This is an error that slipped in as a result of #4526 and resulted in the exit room id being stored as the special exit name in the BGL graph that is built for the A* search algorithm to work on. Consquently if that route was used in a path the special exit command produced for that step was garbage which also tripped off warnings about invalid characters being sent to the MUD as an exit direction. Thanks to @ryanstadther for bringing this to my attention! Signed-off-by: Stephen Lyons <slysven@virginmedia.com>

…t#4526) This will enable some simplification of code. During development it became clear that the Lua API `getSpecialExits(...)` function was a bit defective and did not behave as documented in the Wiki: it was only showing one special exit at random that led to a particular exit room in the (admittedly unlikely) event of there being more than one. It also used the special exit name/command as a key in a sub-table with the special exit lock status of that exit as a "0" or "1" string value. This PR repairs the above function by adding an optional boolean argument that: * if omitted or false, replicates the previous behaviour but if there is more than one special exit to the same room it always picks one with the lowest exit weight that is unlocked or if there is none it picks one with the lowest weight that is locked. This will be compatible with old scripts. * if true, returns ALL the exits in the sub-table that lead to the particular room id that is the key in the main table, again those exit commands are the keys with a value being a "0" or "1" depending on whether the exit is unlocked or locked respectively. For the record, the original implementation of special exits was introduced in commit: e0ba28d and that was supported by the addition of map format version 6. Locking of Special Exits was added in somewhere between: 19f8563 and: 070912e (which revised the map format to 11). Also: * use a couple of `const QString`s as templates in the `dlgRoomExit.cpp` file to remove 95 duplicated `QStringLiterals` from the read-only code segment of the compile object file. * add `const` where relevant to some `TRoom` methods. * work harder to ensure than when a special exit is deleted from a `TRoom` then elements that were related to it are also cleaned up. * prepare to save the new `TRoom` data structures in the next Mudlet map file format (21) when it is enabled. In the meantime a workaround to convert the in-game data to the current format is utilised for all current map formats Mudlet can currently use. This will impact a little on the save/loading speeds but that is the cost of simplifying the code that works with special exits elsewhere in the application. * revise and extend the error handling for the room special exit functions generally so that they confirm to our throwing an error on argument type issue (and reporting the faulty argument) and returning `nil` plus an error message for a run-time value problem. Signed-off-by: Stephen Lyons <slysven@virginmedia.com> Co-authored-by: Vadim Peretokin <vperetokin@gmail.com>

…ad (Mudlet#4574) Since Mudlet#4526 was merged into the development branch 4 days ago reading of old maps (formats before 8) built from Mudlet prior to 8e0f9db (merged on 2011/01/24) will chop the first character from the name/command of every special exit... This was caused by a copy-paste error in the code that converted the older style of storing the special exits and their locked status that was used prior to that PR. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>

@ryanstadther

…#4722) This is an error that slipped in as a result of Mudlet#4526 and resulted in the exit room id being stored as the special exit name in the BGL graph that is built for the A* search algorithm to work on. Consquently if that route was used in a path the special exit command produced for that step was garbage which also tripped off warnings about invalid characters being sent to the MUD as an exit direction. Thanks to @ryanstadther for bringing this to my attention! Signed-off-by: Stephen Lyons <slysven@virginmedia.com>

SlySven added the enhancement label Dec 24, 2020

SlySven assigned vadi2 Dec 24, 2020

SlySven requested a review from a team as a code owner December 24, 2020 17:06

SlySven requested review from a team December 24, 2020 17:06

SlySven and others added 2 commits December 24, 2020 19:41

Merge branch 'development' into UpgradeMap_storeSpecialExitsAndLocksS…

5071911

…eparately

vadi2 reviewed Dec 28, 2020

View reviewed changes

vadi2 assigned SlySven and unassigned vadi2 Dec 28, 2020

SlySven assigned vadi2 and unassigned SlySven Dec 29, 2020

vadi2 approved these changes Dec 30, 2020

View reviewed changes

Revise: update warning message following peer review

73ec617

Signed-off-by: Stephen Lyons <slysven@virginmedia.com>

vadi2 approved these changes Dec 31, 2020

View reviewed changes

SlySven merged commit 507aa21 into Mudlet:development Dec 31, 2020

SlySven deleted the UpgradeMap_storeSpecialExitsAndLocksSeparately branch December 31, 2020 22:06

SlySven mentioned this pull request Jan 3, 2021

BugFix: stop special exits being modified when old format maps are read #4574

Merged

SlySven mentioned this pull request Jan 30, 2021

Enhance: import/export map as JSON #4546

Merged

SlySven mentioned this pull request Jan 30, 2021

BugFix: repair special exits in speedwalks with corrupt names #4722

Merged

SlySven mentioned this pull request Sep 26, 2021

Add: Automatically draw area exit names #5456

Closed

gang929 added a commit to gang929/Mudlet that referenced this pull request Oct 1, 2021

adapt for Mudlet#4526

9d6650e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Refactor: store special exits and their lock status separately#4526

Refactor: store special exits and their lock status separately#4526
SlySven merged 4 commits intoMudlet:developmentfrom
SlySven:UpgradeMap_storeSpecialExitsAndLocksSeparately

SlySven commented Dec 24, 2020 •

edited

Loading

Uh oh!

add-deployment-links bot commented Dec 24, 2020 •

edited

Loading

Uh oh!

vadi2 left a comment

Uh oh!

vadi2 Dec 28, 2020

Uh oh!

SlySven Dec 29, 2020

Uh oh!

vadi2 Dec 30, 2020

Uh oh!

SlySven Dec 31, 2020

Uh oh!

vadi2 Dec 28, 2020

Uh oh!

SlySven Dec 29, 2020

Uh oh!

vadi2 Dec 30, 2020

Uh oh!

vadi2 Dec 28, 2020

Uh oh!

SlySven commented Dec 31, 2020

Uh oh!

SlySven commented Jan 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

SlySven commented Dec 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

add-deployment-links bot commented Dec 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test versions

Uh oh!

vadi2 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SlySven commented Dec 31, 2020

Uh oh!

SlySven commented Jan 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

SlySven commented Dec 24, 2020 •

edited

Loading

add-deployment-links bot commented Dec 24, 2020 •

edited

Loading