Subcommands bug

Hi! I’ve found a bug in the Subcommands extension. Well, in my 6M62 port of it at least.

It gives a run-time error when you try to check the subcommand of something after the parser has printed a “clarifying the choice of” parenthetical, such as “(from the barker)” here.

[code]“Subcommands bug.”

Include Subcommands by Daniel Stelzer.

Before doing anything:
if the noun is not nothing:
say “Noun subcommand: [the subcommand of the noun].”;
if the second noun is not nothing:
say “Second noun subcommand: [the subcommand of the second noun].”;
continue the action.

Understand the command “buy” as something new.
Understand “buy [something] from [someone]” as buying it from. Buying it from is an action applying to two visible things.

Carry out buying something from someone (this is the standard purchasing rule):
say “[The noun] [do] not seem to be available for sale.”

The Midway is a room. The description is “Here in front of the pharmacy in the southwestern corner of the town square, various contests have been set up – a strong-man contest, a contest to see who can burst the most balloons using a styrofoam dart-plane, and so on.”

The barker is a man in the Midway. The barker wears a blue suit. The description of the barker is “He is dapper in his suit, as though he might belong to an especially vivid barber-shop quartet.” The description of the suit is “Carefully tailored in bright blue linen, with fine white pinstripes.”

The barker is carrying a tube. The description of the tube is “The tube claims to be full of restoration gel, but said gel has mostly gone. If only it had been a larger container to start with.” Understand “gel” or “prize” or “fabulous prize” or “label” or “restoration gel” as the tube.

Test me with “buy tube from barker / buy suit from barker / buy / buy suit”.[/code]
And another question altogether: is there any way to prevent the parser from printing that clarifying text? I’ve tried with different “Rule for clarifying the parser’s choice of” and “Rule for supplying a missing second noun”, but none seems to work in this case.
subcommands.i7x (15.7 KB)

Try adding this (while also keeping your current grammar):

Understand "buy [something]" as buying it from.

Rule for supplying a missing second noun while buying something from:
	if the location of the barker is the location, now the second noun is the barker;
	otherwise say "You will have to specify whom to buy [the noun] from."

This will allow you to type commands like “buy suit” and “buy tube” and have the barker selected as the second noun without clarifying text being printed.

If you want to also allow “buy” with no noun, no second noun, and no clarifying text, you have a couple of options:

(1) You could understand “buy” as buying it from and write a ‘rule for supplying a missing noun while buying something from’, and it would be up to your rule to decide which noun to pick.

(2) You could write a ‘rule for clarifying the parser’s choice of’ that does nothing. This would suppress the clarifying text, but the parser would still make a choice as it usually does. You could affect this choice via ‘does the player mean’ rules.

However, I wouldn’t recommend either of these latter two options. I think that inferring a noun in this scenario without printing clarifying text is a bad idea. You’d have exchanges like:

Thanks! I think that the first part will do. I can see that I should have read the documentation, it is pretty clear there. (§18.32).

I also checked the Subcommands bug with 6L38 and Draconis’s original version, and the same run-time problem happened. So at least I didn’t break it.

I think the first thing to do about the subcommands bug is to decide what the value of subcommand (a snippet) should be for noun or second noun that doesn’t match anything in the player’s input because the parser inferred it or a rule for supplying a missing noun/second noun supplied it.

A couple of other observations:

If an object isn’t matched in the input and later becomes the noun or second noun (e.g., via a rule for supplying), its subcommand value will be an outdated one that was assigned the last time that it was matched (likely during a previous command).

The same object can be both the noun and the second noun (BUY BARKER FROM BARKER), and it can be referred to differently in each part of the command (BUY BARKER FROM MAN), yet each object has only one subcommand property with a single snippet value.

Those are all valid concerns, of course. But I think it would be an improvement if we could just silence the error for now, i.e. put in an extra check somewhere to make the parser know the correct length of the subcommand text and not look for words 3 to 2 in an empty string.

I don’t quite understand what goes on in the modified NounDomain or in the parsing in general. It seems that what should be happening is that every matched object is given a string property (called subcommand) that corresponds to the words that the parser used to identify it. But now when we ask the parser to try looking into that subcommand value, it seems to look for words at certain positions in the player’s command rather than at a string we gave to the object, positions that may no longer exist because the player’s command has changed.

The subcommand property is a snippet, not a string. A snippet is a reference to word positions in the input buffer – it’s literally two numbers (firstword, wordcount) packed into a value.

Yeah, I figured it was something like that. So is there any way to make it a string instead?

The basic sequence of events during parsing is input -> tokenization -> parsing -> action generation (or parser error).

The user types in some input, which goes into the input buffer as a sequence of characters. Tokenization is the process of dividing the input into distinct words and looking each one up in the game’s dictionary. The results of this process go into a separate buffer called the parse buffer. It winds up containing the number of words in the input and a record for each word consisting of the address of the dictionary entry for that word (or 0 if the word wasn’t found), the offset into the input buffer where the corresponding word begins, and the length of the word in characters.

The contents of the parse buffer are then used in the parsing phase to match grammar tokens and determine which actions should be generated. NounDomain is a routine called during parsing that tries to match a presumed object name in the input against the collection of game objects that are in a certain scope (usually the contents of the player and the contents of the location). This is the routine that the Subcommand extension reimplements with extra code to note which portion of the input matched a particular object and to store that information as an object property.

So, with that background in mind, the misconception in what you wrote is that subcommand is a string property (called a text in I7). It’s actually a snippet property which is a type representing a consecutive sequence of words in the player’s input. Under the covers, it boils down to a word number and a length. The word number indicates which word in the parse buffer begins the sequence, and the length indicates how many words are in the sequence.

(On preview, I see that Zarf came by and said the essence of this in two sentences.)

Yeah, it could be a string. I suspect that Draconis didn’t want to deal with managing storage for and copying a variable-length text when he could store a fixed length snippet through which the text could be accessed.

One issue with snippets is that the default value is less obviously uninitialized. A number defaults to 0, a text defaults to “”, but a snippet defaults to word 1 of the input buffer. That’s why, with the rule for supplying a missing second noun in place, we get:

I experimented with using length 0 snippets as a way to represent “doesn’t exist”. I7 seems happy enough with them. I’m to able to print them and perform comparisons against them without generating runtime errors:

Include (-
[ ZeroSnippet obj;
  obj.parsed_snippet = (obj.parsed_snippet / 100) * 100;  ! zero out length
];
-)

To zero out (obj - object):
	(- ZeroSnippet({obj}); -)

After reading a command when the player's command includes "buy":
	zero out the barker;
	if the subcommand of the barker includes "barker":
		say "barker's subcommand includes 'barker'.";
	otherwise:
		say "barker's subcommand doesn't include 'barker'."

Sorry, I’m afraid I don’t quite understand how to use that.

Even if I zero out the tube before checking its subcommand, there will still be a run-time problem if the player supplies a second noun:

After reading a command when the player's command includes "buy": zero out the tube; if the subcommand of the tube includes "gel": say "tube's subcommand includes 'gel'."; otherwise: say "tube's subcommand doesn't include 'gel'."

And what does that (obj.parsed_snippet / 100) * 100 do?

It sets the 2 lower decimal digits of parsed_snippet to zero (it’s integer division so 2 decimal digits are lost). eg 1234 / 100 = 12, 12 * 100 = 1200

Specifically this would appear to set one of the snippet values to 0, according to the way snippets are packed and stored in a single inform number.

As per inform7.com/extensions/Ron%20New … ce_17.html it looks like the snippet is packed into a single number by ensuring the character offset for the beginning is a number from 0-99 (which wraps around for very long input?) and the offset for the end is packed into the same number by multiplying it by 100, and these are added together. This seems unusual to me only in the choice of a base-10 number, rather then a base-2 (like 128)

As turnip said, it sets the length portion of the snippet to 0. The word number and length are packed into a snippet by:

snippet = word number * 100 + length

so, snippet / 100 gives the word number and snippet % 100 gives the length.

(snippet / 100) * 100 isolates the word number, discarding the length, and then multiplies it by 100, which turns it back into a snippet with a 0 length.

I instrumented the code a bit more, removed the ‘after reading a command’ rule and instead added code to zero the snippet length for the second noun in the ‘rule for supplying a missing second noun’. I also have this rule print the snippet’s numerical representation before and after zeroing the length. Here’s the code that I’m using:
[rant][code]
“Subcommands bug.”

Include Subcommands by Daniel Stelzer.

Before doing anything:
if the noun is not nothing:
say “Noun subcommand: [the subcommand of the noun].”;
if the second noun is not nothing:
say “Second noun subcommand: [the subcommand of the second noun].”;
continue the action.

Understand the command “buy” as something new.
Understand “buy [something] from [someone]” as buying it from. Buying it from is an action applying to two visible things.

Understand “buy [something]” as buying it from.

Rule for supplying a missing second noun while buying something from:
if the location of the barker is the location:
now the second noun is the barker;
say “before: [parsed snippet for the second noun].”;
zero out parsed snippet for the barker;
say “after: [parsed snippet for the second noun].”;
otherwise:
say “You will have to specify whom to buy [the noun] from.”

Include (-
[ PrintParsedSnippet obj;
print obj.parsed_snippet;
];

[ ZeroParsedSnippet obj;
obj.parsed_snippet = (obj.parsed_snippet / 100) * 100; ! zero out length
];
-)

To say parsed snippet for (obj - object):
(- PrintParsedSnippet({obj}); -)

To zero out parsed snippet for (obj - object):
(- ZeroParsedSnippet({obj}); -)

Carry out buying something from someone (this is the standard purchasing rule):
say “[The noun] [do] not seem to be available for sale.”

The Midway is a room. The description is “Here in front of the pharmacy in the southwestern corner of the town square, various contests have been set up – a strong-man contest, a contest to see who can burst the most balloons using a styrofoam dart-plane, and so on.”

The barker is a man in the Midway. The barker wears a blue suit. The description of the barker is “He is dapper in his suit, as though he might belong to an especially vivid barber-shop quartet.” The description of the suit is “Carefully tailored in bright blue linen, with fine white pinstripes.”

The barker is carrying a tube. The description of the tube is “The tube claims to be full of restoration gel, but said gel has mostly gone. If only it had been a larger container to start with.” Understand “gel” or “prize” or “fabulous prize” or “label” or “restoration gel” as the tube.

The barker has a text called nickname.

When play begins:
now the nickname of the barker is “Steve-o!”

Test me with “buy tube from barker / buy suit from barker / buy / buy suit”.
Test more with “buy tube / buy suit / buy tube from barker / buy suit from barker / buy / buy tube / buy tube from barker / buy tube”.
[/code][/rant]
Looking at the results of “test me” and “test more”, we see that the runtime errors first start occurring when the parser infers a noun (and prints a clarification in parentheses) and then happen subsequently when we fail to fully specify both the noun and second noun.

It’s reasonable to ask “is the problem really happening because the parser is inferring a noun, or is it because barker is the thing being inferred, which causes the noun to be the same as the second noun?”

To determine this, I added a ‘does the player mean’ rule to the program:

Does the player mean buying the tube from: it is very likely.

and reran “test more”. Here are the results, starting at step 5 where we first saw the runtime error:

This suggests that it is inferring the noun, and not the noun being the same as the second noun, that triggers the problem.

I’m going to take a closer look at the extension and see if I can get to the bottom of this.

(Grab a cup of coffee for this one…)

So, after further investigation, I’ve learned that my earlier statement that I7 is happy with zero-length snippets needs some qualification.

It’s only happy with zero-length snippets that start at word 1. It treats these as “empty” in just the way that we wanted. However, zero-length snippets that start at any other word will produce a runtime error.

(The Subcommands extension sometimes violates this constraint and sets an object’s subcommand to a zero-length snippet starting at a word number > 1, hence the runtime errors that Angstsmurf initially observed. We’ll get back to that later in this post.)

Here’s PrintSnippet from the I6 template code:

[ PrintSnippet snip from to i w1 w2;
	w1 = snip/100; w2 = w1 + (snip%100) - 1;
	if ((w2<w1) || (w1<1) || (w2>WordCount())) {
		if ((w1 == 1) && (w2 == 0)) rfalse;
		return RunTimeProblem(RTP_SAYINVALIDSNIPPET, w1, w2);
	}
	from = WordAddress(w1); to = WordAddress(w2) + WordLength(w2) - 1;
	for (i=from: i<=to: i++) print (char) i->0;
];

Recall that snip/100 is the word where the snippet begins and snip%100 is the snippet length in words. We can see that, when the snippet length is 0, w2 is set to w1 - 1, which explains the messages like “words 2 to 1”.

Notice the special-casing for w1 == 1 && w2 == 0. The other template routines SpliceSnippet and SnippetIncludes have similar special-casing.

However, SnippetMatches uses topic GPRs (general parsing routines) that behave a little strangely with length 0 snippets. Here’s an example GPR used to match a snippet against the topic “hello”:

[ Consult_Grammar_100 
    range_from ! call parameter: word number of snippet start

    range_words ! call parameter: snippet length

    original_wn ! first word of text parsed

    group_wn ! first word matched against A/B/C/... disjunction

    w ! for use by individual grammar lines

    rv ! for use by individual grammar lines

    ;
    wn = range_from; original_wn = wn; rv = GPR_PREPOSITION;
        if (NextWordStopped() ~= 'hello') jump Fail_1;
        if ((range_words==0) || (wn-range_from==range_words)) return rv;
        .Fail_1; rv = GPR_PREPOSITION; wn = original_wn;
    return GPR_FAIL;
];

range_from is the snippet start word and range_words is the snippet length. This tests the first word in the snippet against “hello” and returns failure if it doesn’t match. If it does match, it tests to see if the snippet length is 0 or the snippet length equals the number of words it took to match the token (in this case, that the snippet length is 1). If this test succeeds, the routine returns success. Otherwise, it returns failure. It’s unclear to me why range_words == 0 is treated as success here.

So, that digression aside, we can understand what happened during the “test more” test in my previous post. Recall that the output was:

There was only one word of input, so the parser had to infer the noun. This caused match_from to be 2 (the word beyond the end of the input) and match_length to be 0 in NounDomain. The extension used these values to set the noun’s parsed_snippet to match_from * 100 + match_length, resulting in a value of 200. Specifically, this was via the case where number_matched > 1 but Adjudicate is able to pick a single winner.

After this, the ‘rule for supplying a missing second noun’ kicked in, picked the barker as the second noun, and zeroed out its snippet. This was already zero, so it was a nop.

Finally, the ‘before doing anything’ rule attempted to print out the noun and second noun’s subcommand properties. This triggered calls to PrintSnippet, which generated the runtime errors via RunTimeProblem().

Both the noun and the second noun ought to have generated errors, but it seems that after a runtime error is printed, subsequent ones are suppressed until the next turn. See RTP.i6t: Reporting and how RTP_Buffer–>0 is used in conjunction with ClearRTP.

In order to fix some of the errors in my test program, we can change ZeroParsedSnippet to not only zero the snippet length, but also to set the snippet start word to 1:

[ ZeroParsedSnippet obj;
  obj.parsed_snippet = 100;
];

However, this only helps us when the inferred noun is the same as the second noun. If we include the ‘does the player mean buying the tube from: it is very likely’ rule, we still see errors because of the extension returning 200 when we type BUY and the parser infers the tube as the noun.

To really fix the problem, we need to modify Subcommands.i7x:

    wn = match_from+match_length;
    ! === NEW ===
    if (match_length == 0) {
        snip = 100;
    } else {
        snip = 100 * match_from + match_length;
    }
    ! === END ===

I’m not sure why snip isn’t used to set parsed_snippet here:

	if (dont_ask) {
	! === NEW ===
	(match_list-->0).parsed_snippet = 100 * match_from + match_length;
	! === END ===
	return match_list-->0;
	}

We should either use snip here, or we’ll need to add a similar conditional as the one above for when match_length == 0.

This mostly fixes things, but it still leaves authors having to deal with clearing stale subcommands in rules for supplying missing nouns and second nouns. To fix this, we also need to replace and modify ActionVariablesNotTypeSafe, from which the two supplying a missing noun/second noun activities are called:

	    if ((mask & NEED_NOUN_ABIT) && (noun == nothing)) {
	    	@push act_requester; act_requester = nothing;
	        CarryOutActivity(SUPPLYING_A_MISSING_NOUN_ACT);
	        @pull act_requester;
	        if (noun == nothing) {
	        	if (say__p) rtrue;
		    	if (actor == player) { ACTION_PROCESSING_INTERNAL_RM('D'); new_line; }
		        rtrue;
	    	}
                ! === NEW ===
                noun.parsed_snippet = 100;
                ! === END ===     
	    }

(and similarly for the second noun)

I’ve attached a version of Subcommands.i7x with these changes to this post.

Here’s my earlier test program modified not to alter any of the snippet values at the app level (i.e., outside of the extension) and altered to print out empty snippets in a more pleasant way.
[rant][code]
“Subcommands bug.”

Include Subcommands by Daniel Stelzer.

Before doing anything:
if the noun is not nothing:
say “Noun subcommand: [subcommand for the noun].”;
if the second noun is not nothing:
say “Second noun subcommand: [subcommand for the second noun].”;
continue the action.

Understand the command “buy” as something new.
Understand “buy [something] from [someone]” as buying it from. Buying it from is an action applying to two visible things.

Carry out buying something from someone (this is the standard purchasing rule):
say “[The noun] [do] not seem to be available for sale.”

Understand “buy [something]” as buying it from.

Rule for supplying a missing second noun while buying something from:
if the location of the barker is the location:
now the second noun is the barker;
otherwise:
say “You will have to specify whom to buy [the noun] from.”

Does the player mean buying the tube from: it is very likely.

Include (-
[ PrintParsedSnippet obj;
print obj.parsed_snippet;
];

[ ZeroParsedSnippet obj;
obj.parsed_snippet = 100;
];
-)

To say parsed snippet for (obj - object):
(- PrintParsedSnippet({obj}); -)

To zero out parsed snippet for (obj - object):
(- ZeroParsedSnippet({obj}); -)

To decide whether (S - snippet) is empty:
(- {S} == 100 -)

To say subcommand for (obj - object):
say “[if subcommand of obj is empty](absent from input)[else][subcommand of obj]”.

The Midway is a room. The description is “Here in front of the pharmacy in the southwestern corner of the town square, various contests have been set up – a strong-man contest, a contest to see who can burst the most balloons using a styrofoam dart-plane, and so on.”

The barker is a man in the Midway. The barker wears a blue suit. The description of the barker is “He is dapper in his suit, as though he might belong to an especially vivid barber-shop quartet.” The description of the suit is “Carefully tailored in bright blue linen, with fine white pinstripes.”

The barker is carrying a tube. The description of the tube is “The tube claims to be full of restoration gel, but said gel has mostly gone. If only it had been a larger container to start with.” Understand “gel” or “prize” or “fabulous prize” or “label” or “restoration gel” as the tube.

The barker has a text called nickname.

When play begins:
now the nickname of the barker is “Steve-o!”

Test me with “buy tube from barker / buy suit from barker / buy / buy suit”.
Test more with “buy tube / buy suit / buy tube from barker / buy suit from barker / buy / buy tube / buy tube from barker / buy tube”.
[/code][/rant]
and here’s the output of running “test me” and “test more” with this program and the updated Subcommands extension:
[rant]

[/rant]
Please take a look and let me know if anything seems broken. Thanks.

Edit: This patch doesn’t address some issues that arise from noun and second noun being the same object with only one subcommand property that is written twice, once for the object’s role as noun and once for its role as second noun.

For example, if we type BUY BARKER with a rule for supplying a missing second noun in place, then the barker object’s subcommand will be set to an empty snippet (100) even though it was referred to in the input in its role as noun.

Similarly, if we type BUY BARKER FROM, the parser will infer (barker) as the second noun and the extension will set the object’s subcommand to the empty snippet.

If the player refers to the object in both roles explicitly, but using different noun phrases (e.g., BUY BARKER FROM MAN), then only one of those phrases will be captured in the object’s subcommand property.

Finally, if the player refers to the object in both roles explicitly using the same noun phrase, there will still be a subtle (and probably unimportant) error in that the subcommand’s start word value will be incorrect for one of the object’s roles.

To solve these issues, we’d need to store two snippets for each object, one for noun and one for second noun, and figure out which to use when assigning their values, which might be a problem from within NounDomain. It seems as though noun would always be filled in first, but I recall that the parser does lookahead for MULTIEXCEPT and MULTIINSIDE tokens so there may be cases where the second noun is parsed first. We’ll have to look at the details there and see what we can do. We’ll also have to take action_reversed into account.
Subcommands.i7x (18.8 KB)

1 Like

Thanks for all the help, vlaviano! I wasn’t as careful as I should have been in checking for invalid snippets.

The issues with the same object being both the noun and the second noun are an unfortunate consequence of another use-case: I wanted the subcommand to be a single property, regardless of where it was parsed, so that it could be useful in disambiguation control. Probably the ideal solution would be to alter snippets to hold four values, with functions like MatchSnippet recursing if necessary—but allowing 2 decimal digits for each field runs into the limit of the Z-machine’s 16-bit integers, so I’d have to recode all of the snippet functions to use e.g. a factor of 64 instead…either way, I’ll start experimenting with this when I get a chance.

Yeah, thanks again! I’ve put Subcommands back in and everything seems to work swimmingly. Good work!