Help wanted writing I6 library tests

I’ve recently been investigating inform6lib issue 38 (analysis and patch forthcoming). The issue involves library messages, and it motivated me to read through the library and look at all calls to L__M for similar errors.

This post describes four issues that I found during this sweep and proposes fixes for them. (Pull request sent on github with these patches.)

Voices and tenses for disambiguation questions

The parser asks a disambiguation question to the player when the object of a command is ambiguous. These questions differ slightly depending on whether the command’s grammar requires a creature token for the object or not. Reading through the messages in english.h, I noticed that two such messages (Miscellany 48 and 49) are implemented inconsistently. Message 49 makes use of the new voices and tenses feature in lib 6/12 while message 48 doesn’t.

Here’s a test program demonstrating the issue:

Constant Story "VOICE, TENSE, AND DISAMBIGUATION";
Constant Headline "^An Interactive Regression Test^";
Constant DEBUG;

! Voice and tense incorporated into disambiguation message for non-creature
! tokens but not for creature tokens.

! Test me with "take / rock / kiss / chris / drop rock / bob, take / coin /
!     bob, kiss".

Include "parser.h";
Include "verblib.h";

Object Start_Room "Somewhere"
  with description "I was not sure where I was.",
  has  light;

Object -> Bob "Bob"
  with name 'bob',
  has  animate male proper;

Object -> Chris "Chris"
  with name 'chris',
  has  animate male proper;

Object -> rock "rock"
  with name 'rock';

Object -> coin "coin"
  with name 'coin';

[ Initialise;
  location = Start_Room;
  player.narrative_voice = 1;
  player.narrative_tense = PAST_TENSE;
];

Include "grammar.h";

Here’s the code in NounDomain responsible for generating these disambiguation prompts:

    ! Now we come to the question asked when the input has run out
    ! and can't easily be guessed (eg, the player typed "take" and there
    ! were plenty of things which might have been meant).

  .Incomplete;

    ...
    if (context == CREATURE_TOKEN) L__M(##Miscellany, 48);
    else                           L__M(##Miscellany, 49, actor);

And here’s the code in LanguageLM that constructs these messages:

  Miscellany: switch (n) {
        ...
        48: print "Whom do you want";
            if (x1 ~= player && x1 ~= nothing) print " ", (the) x1;
            print " to "; PrintCommand(); "?";
        49: print "What ";
            CSubjectVoice(player, "do", "do", "does", "did");
            print " ";
            CSubjectVerb(player, false, true, "want", "want", "want", "want");
            if (x1 ~= player && x1 ~= nothing) print " ", (the) x1;
            print " to "; PrintCommand(); "?";
        ...
    }

We can see two issues here: (1) the call to L__M(##Miscellany, 48) doesn’t pass actor, even though message 48 expects it to be passed as x1. This causes questions about third party actions (“Whom do you want Bob to kiss?”) to instead be phrased as questions about player actions (“Whom do you want to kiss?”), because x1 is nothing, and (2) message 48 does not use CSubjectVoice and CSubjectVerb to apply voice and tense, so the message is always in the 2nd person present, even if voice and tense are configured differently.

The solution to the first issue is to add actor to the L__M(##Miscellany, 48) call in NounDomain:

    if (context == CREATURE_TOKEN) L__M(##Miscellany, 48, actor);
    else                           L__M(##Miscellany, 49, actor);

The solution to the second issue is to rewrite message 48 to make use of CSubjectVoice and CSubjectVerb in the same way that message 49 does:

  Miscellany: switch (n) {
        ...
        48: print "Whom ";
            CSubjectVoice(player, "do", "do", "does", "did");
            print " ";
            CSubjectVerb(player, false, true, "want", "want", "want", "want");
            if (x1 ~= player && x1 ~= nothing) print " ", (the) x1;
            print " to "; PrintCommand(); "?";
        ...
    }

With the fix applied, we have this output:

WriteAfterEntry

Reading verblibm.h, I noticed an issue with WriteAfterEntry in which a container containing a concealed or scenery object can be flagged as empty even if it contains a visible object (depending on the order of the container’s children).

Here’s a test program demonstrating the issue:

Constant Story "~EMPTY~ CONTAINER";
Constant Headline "^An Interactive Regression Test^";

Constant DEBUG;

! WriteAfterEntry:
! Concealed or scenery objects inside a container can cause it to be described
! as empty even if there are also visible objects inside.

! Test me with "put rock in bucket / l".

Include "Parser";
Include "VerbLib";

Object TestChamber "Test Chamber"
  with description "This is the Test Chamber.",
  has  light;

Object -> bucket "bucket"
  with name 'bucket',
  has  container open;

Object -> -> bump "bump"
  with name 'bump',
  has  scenery;

Object -> rock "rock"
  with name 'rock';

[ Initialise;
  location = TestChamber; 
];

Include "Grammar";

Here’s the output:

"EMPTY" CONTAINER
An Interactive Regression Test
Release 1 / Serial number 160428 / Inform v6.33 Library v6.12.1 SD

Test Chamber
This is the Test Chamber.

You can see a bucket (which is empty) and a rock here.

>put rock in bucket
(first taking the rock)
You put the rock into the bucket.

>l

Test Chamber
This is the Test Chamber.

You can see a bucket (which is empty) (in which is a rock) here.

>tree
...
Test Chamber (25)
  yourself (20)
  a bucket (26)
    a rock (28)
    a bump (27)

WriteAfterEntry uses 3 bits to represent objects as being combinations of empty (4), closed (2), and providing light (1). It stores these flags in the variable combo and calls L__M(##ListMiscellany, combo) to print an appropriate message about an object. ListMiscellany msgs 1-7 are are used to describe objects with various combos of these attributes.

Here’s the code that WriteAfterEntry uses to determine whether a container is empty and thus whether the empty bit should be set in combo.

        if ((o has container && (o has open || o has transparent))) {
            objectloop(i in o) {
                if (i has concealed or scenery) j = false;
                if (i hasnt concealed && i hasnt scenery) j = true;
            }
            if (~~j) combo=combo+4;
        }

In 6/11, a container was considered empty if it had no children. This gave away information to the player. If a concealed object was hidden in an otherwise empty container, the container would not be described as empty in object listings. In 6/12, the code above was added to ignore concealed or scenery objects when determining a container’s emptiness.

This code iterates through the objects in a container setting j to true if a visible object (not concealed and not scenery) is found and setting it to false otherwise. After the loop, if j remains false, the container is considered to be empty.

The intent appears to be to set j to true if any visible object is in the container, but every time the loop encounters a concealed or scenery object, j is cleared again, so the end result depends on the order in which the container’s contents are encountered. This is what leads to the results that we see with the test program above, where the bucket contains a rock and a scenery object. The rock is encountered first and causes j to be set to true, but the scenery object causes j to be set back to false, leaving the bucket considered empty despite the rock being inside.

The solution is to not clear j once it’s been set. Any visible object should cause the container to be considered nonempty. Having encountered such an object, there’s no need to continue iterating over the container’s contents. Here’s my proposed fix for this issue:

        if ((o has container && (o has open || o has transparent))) {
            objectloop(i in o) {
                if (i hasnt concealed && i hasnt scenery) {
                    j = true; break;
                }
            }
            if (~~j) combo=combo+4;
        }

With this fix in place, we get this output:

ImplicitDisrobe issue

[ ImplicitDisrobe obj
    res ks;
    if (obj hasnt worn) rfalse;
    res = CheckImplicitAction(##Disrobe, obj);
    ! 0 = Take off object, Tell the user (normal default)
    ! 1 = Take off object, don't Tell
    ! 2 = don't Take object  continue       (default with no_implicit_actions)
    ! 3 = don't Take object, don't continue
    if (res >= 2) rtrue;
    ks = keep_silent; keep_silent = 1; <Disrobe obj, actor>; keep_silent = ks;
    if (obj has worn && obj in actor) rtrue;
    if (res == 0 && ~~keep_silent) L__M(##Drop, 3, noun);
    rfalse;
];

ImplicitDisrobe is passed a single argument, obj, that is the item to be disrobed. While the rest of the routine operates on obj, the call to L__M(##Drop, 3) is passed noun as an argument. The existing library code always calls ImplicitDisrobe on noun, so there is no practical difference, but it’s still inconsistent to pass noun to L__M instead of obj and could lead to bugs in the future.

Proposed fix:

    if (res == 0 && ~~keep_silent) L__M(##Drop, 3, obj);

RemoveSub issue

When the player attempts to remove an object from a closed container that can’t be implicitly opened, an incorrect response is printed.

Here’s a test program demonstrating the issue:

Constant Story "REMOVE OBJECT FROM UNOPENABLE CONTAINER";
Constant Headline "^An Interactive Regression Test^";
Constant DEBUG;

! Test me with "remove emerald from box".

Include "parser.h";
Include "verblib.h";

Object Start_Room "Somewhere"
  with description "You're not sure where you are.",
  has  light;

Object -> glass_box "glass box"
  with name 'glass' 'box',
  has  container openable locked transparent;

Object -> -> emerald "emerald"
  with name 'emerald' 'gem';

[ Initialise;
  location = Start_Room;
];

Include "grammar.h";

This is the library code responsible:

[ RemoveSub i;
    i = parent(noun);
    if (i && i has container && i hasnt open && ImplicitOpen(i)) return L__M(##Remove, 1, noun);
    ...
];
  Remove: switch (n) {
        1:  CSubjectIs  (x1,true); " unfortunately closed.";
        ...
    }

If noun, the object being removed, is in a closed container (called i), and ImplicitOpen(i) fails, then we call L__M(##Remove, 1) to print a message that i is closed. However, RemoveSub passes noun rather than i to L__M, producing a message about the object being removed being closed instead of its container being closed.

The solution is to pass i (the container) to L__M instead of noun (the object contained):

[ RemoveSub i;
    i = parent(noun);
    if (i && i has container && i hasnt open && ImplicitOpen(i)) return L__M(##Remove, 1, i);
    ...
];

With the fix in place, we get this output:

Too many arguments passed to L__M

The majority of the action routines in the library pass extra unused args to L__M for at least one message.

Here’s a list of the actions that have messages with extra args: Answer, Ask, Attack, Burn, Buy, Climb, Cut, Dig, Drink, Drop, EmptyT, Enter, Examine, Insert, Jump, JumpOver, Kiss, Listen, ListMiscellany, Look, LookUnder, Mild, Miscellany, Objects, Pray, Pull, Push, PushDir, Remove, Rub, Search, Sing, Sleep, Smell, Sorry, Squeeze, Strong, Swim, Swing, Take, Taste, Tell, Think, ThrowAt, Tie, Touch, Turn, Wait, Wake, WakeOther, WaveSub, WaveHands.

The most common form of this is an extra noun arg passed to L__M that is mapped to parameter x1 and unused.

Earlier in the 6/12 beta process, a post from auraes informed us that sometimes extra information is required for non-English language messages compared with English ones. This requires passing args to L__M that appear to be unused when looking only at english.h.

I think that the library has gone a bit overboard with The Great Nounification of L__Ms, but auraes’s issue is one to keep in mind if we ever start pruning these seemingly superfluous calls back.