Skip to content
This repository has been archived by the owner on Jul 9, 2018. It is now read-only.

Orders to NPC to take and drop multiple objects not working #30

Closed
DavidGriffith opened this issue Mar 12, 2016 · 7 comments
Closed

Orders to NPC to take and drop multiple objects not working #30

DavidGriffith opened this issue Mar 12, 2016 · 7 comments
Assignees
Labels
Milestone

Comments

@DavidGriffith
Copy link
Owner

Somehow the noun global variable gets set to "nothing" when an NPC is ordered to do something with multiple items.

Constant Story "GRAMMAR AND ORDERS";
Constant Headline "^An Interactive Bug Reproduction^";
Constant DEBUG;

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

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

Class Rock
    with name "rock" "rocks//p",
    short_name "rock",
    plural "rocks";

Rock ->;
Rock ->;
Rock ->;
Rock ->;

Object -> stone "stone"
    with name "stone";

Object -> girl "girl"
  with name 'girl',
       orders [;
           WaveHands: "The girl waves energetically.";
       Take:    print (name) noun, "^";
            <<Take noun, actor>>;
       Drop:    print (name) noun, "^";
            <<Drop noun, actor>>;
       ],
       grammar [;
           if (verb_word == 'jump') {
               action = ##WaveHands;
               noun = 0;
               second = 0;
               rtrue;
           }    

       if (verb_word == 'blarp') {
        action = ##Take;
        rtrue;
       }

    ! Didn't understand any of this, bailing out.
       ],
  has animate female transparent;

[ Initialise;
  location = Start_Room;
  "It is time to do some bugfixing...";
];
Include "grammar.h";

Expected response:

>girl, drop rocks
rock: rock
Dropped.
rock: rock
Dropped.
rock: rock
Dropped.
rock: rock
Dropped.

>

Actual response:

>girl, drop rocks
nothing

[** Programming error: tried to test "in" or "notin" of nothing **]

[** Programming error: tried to test "in" or "notin" of nothing **]
Perhaps she should take nothing
[** Programming error: tried to find the "parent" of nothing **]

[** Programming error: tried to test "has" or "hasnt" of nothing **]
 out of
[** Programming error: tried to find the "parent" of nothing **]
nothing first.

>
@DavidGriffith
Copy link
Owner Author

Problem first appears at 5675c83 (Fix Issue L61115 (Mantis 977))

Issue L61115 was "'multiheld' can match unholdable objects"

@DavidGriffith
Copy link
Owner Author

@DavidGriffith DavidGriffith self-assigned this Mar 16, 2016
DavidGriffith added a commit that referenced this issue Mar 16, 2016
…r_act().

Still code duplication in actor_act() to get rid of.
DavidGriffith added a commit that referenced this issue Mar 19, 2016
The first time we call self.actor_act(), some things must be done
slightly differently in order to avoid problems when doing something to
several items at once.  For instance: TAKE ROCKS. See this function in
1d95759 to see how there used to be
essentially two functions.  The need for this distiction came about with
Issue #30.  So, if this is the first time calling actor_act(), pass true
on the fifth parameter (ft).
DavidGriffith added a commit that referenced this issue Apr 17, 2016
Evidently my attempt to consolidate Roger's code back into actor_act()
for #30 wasn't entirely successful.
@DavidGriffith
Copy link
Owner Author

broken again

@DavidGriffith DavidGriffith reopened this Apr 18, 2016
@DavidGriffith DavidGriffith changed the title Orders to NPC to drop multiple objects not working Orders to NPC to take and drop multiple objects not working Apr 18, 2016
DavidGriffith added a commit that referenced this issue Apr 18, 2016
@DavidGriffith
Copy link
Owner Author

DavidGriffith commented Apr 18, 2016

This latest attempt fixes #34, but results in this:

>girl, wave
The girl waves energetically.
! correct

>girl, jump
Nothing to do!
!wrong

>girl, drop rock
rock
The girl observes that the rock is already here.
!correct

>girl, drop rocks
There is no reply.
!wrong

>girl, take rocks
rock: rock
Taken.
rock: rock
Taken.
rock: rock
Taken.
rock: rock
Taken.
!correct

>girl, jump
rock: The girl waves energetically.
rock: The girl waves energetically.
rock: The girl waves energetically.
rock: The girl waves energetically.
!wrong

>g
rock: The girl waves energetically.
rock: The girl waves energetically.
rock: The girl waves energetically.
rock: The girl waves energetically.
!wrong

@DavidGriffith
Copy link
Owner Author

Here's a new test program, this time including Dyslexic Dan from DM4 Example 32 so I can check that part of the bug. For some reason, I decided to check the Glulx version. Curiously, Z-machine and Glulx perform differently. Doing GIRL, JUMP will at all times work correctly under Glulx. Doing GIRL, TAKE ROCKS will not.

Constant Story "GRAMMAR AND ORDERS";
Constant Headline "^An Interactive Bug Reproduction^";
Constant DEBUG;

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

!statusline time;

Class Rock
    with name "rock" "rocks//p",
    short_name "rock",
    plural "rocks",
    has supporter;

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


Rock ->;
Rock ->;
Rock ->;
Rock ->;

Object -> stone "stone"
    with name "stone",
    has supporter;

Object -> box "box"
    with name "box",
    has container open enterable;

Object -> "cow pie"
  with name 'cow' 'pie',
       description "That's one healthy cow.",
       before [;
           Examine: ;
           default: "Nope.";
       ];

Object -> girl "girl"
  with name 'girl',
       orders [;
           WaveHands: "The girl waves energetically.";
       Take:!   print (name) noun, "^";
            <<Take noun, actor>>;
       Drop:!   print (name) noun, "^";
            <<Drop noun second, actor>>;
       PutOn:   <<PutOn noun second, actor>>;
       ],
       grammar [;
           if (verb_word == 'jump') {
               action = ##WaveHands;
!               noun = Stone;
!               second = 0;
               rtrue;
           }    

    ! Didn't understand any of this, bailing out.
       ],
  has animate female transparent;

Object -> Dan "Dyslexic Dan"
  with name 'dan' 'dyslexic',
       grammar [;
           if (verb_word == 'examine' or 'x//') {
               verb_wordnum++; return -'danx';
           }
       ],
       orders [;
           Examine: "~What,~ says Dan, ~you want me to examine ",
                        (the) noun, "?~";
           Inv: "~That I can do,~ says Dan. ~I'm empty-handed.~";
           default: "~Don't know how,~ says Dan.";
       ],
       initial "Dyslexic Dan is here.",
  has  animate proper;


[ Initialise;
  location = Start_Room;
  "It is time to do some bugfixing...";
];

Include "grammar.h";

Verb 'danx' * 'foo' -> Inv;

With Glulxe:

>girl, wave
inp1:          nothing
inp2:          nothing
inputobjs-->1: nothing
inputobjs-->2: nothing
inputobjs-->3: nothing
noun:          nothing
second:        nothing
actor:         girl
---
FOO
The girl waves energetically.

>girl, jump
inp1:          nothing
inp2:          nothing
inputobjs-->1: <illegal object number 2>
inputobjs-->2: nothing
inputobjs-->3: nothing
noun:          nothing
second:        nothing
actor:         girl
---
FOO
The girl waves energetically.

>girl, drop rock
inp1:          rock
inp2:          nothing
inputobjs-->1: <illegal object number 1>
inputobjs-->2: rock
inputobjs-->3: nothing
noun:          rock
second:        nothing
actor:         girl
---
FOO
The girl observes that the rock is already here.

>>girl, drop rocks
inp1:          <illegal object number 1>
inp2:          girl
inputobjs-->1: <illegal object number 2>
inputobjs-->2: <illegal object number 1>
inputobjs-->3: girl
noun:          <illegal object number 145842>
second:        girl
actor:         girl
---
FOO
BAR
inp1:          <illegal object number 1>
inp2:          girl
inputobjs-->1: <illegal object number 2>
inputobjs-->2: <illegal object number 1>
inputobjs-->3: girl
noun:          <illegal object number 145842>
second:        girl
actor:         yourself
---
BAZ
ZOOM
There is no reply.

>girl, take rocks
inp1:          nothing
inp2:          nothing
inputobjs-->1: <illegal object number 1>
inputobjs-->2: nothing
inputobjs-->3: nothing
noun:          nothing
second:        nothing
actor:         girl
---
FOO

[** Programming error: tried to test "has" or "hasnt" of nothing **]

[** Programming error: tried to test "in" or "notin" of nothing **]

[** Programming error: tried to find the "parent" of nothing **]

[** Programming error: tried to find the "parent" of nothing **]

[** Programming error: tried to test "has" or "hasnt" of nothing **]

[** Programming error: tried to test "has" or "hasnt" of nothing **]

[** Programming error: tried to "move" nothing to girl **]

[** Programming error: tried to "give" an attribute to nothing **]
Taken.

>girl, jump
inp1:          nothing
inp2:          nothing
inputobjs-->1: <illegal object number 2>
inputobjs-->2: nothing
inputobjs-->3: nothing
noun:          nothing
second:        nothing
actor:         girl
---
FOO
The girl waves energetically.

>g      
inp1:          nothing
inp2:          nothing
inputobjs-->1: <illegal object number 2>
inputobjs-->2: nothing
inputobjs-->3: nothing
noun:          nothing
second:        nothing
actor:         girl
---
FOO
The girl waves energetically.

>

@DavidGriffith
Copy link
Owner Author

DavidGriffith commented Apr 21, 2016

Commentary from Vince at http://www.intfiction.org/forum/viewtopic.php?f=7&t=19868&start=40#p108515

I've been looking into this issue and making some progress. Full update soon.

For now, I want to mention that the girl is a red herring. The issue has nothing to do with conversation or orders. We can see the same thing when the player is the actor

>drop rock
The rock is already here.

>drop rocks
There is nothing to drop.

>take rocks
rock: Taken.
rock: Taken.
rock: Taken.
rock: Taken.

>take rock
You already have that.

>take rocks
There is nothing to take.

The DROP ROCK and TAKE ROCK commands make it past the parser and the responses that we see come from action handling.

The DROP ROCKS and (the second) TAKE ROCKS commands generate parser errors and never make it to the action handling stage.

@DavidGriffith
Copy link
Owner Author

After going over the DM4, Vince has a solution to the DROP ROCKS problem:
Copied from http://www.intfiction.org/forum/viewtopic.php?f=7&t=19868&start=40#p108572

I've finished looking into the DROP ROCKS issue.

TL;DR: It's not a bug, it's a feature.

Read on for the gory details, some actual bugs, and some thoughts on inform6lib development.

What's the issue?

When we run the test program for issue #30 (mantis http://inform7.com/mantis/view.php?id=1885) , we see:

Somewhere
You're not sure where you are.

You can see four rocks, a stone and a girl here.

>girl, drop rock
The girl observes that the rock is already here.

>girl, drop rocks
There is no reply.

>drop rock
The rock is already here.

>drop rocks
There is nothing to drop.

When all of the rocks are on the ground and we try to drop a single rock, parsing succeeds and we receive a response from the Drop action telling us that the rock is already here.

When we try to drop multiple rocks, parsing fails and no action is generated. Instead, we receive an error message from the parser.

When we order the girl to drop the rocks, this error is transformed into a NotUnderstood action, sent to the girl's orders, and redirected to her life, where it ultimately produces the suboptimal default response that we see.

In this post, we'll focus on the simpler DROP ROCKS command with the player as the actor.

What's causing it?

At parser trace level 1, we can see the big picture:

>drop rocks
[Parsing for the verb 'drop' (3 lines)]
[line 0 * multiheld -> Drop]
[line 1 * multiexcept 'in' / 'into' / 'down' noun -> Insert]
[line 2 * multiexcept 'on' / 'onto' noun -> PutOn]
There is nothing to drop.

There are 3 grammar lines for DROP, and "drop rocks" fails to match any of them. Line 0 (* multiheld -> Drop) is one that we'd expect to match our input and the one that we'll focus on here.

The grammar requires the word 'drop' followed by text matching a multiheld token. The DM4 describes multiheld as "one or more held objects", but, as we can see from our success parsing DROP ROCK, this isn't a hard and fast rule; the reality is a little more nuanced.

If we crank up the trace level to 5, we can see more detail as the parser tries to parse "rocks" as a multiheld token:

>drop rocks
...
[line 0 * multiheld -> Drop]
 [line 0 token 1 word 2 : multiheld]
  [Object list from word 2]
  [Calling NounDomain on location and actor]
   [NounDomain called at word 2]
   seeking definite object
...
   [NounDomain made 4 matches]
   [Adjudicating match list of size 4 in context 3]
   indefinite type: plural
   number wanted: all
   most likely GNAs of names: 4095
   Scoring match list: indef mode 1 type 8, satisfying 0 requirements:
     The rock (27) in the Somewhere : 156 points
     The rock (28) in the Somewhere : 156 points
     The rock (29) in the Somewhere : 156 points
     The rock (30) in the Somewhere : 156 points
   Best guess the rock (27)   
   Rejecting it
   Best guess the rock (28)
   Rejecting it
   Best guess the rock (29)
   Rejecting it
   Best guess the rock (30)
   Rejecting it
   Best guess ran out of choices
   Made multiple object of size 0]
...

This trace is mostly produced by NounDomain and Adjudicate in parserm.h. The call chain that got us there is:
InformLibrary::play -> InformParser::parse_input -> Parser__parse (Part G) -> ParseToken__ (Part D) -> NounDomain -> Adjudicate

The parser calls ParseToken__ to match a multiheld token (with input "rocks"), which calls NounDomain to find the best matching object or objects in the actor's scope.

Notice that, when NounDomain is first called, we're "seeking definite object", but, by the beginning of Adjudicate, we're looking for all of an indefinite group of objects ("indefinite_type: plural, number wanted: all"). This happens because, when NounDomain calls SearchScope to find matching objects in the actor's scope, TryGivenObject finds that a rock matches the plural dictionary word 'rocks//p'. This causes the parser to switch into indefinite plural mode. We look for "all" (represented by 100) because no specific number has been requested (we didn't type DROP TWO ROCKS):

            if (dict_flags_of_noun & DICT_PLUR) {
                if (~~allow_plurals) k = 0;
                else {
                    if (indef_mode == 0) {
                        indef_mode = 1; indef_type = 0; indef_wanted = 0;
                    }
                    indef_type = indef_type | PLURAL_BIT;
                    if (indef_wanted == 0) indef_wanted = 100;
                }
            }

In contrast, DROP ROCK does not trigger this code and remains in definite mode, accounting for the difference in results that we'll see.

After NounDomain finds multiple objects that match "rocks" ("NounDomain made 4 matches"), it calls Adjudicate to score them and disambiguate among them. This process depends on the actor, action_to_be, the token that we're trying to match, whether we're in definite or indefinite mode, and the state of the world model at the time. It's described in some detail in §33 of the DM4 on pages 240-242, about which more below.

During disambiguation, we reach a block of code specific to indefinite plural mode, and (spoiler) it rejects all four of our rocks! (For I7 people: this code is analogous to the deciding whether all includes activity.):

    if (indef_mode == 1 && indef_type & PLURAL_BIT ~= 0) {
        if (context ~= MULTI_TOKEN or MULTIHELD_TOKEN or MULTIEXCEPT_TOKEN
                     or MULTIINSIDE_TOKEN) {
            etype = MULTI_PE;
            return -1;
        }
        i = 0; offset = multiple_object-->0; sovert = -1;
        for (j=BestGuess() : j~=-1 && i<indef_wanted && i+offset<63 : j=BestGuess()) 
{
            flag = 1;
            if (j has concealed && j has worn) flag = 0;
...
            if (context == MULTIHELD_TOKEN or MULTIEXCEPT_TOKEN && parent(j) ~= actor)
                flag = 0;
...
            n = ChooseObjects(j, flag);
            if (n == 0) n = LibraryExtensions.RunWhile(ext_chooseobjects, 0, j, flag);
            switch (n) {
              2: flag = 0;  ! forcing rejection
              1: flag = 1;  ! forcing acceptance
             !0:            ! going with parser's decision
            }
            if (flag == 1) {
                i++; multiple_object-->(i+offset) = j;
                #Ifdef DEBUG;
                if (parser_trace >= 4) print "   Accepting it^";
                #Endif; ! DEBUG
            }
            else {
                i = i;
                #Ifdef DEBUG;
                if (parser_trace >= 4) print "   Rejecting it^";
                #Endif; ! DEBUG
            }
        }
 ...
        return 1;
    }

The for loop iterates over all candidate objects and accepts or rejects each one in turn based on a series of tests. For DROP ROCKS, all of the rocks are rejected, resulting in a multiple object of size 0 which percolates up the parser and triggers a parse error.

The specific test that rejects the rocks is " if (context == MULTIHELD_TOKEN or MULTIEXCEPT_TOKEN && parent(j) ~= actor)".

So, that's what's happening.

In indefinite plural mode, for a multiheld token, an object is rejected unless the actor is holding it, even when there are no other candidates.

In definite mode, an object might be disfavored for multiheld because it's not held, but it's not rejected outright when there are no other candidates.

This code raises a couple issues that I'll discuss later in this post.

  • Why is MULTIEXCEPT also included in this test when it ostensibly has nothing to do with held objects but simply excludes the indirect object?
  • This indef plural code differs from the 6/11 version, with the initial value of flag reversed. The 6/12 version of the test for concealed and worn is incorrect.

Notice also that an author can provide a ChooseObjects function that can override the parser's decision (stored in flag). See DM4 §33 page 239.

Is this behavior intended?

Yes.

It's mentioned in the DM4's explanation of disambiguation:

(From DM4 Section 33 page 241 rule 7ip (indef plural)

(7ip) The following rule applies only in indefinite mode and provided the player has
typed something definitely implying a plural, such as the words all'' orthree''
or coins''. Here the parser already has a target number of objects to choose: for instance 3 forthree'', or the special value of 100, meaning an unlimited number'', forall'' or coins''. Go through the list of objects inbest guess'' order (see below). Mark each as
accept'' unless: (i) it has worn or concealed; (ii) or the action is Take or Remove and the object is held by the actor; (iii) or the token is multiheld or multiexcept and the object isn't held by the actor; (iv) or the target number isunlimited'' and S=20 (rounded down to the nearest
integer) has fallen below its maximum value, where S is the score of the
object.
The entry point ChooseObjects(object,accept_flag) is now called and can
overrule the accept''/reject'' decision either way. We keep accepting objects like
this until the target is reached, or proves impossible to reach.

Section (iii) makes it clear that this is intended behavior.

For additional evidence, in issue 0001488, filed against I7 about a multiexcept issue, but stemming from the exact same line of code, GN says:

(quote from Graham Nelson:)

I've looked into this, and it's by design - the design of the old I6 parser. What happens is not that "2 mints in bowl" isn't recognised: in fact it is recognised, but the standard rules about what can go into a multiple object apply, and they reject the mints because they're not being held by the current actor. This can be changed by hacking with the "does the player mean" rules.

(The I7 does the player mean rules are roughly analogous to ChooseObjects.)

Ok. It's intended behavior. Why?

So that this happens:

>take two rocks
rock: Taken.
rock: Taken.

>drop rocks
rock: Dropped.
rock: Dropped.

instead of this:

>take two rocks
rock: Taken.
rock: Taken.

>drop rocks
rock: Dropped.
rock: Dropped.
rock: The rock is already here.
rock: The rock is already here.

(Similarly for DROP ALL.)

The idea behind the indef plural disambiguation is that we want to decide what should reasonably be included in "all" or a group of objects based on the context of the actor, the action, and the world.

The particular rule that caused our issue says that, when an actor is performing an action that prefers held objects, those are the objects that the parser should choose preferentially. It only breaks down when there are no candidate objects that meet the criteria, as in DROP ROCKS when all rocks are on the ground.

So what can we do?

If we want the best of both worlds, we can write a ChooseObjects routine for our test program that will allow the parser to reject rocks on the ground when there are better candidates, while forcing it to accept rocks on the ground when there are none. (This also handles Take which suffers from a similar issue.)

[ ChooseObjects obj code x;
  if (code == 2) rfalse;
  if (action_to_be == ##Drop && obj ofclass Rock && obj notin actor) {
      objectloop (x in actor) {
          if (x ofclass Rock) rfalse;
      }
      rtrue;
  }
  if (action_to_be == ##Take && obj ofclass Rock && obj in actor) {
      objectloop(x ofclass Rock) {
          if (TestScope(x, actor) && x notin actor) rfalse;
      }
      rtrue;
  }
  rfalse;
];

With this ChooseObjects in place, we get these results:

Somewhere
You're not sure where you are.

You can see four rocks, a stone and a girl here.

>take two rocks
rock: Taken.
rock: Taken.

>drop rocks
rock: Dropped.
rock: Dropped.

>drop rocks
rock: The rock is already here.
rock: The rock is already here.
rock: The rock is already here.
rock: The rock is already here.

>take two rocks
rock: Taken.
rock: Taken.

>take rocks
rock: Taken.
rock: Taken.

>take rocks
rock: You already have that.
rock: You already have that.
rock: You already have that.
rock: You already have that.

Can we do more?

We could change the parser to relax this rejection filter when there is an undifferentiated set of candidates while still using it to disambiguate among candidates with differing characteristics.

However, it would take a bit of restructuring since the indef plural loop rejects candidates one at a time in decreasing BestGuess order. It's only later in Adjudicate that objects are grouped into equivalence classes.

Also, it's not clear that everyone would agree that the behavior in the transcript above is desirable. To quote the DM4's introduction to ChooseObjects:

(from DM4 Section 33 page 239)

The Inform parser resolves ambiguous object names with a pragmatic algorithm
which has evolved over the years (see below). Experience also shows that no two
people ever quite agree on what the parser should ``naturally'' do. Designers have an
opportunity to influence this by providing an entry point routine called ChooseObjects:

My conclusion: use ChooseObjects.

Here ends the discussion of DROP ROCKS. Next up are a couple of side issues, and then some thoughts about I6 library development moving forward.

Back to David now:

The side issues will be written up separately and the I6 library development concerns will be discussed at intfiction.org until someone decides to file an issue report about it if such a thing is necessary.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

1 participant