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

OOPS processing broken by L61109 #44

Open
DavidGriffith opened this issue May 31, 2016 · 0 comments
Open

OOPS processing broken by L61109 #44

DavidGriffith opened this issue May 31, 2016 · 0 comments
Labels
Milestone

Comments

@DavidGriffith
Copy link
Owner

Assessment from @vlaviano:

Oops assumes that the parser stops parsing a syntax line when there's an error and moves on to the next line. That's no longer true after the L61109 fix. Now, the parser keeps parsing the line looking for "better" errors, but oops_from is still reset each time the parser tries to match a new noun token. This is a problem, because there's no guarantee that the new token will fail or, if it does, that its error will be selected as line_etype over the earlier one. I think the fix is to tie oops_from more directly to line_etype and best_etype. We'd replace saved_oops with line_oops and best_oops. One wrinkle is that, if multiple lines produce the same error, it actually matters for oops which line we consider to have failed since their oops_from values could differ. On the positive side, we'd have the opportunity to choose based on more than just the order of the grammar lines.

The problem with "GIVE ROCK TO FOO. OOPS GIRL" was that both ##Give's regular and reversed grammar lines were failing with CANTSEE_PE. The CantSee function sets saved_oops, and later, if CANTSEE_PE is the best error message across all grammar lines, oops_from is set from saved_oops. Since each of these two lines was setting saved_oops differently, the order that they appeared in the grammar determined the order in which they wrote saved_oops and thus the final value of saved_oops that was used. In this case, the winning line was the reversed grammar that didn't match the sense of the original input and, within that line, oops_from was set to a less obvious part of the input. The reversed grammar line (* creature held) first failed to match "ROCK" with the creature token, producing ANIMATE_PE, then the L61109 fix caused the parser to continue parsing the line and try to match "TO" with the held token, producing CANTSEE_PE and setting saved_oops accordingly. (The fix for L61128 skips past descriptors, but that doesn't help here.)

This bug was originally reported as Issue #11. I managed to mask some of the symptoms of this with 7b046b7 and closed that Issue so that 6.12.1 can be released.

@DavidGriffith DavidGriffith added this to the 6.12.2 milestone Jun 2, 2016
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