DavidGriffith / inform6lib

The Inform6 interactive fiction standard library (moved to https://gitlab.com/DavidGriffith/inform6lib)
Other
22 stars 9 forks source link

Responses in DropSub need work #32

Closed DavidGriffith closed 8 years ago

DavidGriffith commented 8 years ago

Starting at http://www.intfiction.org/forum/viewtopic.php?f=7&t=19461&start=30#p107693, vlaviano points out how some responses from DropSub don't make sense. "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.".

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:

DROPSUB IMPLICIT TAKE
An Interactive Investigation
Release 1 / Serial number 160320 / Inform v6.33 Library 6/12-beta1 SD

Start Room
This is the starting room.

You can see a big table here.

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

>drop tin
Dropped.

>get on table
You get onto the big table.

>drop mint
Perhaps you should take the breath mint out of the metal tin first.

>drop tin
Perhaps you should take the metal tin out of the Start Room first.

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?

DavidGriffith commented 8 years ago

More commentary from vlaviano:

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.

DavidGriffith commented 8 years ago

http://inform7.com/mantis/view.php?id=1888

vlaviano commented 8 years ago

For comparison, lib 6/11's behavior:

DROPSUB IMPLICIT TAKE                                                           
An Interactive Investigation                                                    
Release 1 / Serial number 160511 / Inform v6.33 Library 6/11 S                  

Start Room                                                                      
This is the starting room.                                                      

You can see a big table here.                                                   

>i                                                                              
You are carrying:                                                               
  a metal tin (which is closed)                                                 

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

>drop tin                                                                       
Dropped.                                                                        

>get on table                                                                   
You get onto the big table.                                                     

>drop mint                                                                      
You haven't got that.                                                           

>drop tin                                                                       
You haven't got that.
vlaviano commented 8 years ago

I've looked at this issue.

In lib 6/11, an attempt to drop an indirectly contained object would produce a response like this:

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

>drop mint                                                                      
You haven't got that.                                                           

People found this misleading, because the player does in fact have the object, despite not holding it directly.

This lead to Suggestion #122 on the old Inform 6 Suggestions List:

122.  You can take an item from the SACK_OBJECT but you can't drop an item directly if
it's in the sack; instead you get the rather unhelpful 'You haven't got that'. It would
be good to at least get a response like 'You need to take that out of the sack first'
(or whatever).

Notice the "at least" in the suggestion. The implication is that there are two levels at which this issue could be solved: (1) the library could provide a more helpful message indicating that the object must be taken out of the container before it's dropped instead of saying that the player doesn't have it, or (2) the library could go even further and allow the item to be dropped directly from the sack by performing an implicit take.

The first level solution was implemented in lib 6/12, producing the following results:

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

>drop mint
Perhaps you should take the breath mint out of the metal tin first.

The second level solution was implemented in the inform-2006 tree, ported into the inform6lib tree, disabled because of bugs and finally re-enabled with the recent fix for issue #31. The resulting behavior is:

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

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

This brings us to issue 32. The new "Perhaps you should take the foo out of the bar" library message was intended for dropping objects that are indirectly contained by the actor, but it also applies to dropping objects that are outside of the actor, but not siblings of the actor. For example, if an object is on the floor of a room, but the actor is on an enterable supporter in that room (or vice versa). In those cases, we see inappropriate output like:

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

>drop tin
Dropped.

>get on table
You get onto the big table.

>drop mint
Perhaps you should take the breath mint out of the metal tin first.

>drop tin
Perhaps you should take the metal tin out of the Start Room first.

There are several possible ways to resolve this.

In an earlier post, I suggested broadening the constraint "if noun in parent(actor)" (the test used to decide when to say "The foo is already here.") to something like "if noun in location".

However, having researched the history of this issue, I now favor simply reverting L__M(##Drop, 2) back to its pre-Suggestion #122 lib 6/11 form, with updates for the 6/12 voices and tenses feature.

The message was changed as a response to Suggestion #122. Now that the better solution (implicit taking) is in place, there's no longer a need for the message change. Indirectly contained objects are now implicitly taken if possible (and a parser NOTHELD error msg is produced if not - not ideal, but a separate issue related to an ImplicitTake call being added to the parser for multiheld tokens, distinct from its notheld mode for held tokens), so the library message is only produced for objects that are outside the actor. In these cases, the new message can be inappropriate ("Perhaps you should take the metal tin out of the Start Room first"), and the original "You haven't got that" message is more broadly applicable.

The change is minimal, which is good for stability this close to a release, and it maintains consistency with lib 6/11 and Inform 7 behavior (not strictly necessary, but I don't see a need to diverge gratuitously).

Here's the new msg, in english.h:

[ LanguageLM n x1 x2;
  ...
  Drop: switch (n) {
        ...
        2:  CSubjectVerb(actor, false, false, "haven't got", 0, "hasn't got",
                         "didn't have");
            " ", (the) x1, ".";
        ...

I retained "haven't/hasn't got" for the present tense, but "hadn't gotten" seemed slightly off to me for the past tense, so I went with "didn't have". I also followed the 6/12 trend of replacing "that/those" with the name of the object.

The output with this change in place is:

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

>drop tin
Dropped.

>get on table
You get onto the big table.

>drop mint
You haven't got the breath mint.

>drop tin
You haven't got the metal tin.

>get off table
You get off the big table.

>take tin
Taken.

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