[I6] Inform 6.33.1-b3 issues

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.

I’ll go for that. Now this brings up another potential irritation. Consider this:

That doesn’t sound right. It should respond with “The mint is already here.”.

This is because of the following in DropSub:

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

where msg 1 is “already here” and msg 2 is “take it out of the foo first”.

But this is not a new problem triggered by the proposed change; it’s an existing issue with the test “noun in parent(actor)”, which is too weak for its intended purpose.

Consider the following modified version of our test program:

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 -> table "big table"
  with name 'big' 'table',
  has enterable supporter;

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";

When we run this with the unmodified library, we see:

So, I think that the issue that you pointed out needs to be fixed regardless. I’d suggest treating it as a separate issue: dropping an object that is not carried by the actor and not directly contained in the parent of the actor produces an inappropriate response. I think that the correct response is arguable depending on how we define “here”. Does it mean the immediate parent of the actor, or does it mean in the broader location / in scope?

(In my previous post, s/player/actor in ~~IndirectlyContains(player, obj))

More thoughts on this:

Both of these issues are intertwined.

As far as I can tell, there are 3 things that we need to consider:

  1. The ##Drop grammar’s multiheld token and the ImplicitTake call that this causes the parser to generate
  2. DropSub and its own call to ImplicitTake
  3. The body of ImplicitTake

Note that the special case “if (action_to_be == ##Drop)” test in ImplicitTake returns false, while the parser’s call to ImplicitTake treats a true return value as failure.

I think that this effectively makes the parser’s ImplicitTake a noop for ##Drop and causes us to move on to DropSub, where ##Drop processing (including the ImplicitTake) really happens. We need to keep this in mind when modifying (or removing) that test in ImplicitTake, lest we restore the significance of the parser’s ImplicitTake without realizing it.

Not that I’m saying that the current situation is a good thing. It would be nicer if ##Drop were handled more consistently with other actions that require held objects.

As I think over this latest issue, I think after it is fixed, it’ll be time for a point release of the 6/12 Library. I haven’t quite decided how to display the point release. I’m thinking having the Library itself call itself 6/12.1 and the package identify itself as 6.12.1.

I was browsing through the I7 manual the other day, and I noticed that the “can’t drop something that’s not directly carried” limitation is mentioned as part of example 224 (Celadon). It doesn’t directly bear on the solution for I6, but it shows us that (1) the current behavior is known, not purely accidental, and (2) the potential desire for different behavior is also understood.