Unexpected UNDO Problem [Resolved Again!]

I'd blocked UNDO in a couple of my games because of problems with some of the puzzles involved. This was done by introducing my own UNDO command. Deciding this was unnecessarily restrictive I tried changing the code in the command to the form:

if (some condition) {
  msg ("Sorry, 'undo' is not available here.")
}
else {
  undo
}

This does invoke undo, but the effect is different. In particular, the map display is not updated, as used to be the case before Quest 5.8. The issue seems connected with the conditional, as all is fine if I just use undo by itself in the command???


I'm not entirely sure how the structure works around the undo command. But a few things spring to mind. Are you modifying the script on the built-in undo command, or creating your own? And in the latter case, have you set the isundo flag on the command to true?

After a quick look, I think what you want might be:

SuppressTurnscripts()
if (some condition) {
  msg ("Sorry, 'undo' is not available here.")
}
else {
  undo
  Grid_DrawPlayerInRoom (game.pov.parent)
}

(the core undo command uses if (GetBoolean (game, "gridmap")) { to check if the map is enabled before redrawing it; but I assume that if you're asking that means that there is a map. If it's available in some areas but not others, you can add the check again)


Thanks for the usual rapid response mrangel! Yes, I'm putting in my own undo command at the user script level, where I don't see an isundo flag?

My main concern is understanding why the following two command scripts have different outcomes?

undo
if (true) {
  undo
}

The script of the built-in undo command is:

  <command name="undo">
    <pattern type="string">[undo]</pattern>
    <script>
      undo
      game.suppressturnscripts = true
      if (GetBoolean (game, "gridmap")){
        Grid_DrawPlayerInRoom (game.pov.parent)
      }
    </script>
    <isundo/>
  </command>

The thing with the map is probably because the core undo command runs the undo script-statement and then redraws the map; which yours doesn't.

You might have to use code view to add the <isundo/>. It isn't shown anywhere in the editor, as far as I'm aware, because the developer didn't consider wanting to create your own undo command.

The flag tells the parser not to create a transaction before starting the command. So in practice, when you have undo in a script, it rolls back to the point where the player entered the last command without an isundo flag. So if your undo command doesn't have isundo, it may attempt to undo itself instead of the previous command.


Not sure what I was doing previously so please ignore my last message for the moment as I can't recreate what I thought I'd tested...
...anyway, I'd also misunderstood what was needed to re-implement the undo command so I'll get on with that.
Thanks again mrangel!


Selective undo now implemented in L Too:
http://textadventures.co.uk/games/view/e4hrqb_vpeo4np1a0jel9g/l-too-another-mathemagical-adventure
Anyone interested should feel free to make sure I've done it properly!


If you're playing with undo, it might be worth learning about begin transaction too. It basically creates a checkpoint that undo can roll back to.

It takes a single parameter, which is a command element (but it doesn't seem to do anything with it, so possibly any object would work).

When you use the undo script command, it restores all objects and their attributes to the values they had at the last time begin transaction was called; but doesn't resume any scripts that were running then (and so doesn't restore local variables).

The parser calls begin transaction immediately before running any player-entered command that doesn't have its isundo flag set to true.


Thanks mrangel. Within my games, I use an attribute in_puzzle and just prevent an undo if it is set. I think this avoids me having to worry about the subtleties of the underlying undo operation...though as I type this I realise an undo could be used to reverse back into a puzzle. I'd better check!


First thing that comes to my mind is something like:

undo
while (game.in_puzzle) {
  undo
}

Don't know if that would work or if it's suitable; but might restrict 'undo' by making it so that when you're doing a puzzle, 'undo' will take you all the way back to the beginning rather than step by step.


If you want to stop the user undoing a puzzle entirely, you could use a turn timeout to unset the flag a turn later; prevent the user undoing the final move of the puzzle.

Thanks, for the first game, the single undo seems fine, so I'll not update it to include the looping suggestion. I'll now see how it looks in the second game...


...oh dear! I need to rethink the use of undo. I found problems with its use in my second game (Giantkiller Too) in relation to puzzles and the above fixes aren't enough.

Just to explain the background here: up to Quest 5.8, undo didn't handle the map properly and as all my games use a map, I excluded undo. No one complained about that but with Quest 5.8 available I decided to remove unnecessary restrictions and allowed undo in all but two games that had extended puzzles. I've now looked at removing that restriction as well but had trouble with one of the games...which has also revealed issues with undo in all of my games!

In these games I allow the map and game panes to be brought in and removed dynamically. Undo is not handling these changes properly. There are several issues. For example, note the effect of the following commands issued on entry to L Too (link given above). Also undo doesn't affect the visibility of the map or games panes.

> map
The map is now shown above. Type map again to hide it.

> map
The map has been hidden. Type map again to restore it.

> undo
Undo: map

> undo
Undo: map

> undo
Undo: map
Error running script: The given key was not present in the dictionary.

All of my games are cruelty-free (there is always a way to recover from any difficulty) so I'm inclined to block the use of undo throughout.


The issue with the map is because undo only restores the state of objects and attributes. It doesn't run any javascript, or notify the frontend.

If you have an attribute to track whether the map is shown or hidden (or similar things), you would probably need to make the undo command check those attributes after undoing, and show or hide as appropriate.
For example:

SuppressTurnscripts()
if (some condition) {
  msg ("Sorry, 'undo' is not available here.")
}
else {
  undo
  Grid_DrawPlayerInRoom (game.pov.parent)
  if (GetBoolean (game, "maphidden")) {
    JS.uiHide("#gamePanel")
  }
  else {
    JS.uiShow("#gamePanel")
  }
}

It's likely that any UI Initialisation code which depends on variables that change during play will need to be copied onto the end of the undo command.


It doesn't run any javascript, or notify the frontend.

Ah, I see now that undo relies on there being code to reverse every operation performed, and as that can't be specified for 'external' operations, there will be a way for undo to fail in many games. I've not seen anyone report a problem with undo or complain if it is missing, so on balance, I'll just block it in future, though there is no rush to make this change.

Thanks for the explanation mrangel!


It shouldn't be too much of a problem, really.

The only things that don't revert are things on the frontend - which would mess up in exactly the same way when you load a saved game.
(using undo is pretty much the same as restoring a save)

In the example above, this code:

  if (GetBoolean (game, "maphidden")) {
    JS.uiHide("#gamePanel")
  }
  else {
    JS.uiShow("#gamePanel")
  }

should already be in your UI Initialisation script, so it's just a case of copying it over.

Actually, when the next version of Quest is proposed, I'll suggest the inclusion of a script, game.userinterfacesync or similar, which would be run by the core libraries both after a successful undo, and after the UI Initialisation script. Make this issue more intuitive so that this code only needs to be updated in one place.


using undo is pretty much the same as restoring a save

I did try out your suggestion of using the restore code in the sample game Cloak of Darkness,
http://textadventures.co.uk/games/view/cxbbr4mqakkylkr80ypjhg/cloak-of-darkness-another-version
but all is not well:

> map
The map is now shown above. Type MAP again to hide it.

> undo
Undo: map

> undo
Undo: map
Error running script: The given key was not present in the dictionary.

Feel free to experiment!


Just looking at the above text output, I'd say that one issue is that you can't undo a previous undo command -- that's like giving yourself a lobotomy as someone once told me.

Maybe you could figure out a way to get the UNDO command to check and see if last turn's command was also "undo"? If so, the UNDO command would tell the player that UNDO can't be used this turn.


Maybe you could figure out a way to get the UNDO command to check and see if last turn's command was also "undo"? If so, the UNDO command would tell the player that UNDO can't be used this turn.

That's what the isundo flag is for. It should undo the last transaction = the last non-undo command; so working backwards through all the things the player has done.

@DavyB Am I correct in thinking that the same thing happens if you start with "undo"? As in, the player has entered two commands and then attempted to undo three times, so there is nothing left to undo?

If you want a better error message in this case, I'd suggest a turnscript:

if (HasObject (game.pov, "currentcommandpattern")) {
  if (not GetBoolean (game.pov.currentcommandpattern, "isundo")) {
    game.hasacted = true
    destroy (this.name)
  }
}

and an undo command:

if (not GetBoolean (game, "hasacted")) {
  msg ("You have to do something before you can undo it!")
}
else … …

I suddenly thought that perhaps the existing undo might have problems so I experimented with a program having just two rooms and a north south link as follows:

<!--Saved by Quest 5.8.6836.13983-->
<asl version="580">
  <include ref="English.aslx" />
  <include ref="Core.aslx" />
  <game name="undo_test">
    <gameid>8c1f42e6-b08b-4fb3-99cc-c06169ce657a</gameid>
    <version>1.0</version>
    <firstpublished>2020</firstpublished>
  </game>
  <object name="room">
    <inherit name="editor_room" />
    <isroom />
    <object name="player">
      <inherit name="editor_object" />
      <inherit name="editor_player" />
    </object>
    <exit alias="north" to="second">
      <inherit name="northdirection" />
    </exit>
  </object>
  <object name="second">
    <inherit name="editor_room" />
    <exit alias="south" to="room">
      <inherit name="southdirection" />
    </exit>
  </object>
</asl>

and then tried going north, then south and trying undo three times in a row. The result is:

You are in a room.
You can go north.

> n

You are in a second room.
You can go south.

> s

You are in a room.
You can go north.

> undo
Undo: s

> undo
Undo: n

> undo
Undo: n
Error running script: Index was out of range. Must be non-negative and less than the size of the collection.Parameter name: index

Yep thats what I was thinking :) I think my code in the last post should give a more sensible error message.

And an idea just came to mind, how to add a 'redo' command :p It's a horrible hack tho


Humble thanks, mrangel. I've added your suggested turnscript to the Cloak of Darkness game, and updated the undo command to:

if (not GetBoolean (game, "hasacted")) {
  msg ("There is nothing to undo!")
}
else {
  undo
  game.suppressturnscripts = true
  reset_options
 }

The result initially looked perfect:
http://textadventures.co.uk/games/view/cxbbr4mqakkylkr80ypjhg/cloak-of-darkness-another-version

...but there is still an oddity connected with the use of a 'links' command that is used to flip hyperlinks on and off. If 'links' is preceded by any command it is happily undone but if it is the first command it is missed, as illustrated below.

> l
You are in the gloomy Foyer of the Opera House. The room is empty and there is nobody else around. There are large entrance doors to the north, with smaller doors to the south and west.

> links
Hyperlinks have now been revealed. Type LINKS again to hide them.

> undo
Undo: links

> undo
Undo: l

> undo
There is nothing to undo!

> links
Hyperlinks have now been revealed. Type LINKS again to hide them.

> undo
There is nothing to undo!

I was given the code to manage links through the forum and happily admit that I don't really know what is going on! Can you see that code? If not, I'll post it.


...now see that commands that include game.suppressturnscripts = true appear to be treated like any other command in an undo sequence EXCEPT when the first in the sequence. Taking this out of the LINKS command in Cloak of Darkness game appears to fix it (now consistent with MAP and PANES) but perhaps there are important consequences that I've not yet discovered?

I see game.suppressturnscripts = true is used in SAVE and some other standard commands.


...just noticed another little issue with the standard undo operation. The location name, if shown at the top of the screen, is not updated following an undo that moves the player?


Following the above discussion, I've returned to the two games that include extended periods when a player is within a puzzle. The following code nicely handles the prevention of UNDO while in a puzzle and the case where a player backtracks into a puzzle. One issue, however, is that allowing UNDO now gets around a policy of charging for hints in a game (i.e. use HINT and then UNDO). A heavy-handed way round this problem is to 'charge' for each undo as if it were a hint but is there a more subtle / better way?

if (game.in_puzzle) {
  report_error ("The UNDO operation is not available while completing a puzzle.")
}
else if (not GetBoolean (game, "hasacted")) {
  msg ("There is nothing to undo!")
}
else {
  undo
  game.suppressturnscripts = true
  reset_options
  while (game.in_puzzle) {
    undo
    game.suppressturnscripts = true
    reset_options
  }
}

A heavy-handed way round this problem is to 'charge' for each undo as if it were a hint but is there a more subtle / better way?

If your hint command uses a custom currency like hintpoints, you could do something like:

latest_points = game.hintpoints
undo
if (latest_points < game.hintpoints) {
  // if undoing a turn has given the player more points
  // reset it to what it was before
  game.hintpoints = latest_points
}
// all the other stuff to sync the map, location, etc goes here

If you use a currency to buy hints that is also used for other things, you'd presumably need to make a separate flag or similar. You can use a local variable within the undo command to check whether a certain variable has been changed by the undo action, and then update the attributes accordingly.

I did think about printing a message "Hint points spent are not restored by undo!" - but then realised that if they undo the action before the hint, it would probably trigger the message again as it restores their points to what they were the first time they reached that point.

You could also use this to make an undo command that keeps track of some stuff even if you undo. Could be pretty meta to have characters who get angry because it's rude to go back in time while talking to them, or something.

EDIT:

...just noticed another little issue with the standard undo operation. The location name, if shown at the top of the screen, is not updated following an undo that moves the player?

You need to add a line after the undo:

JS.updateLocation(CapFirst(GetDisplayName(game.pov.parent)))

Many thanks mrangel! That seems to work very well!! The final code is:

if (game.in_puzzle) {
  report_error ("The UNDO operation is not available while completing a puzzle.")
}
else if (not GetBoolean (game, "hasacted")) {
  msg ("There is nothing to undo!")
}
else {
  start_count = game.hint_count
  undo
  game.suppressturnscripts = true
  reset_options
  while (game.in_puzzle) {
    undo
    game.suppressturnscripts = true
    reset_options
  }
  if (start_count > game.hint_count) {
    IncreaseScore (-game.hint_charge*(start_count - game.hint_count))
    game.hint_count = start_count
  }
  JS.updateLocation (CapFirst(GetDisplayName(game.pov.parent)))
}

I'll close this thread once I've tested both games.

Again thanks!


...just one final minor point. The 'isundo' flag associated with any use-defined 'undo' command can be set at the user level as a Boolean attribute of the command, set to 'true'.

All my games have now been updated and checked, so I've closed the thread.


If undoing the hint command doesn't restore your points spent, it may also be useful to set its isundo flag. That should mean that 'undo' skips over it, undoing the previous command instead.

And to make things easier to understand for the player, it might be worth printing a message including game.pov.currentcommand, which after an undo will contain the command that was undone.


Edited: Sorry to open this up again but I found a problem when using the revised undo in a game that had been restored after saving. The game seems to go back to the built-in undo when the game is restored? I thought I had a solution yesterday but now realise I was wrong, so I've run out of ideas! This is not a major issue as the new undo is still much superior to the old one but perhaps it can be fixed?


Apologies mrangel, my testing of your original proposal was woefully inadequate! What I hadn't noticed was that restored games after saving were using the underlying undo rather than the revised version used in the game before saving. I decided to look more carefully at the problem and have made the following changes, which can be seen in my updated Cloak of Darkness game at
http://textadventures.co.uk/games/view/cxbbr4mqakkylkr80ypjhg/cloak-of-darkness-another-version.

  1. the 'clean_up' turnscript that runs when the command list is empty now disables itself rather than using 'destroy' :
if (HasObject (game.pov, "currentcommandpattern")) {
  if (not GetBoolean (game.pov.currentcommandpattern, "isundo")) {
    game.hasacted = true
    this.enabled = false
  }
}
  1. New code has been added to the user interface initialisation script to cover first initialisation and initialisation after a restore
game.hasacted = false
clean_up.enabled = true
firsttime {
  game.restored = false
}
otherwise {
  game.restored = true
}
  1. and finally, the undo command now has the basic structure:
game.suppressturnscripts = true
if (not GetBoolean (game, "hasacted")) {
  msg ("There is nothing to undo!")
  if (game.restored) {
    msg ("(<b>Note:</b> When a game is saved, the moves made up to that point cannot be undone in the restored game.)")
  }
}
else {
  undo
  game.suppressturnscripts = true
}

Hopefully I haven't missed anything this time around!


Ah, I didn't think about the case when a saved game is restored.
In that case, it's probably more efficient to have the turnscript simply:

game.hasacted = true

and in the UI initialisation script:

game.restored = GetBoolean (game, "hasacted")
game.hasacted = false

Yes, even better! Thanks mrangel.


This has been a very interesting exercise!
I prefer the idea of disabling the command and having people save scum instead o:-)


This topic is now closed. Topics are closed after 60 days of inactivity.

Support

Forums