Mudlet / Mudlet

⚔️ A cross-platform, open source, and super fast MUD client with scripting in Lua
https://mudlet.org
GNU General Public License v2.0
736 stars 268 forks source link

Mapper: Custom exit lines, whose exits have commands matching normal directions, are not saved on profile exit #5143

Closed Serity closed 1 week ago

Serity commented 3 years ago

Brief summary of issue / Description of requested feature:

Adding a 'custom exit line' between two rooms, when the source custom exit line's command matches a normal exit ("s", "out"... but not "south"), causes it to be removed upon client closing.

Steps to reproduce the issue / Reasons for adding feature:

  1. Create two rooms.
  2. Link the two rooms with a special exit. Make the command for this special exit "s", or "out", or similar...
  3. Right-click the room with said special exit and 'Custom Exit Line'.
  4. Create the arrow on the custom exit.
  5. Save your map and restart the client.
  6. Your custom exit line is now gone. The 'custom exit line' GUI does not have a check in the 'has custom line' box, so it's actually gone, rather than just not rendering.

Error output / Expected result of feature

The custom exit line should remain.

Extra information, such as Mudlet version, operating system and ideas for how to solve / implement:

tested on 4.11.2 as well as ptb 2021-04-11-027c5 and testing-pr5135-81258fdd

SlySven commented 3 years ago

When you created the custom line - how did you finish the drawing process? You did right click and click "finish" didn't you? I cannot reproduce this error with the current development code (as of d274769a706e063f2299dbbf378b5c3d5063dd38)...

Serity commented 3 years ago

I did~. Here's a step-by-step, for me, on the development code you have just linked - testing-pr5135-81258fdd:

Map with no custom exit line image

Go to custom exit line... image

Select the exit... image

Click one time to create an arrow, right-click and "finish". image

Let's create a new room just to confirm that we are indeed saving the map (which cannot be done without "finishing" the drawn line)... image image

Confirm that the custom exit line was correctly created... image

Save the map...

<!> Saved map to 04-11-2021#23-05-11_map_auto.dat!
(this also updates the map.dat in the main folder)

Close the profile... image

Restart the game and reopen the profile... image

Custom exit line is nowhere to be found! However, the room we created still is there. The box is unchecked, too. image image

Serity commented 3 years ago

That being said, on further inspection of my map - it seems that the only special exits that break are ones where I am using out as the 'special exit' command, which very frequently for my map are exits to other zones, hence the false positive - but also the occasional same-map out also breaks (and yes, I know of the map's proper in/out support, and still choose to use special exits anyways). So I'll update the first post, I think.

SlySven commented 3 years ago

:thinking: Only the out one... now that is suggestive. I've got a pending PR that tidies up some code in the dlgRoomExit::save(...) and I noticed that the code for the out one (it was the last) was a bit different than the others. Depending on which OS you have (I hope it is Linux) - could you try the listed test version for #5145 and see if that behaves the same or works (for other exit directions as well)?

Hang on - you said "out" as an exit direction - well you can't use THAT I think - because it clashes with the normal exit direction out so any custom exit line with that "key" will be associated with a normal exit direction. The same is true for up, down and in...!

I do not think we can fix/handle that - it is a limitation of our way of splitting exits into normal and special ones internally. The normal ones are coded/keyed as ain integer which is easier to code for internally whereas the special ones are coded/keyed as strings. In some parts of the codebase they are combined into a single container (exit weights, custom lines and doors) and in those the following keys (now all lowercase but in the past some were UPPERCASE) are reserved for normal exits and cannot safely be used for specials:

Sorry, but this is a: wontfix (though it is actually a can't fix)...

:slightly_frowning_face:

vadi2 commented 3 years ago

Can't we have custom exit lines on normal exits though?

Serity commented 3 years ago

My exact reasoning for using "out" special exits was as such:

My main problem with using 'out' exits re:issue #5143 is that, at least with the default mapper script, it always creates new rooms when you use 'out' syntax instead of automatically detecting the room (like it would with NSEWUD just due to location coordinates). Even if I adjusted the script to allow for out/in autodetection like portals, I'd also have to set it up so out -f could forcibly create a new room in case the detection is incorrectly detecting an old room - and then those changes would be wiped on a default mapper update anyways.

I think, though, that if Mudlet will not work properly creating custom lines with special exits that have existing-directions-as-names, the UI should reflect this in some way (perhaps prohibiting clicking the special exit in the custom line), because the current behavior not saving properly feels and appears like a bug (although there is also the use case that someone may want a "north" exit but to style the exit in a non-default manner (ergo special exit) rather than the default exit style...).

I did find a workaround suitable for my personal use case, so I'm leaving note of it here in case anyone else encounters a similar problem. Setting the special exit to out, that is, out with a trailing space, using add portal out, allows the (default) mapper to properly move through the exit while using both out and out (assuming your MUD would ignore the trailing spaces), and Mudlet properly retains special exit custom lines for this exit after a reload.

I switched all my exits from out to out with a simple script:

for i,v in pairs(getRooms()) do 
  local c = getSpecialExitsSwap(i) 
  if (c.out) then
    local dest = c.out
    addSpecialExit(i,dest,"out ")
    removeSpecialExit(i,"out")
    count = count+1
  end
end
echo(f"* Switched {count} rooms from 'out' to 'out ' for arrow compatibility")
SlySven commented 3 years ago

Can't we have custom exit lines on normal exits though?

Yes we can - which is why the user cannot use those specific names/identifiers/keys for special exits - as they are used for those normal exit directions on:

Note:

... Setting the special exit to out, that is, out with a trailing space ...

That may backfire - IIRC there is code buried in Mudlet somewhere that trims white-space from some things - and I can't remember whether that includes exit identifiers... as they say "your mileage may vary"

vadi2 commented 3 years ago

Yeah we should improve the UI here - and also support remapping normal exits with special ones, because why not?

SlySven commented 3 years ago

and also support remapping normal exits with special ones, because why not?

That is not going to happen without a major redesign of the TRoom class - I tried something like that a couple of years ago (combining normal and special exits into a single QPair<quint8, QString> key/identifier where the first pair is 1 to 12 for normal exits and 13 for specials, and the second is empty/null for normal exits and the name/command for the specials) - but it means major reworking of several other classes...

:worried:

ZookaOnGit commented 1 week ago

won't fix