[I6] Inform 6.33.1-b3 issues

Whether or not you get the default response of “You eat the apple. Not bad.” depends upon the value returned from the After rule that catches “Take”. Indeed it allows for your vengeful tree example.

I think this is the result of some cleanups I did regarding excess newlines here and there and made things worse in a few places.

This one is definitely a bug. I’ve filed it at inform7.com/mantis/view.php?id=1854

(>TAKE SHOVEL)

I wasn’t confused about what happens so much as conflicted about what should happen. The after() stage of the library’s action processing model interacts with the implicit action aspect of the library in unfortunate ways, I think.

Without implicit actions, after() returning true is pretty straightforward. As with any other stage of action processing, a return value of true is telling the library “no need to continue processing this action. It’s handled.” This could be merely because the after() routine has printed a custom msg reporting the action and thus doesn’t need the library to also report the action, or it could be because something substantive has happened in the game world as a consequence of the action having been completed (such as in the vengeful tree example) and action processing should be stopped so that the player has a chance to respond to it. During the after() stage, “reporting an action” and “continuing action processing” are synonymous, since the library will have done its manipulation of the model world during an earlier stage. So, these two possible interpretations of after()'s return value collapse into one and we don’t care about the distinction.

However, add implicit actions to the mix, and now they are no longer synonymous. In the “eat apple” example, the command generates both an implicit ##Take action and an ##Eat action. If we’re in the apple’s after() routine handling the implicit ##Take, returning true won’t just cause the library not to report the current action (which it presumably wouldn’t have reported anyway since the action is implicit), it will also stop action processing for the subsequent ##Eat action that’s queued up. The library, though, is just stopping action processing because after() returned true. It doesn’t recognize the distinction between the two interpretations of “return true” that now exists because multiple actions are in progress.

We could resolve some of these issues by allowing for better communication between after() routines and the library. In the game->library direction, we could add an extra possible return value from after() so that we have: 0 - continue everything, 1 - stop everything, 2 - stop the current action, but continue with any subsequent actions. In the library->game direction, the library could set an action_is_implicit flag that an after() routine could consult to conditionally print or suppress action reports as needed. This would allow something like:

Another option that doesn’t require library changes is for an author to let LibraryMessages handle custom action reports while after() only handles events that follow on from completed actions. For example, LibraryMessages could test for (noun == apple) and replace the “Taken” msg with “You pick up the apple eagerly”, but following this approach would crud up a game’s LibraryMessages with a bunch of object-specific conditionals.

I think I’ve fixed this. The problem stemmed from improper usage of return value from ZRegion(). In the old inform-2006 repository, there was an effort to get rid of calls to ZRegion(), replacing them with metaclass(). Two calls were left in. One was used properly, but the other treated the return value as it if was from metaclass(). I have changed both calls to ZRegion() to their modern equivalents.

Awesome.

While you’re paying attention to the thread, I noticed another small issue in DropSub in verblibm.h:

    if (noun notin actor && ~~ImplicitTake(noun)) return L__M(##Drop, 2, noun);

Looking at ImplicitTake, it returns true on failure and false on success, so the test above ought to be ImplictTake(noun) rather than ~~ImplicitTake(noun), as it is in all of the other calls in verblibm.h.

I think that this issue has gone undetected in the past because the grammar for Drop uses the multiheld and multiexcept tokens which cause the parser to generate its own ImplicitTake prior to DropSub ever being called. I discovered the issue after having extended the grammar for Drop and having adapted DropSub in one of my own programs to handle a special case (of liquid in a container).

Hmm… Can you come up with a test case?

I investigated this further with the following test case:

Constant Story "DROPSUB IMPLICIT TAKE";
Constant Headline "^An Interactive Investigation^";

Include "Parser";
Include "VerbLib";

Object Start_Room "Start Room"
  with description "This is the starting room.",
  has light;

Object tin "metal tin"
  with name 'metal' 'tin',
  has container openable;

Object -> mint "breath mint"
  with name 'breath' 'mint',
  has edible;

[ Initialise;
  location = Start_Room;
  move tin to player;
];

Include "Grammar";

This has the following behavior:

I had expected drop to have similar behavior as eat, implicitly taking the mint out of the tin.

However, reading ImplicitTake more carefully, I see that the first few lines are:

[ ImplicitTake obj
    res ks supcon;
    switch (metaclass(obj)) { Class, String, Routine, nothing: rfalse; }
    if (obj in actor) rfalse;
    if (action_to_be == ##Drop) rfalse;

It seems that, in the case of ##Drop, ImplicitTake will always return false before changing the model world in any way.

I overlooked this because, in the program that I referred to in my previous post, I had created a new action with its code based on the existing DropSub impl, so action_to_be wasn’t ##Drop. In the case of regular ##Drop, the distinction that I made between multiheld/multiexcept and other tokens is irrelevant since the parser’s invocation of ImplicitTake always matches this action_to_be test and returns false, so DropSub’s ImplicitTake line is called either way.

In light of this, rather than the ~~ImplicitTake() check in DropSub being reversed, it seems that it’s instead superfluous and that the same effect could be achieved with:

if (noun notin actor) return L__M(##Drop, 2, noun);

Evidently, the library explicitly does not want ##Drop ever to do an implicit take, although it’s unclear to me why this should be the case given how ##Eat works. I think that maybe it’s to prevent the parser’s multiheld/multiexcept ImplicitTake, when the user types “drop ”, from producing “(first taking the foo) Dropped.” ImplicitTake could deal with that by extending its acton_to_be check to:

if (action_to_be == ##Drop && obj in parent(actor)) rfalse;

This should allow the “drop ” case to produce “the foo is already here”, while allowing items that are indirectly contained in the actor to be implicitly taken and dropped.

All that being said, I’m not sure to what extent existing games rely on the old behavior or if there’s a mandate to improve I6 library behavior (assuming that we can agree on what constitutes an improvement) vs. just fixing bugs.

This is now fixed. I’ve also fixed the extra newline for PLACES output.

I tracked this one down to a change in parserm.h made in github.com/DavidGriffith/inform … 47b314f0ca (from the old 2006 CVS). In the section prefaced by “! Generate the action”, before the change, the code that now appears in actor_act() appeared before the various tests of i, inp1, and inp2. After the change, this call comes only after tests on these variables succeeds. It’s unclear to me why this change was made, but it clearly is preventing correct behavior.

Simply moving the actor_act() call above the tests works, but not all the way. “Nothing to do!” is still printed afterwards by a call of L__M(##Miscellany, 2); because multiple_object–>0 equals 0. Clearly that printing of ##Miscellany, 2 should not happen if the result from the grammar routine was true. I don’t know how to do this correctly.

I PMed Zarf about this hoping that he might remember what was what, then I decided I’d better open it up for everybody to take a look.

I’ve been looking at some differences between zcode and glulx in the library, and I’ve noticed a small issue.

Banner() in verblibm.h prints out a newline before printing Story for zcode games but not for glulx games:

    if (Story) {
        #Ifdef TARGET_ZCODE;
        #IfV5; style bold; #Endif;
        print "^", (string) Story;
        #IfV5; style roman; #Endif;
        #Ifnot; ! TARGET_GLULX;
        glk($0086, 3); ! set header style
        print (string) Story;
        glk($0086, 0); ! set normal style
        #Endif; ! TARGET_
    }

The result is that a game with introductory text of the form:

[ Initialise;
  location = Start_Room;
  "Some introductory text.";
];

will look good when the game is compiled to zcode, but the intro text will run up against the story name when the game is compiled to glulx.

Fixed.

I noticed that Glulxe will put a newline just before the introductory text whereas most Z-machines don’t. Does anyone know if this newline is generated by the interpreter? I can’t find it in the Library. This effect is not visible with Gargoyle because the text always rises from the bottom instead of descending from the top.

There’s nothing in the spec about it, and the interpreter isn’t generating any extra newline.

ADD: It looks like Zoom formats the text pane with margin space (between the top of the pane and the first line of text). Margins are up to the interpreter.

For the “Orders fully parsed by an animate/talkable object’s grammar routine are not being delivered to its orders routine.” problem (inform7.com/mantis/view.php?id=1813), I’ve committed a partial solution.

Prior to Roger Firth’s movement of code from before “! Generate the action” to InformLibrary.actor_act(), that code would ALWAYS result in a jump to begin__action or to turn__end if (actor ~= player). After the change, this was not always the case. It seems that this change was made to accomodate the new “<action [noun] [second], actor>” optional grammar for invoking R_Process (introduced in Inform compiler 6.33). For the moment, I’m rolling back this chunk to prior to Roger’s change to have an anchor point from which to investigate a more perfect solution.

Thanks goes to Zarf for pointing out to me the precise place and manner of the error in parserm.h that caused the problem.

That makes sense to me. Thanks to Zarf seconded, and thanks to DavidG for investing the time to investigate and fix all of these issues.

I found a new problem that was caused by the fix for L61115. That was “‘multiheld’ can match unholdable objects”. Remember that one? See github.com/DavidGriffith/inform6lib/issues/30. At first I thought it was part of the hangup between grammar and orders that hinges on Roger’s actor_act() code.

Hmm, it looks like the fix for L61115 is also responsible for the “implicit take doesn’t happen for ##Drop when the player encloses but doesn’t directly carry an object” inconsistency that I was going on about earlier in the thread.

The actor_act() hangup and the problem with an NPC taking or dropping multiple objects have been fixed. I’m still working on optimizing the solution to the former. The latter turned out to not really be connected to L61115. I’ll go ahead and write up the implicit take problem.

github.com/DavidGriffith/inform6lib/issues/31

inform7.com/mantis/view.php?id=1886

Investigating the problem with “implicit take doesn’t happen for ##Drop when the player encloses but doesn’t directly carry an object”. It looks like neither 6/11 nor 6/10 would perform the implicit take. This suggests to me that there was some sort of disagreement in 2006 over which is the proper way to do it. Look at this section of code in verblibm.h:

[ ImplicitTake obj
    res ks supcon;
    switch (metaclass(obj)) { Class, String, Routine, nothing: rfalse; }
    if (obj in actor) rfalse;
    if (action_to_be == ##Drop) rfalse;
    ...

If we comment out this line

if (action_to_be == ##Drop) rfalse;

Then we get this:

>open tin
You open the metal tin, revealing a breath mint.

>drop mint
(first taking the breath mint out of the metal tin)
Dropped.

Is this what you were expecting to happen?

Yes, I was expecting DROP MINT to behave the same way as EAT MINT, either with both producing an implicit take or neither.

I think that we might need something like:

if (action_to_be == ##Drop && ~~IndirectlyContains(player, obj)) rfalse; rather than removing the line completely, to prevent results like:

when the player isn’t carrying the mint.