Feature - Implemented Friendly warnings for fuzzy matches in $type[]

[zarqon]

The recent change to location names has prompted me to request something I've wished for periodically from time to time.

Script authors, for various reasons (convenience, memory failure, haste, laziness, syntax highlighting), will sometimes not use the complete name when specifying one of mafia's typed constants. Example: $monster[Yog-Urt]. There's more to her name, of course. The Goddess of Hating... er, Elder Demon of Hating... er... Hatred? Ok, I'll just shorten it to "Yog-Urt" for now because I'm concentrating on testing script functionality, not name specifics.

Later, it can be extremely difficult to remember and then locate these instances, in order to flesh them out. Since mafia fuzzy matches them (which is nice) and doesn't complain about it (which is polite), those of us with poor memories for such details (me, at the very least) often completely forget that we left a partial name in our script. Later, when KoL adds a similar thing and our script generates a "multiple matches" error, we remember... but TOO LATE!

I'd find it quite handy if mafia would print warning messages for incomplete names when specified in $type[] (but not for CLI commands, naturally), complete with script name and line number, so that all those instances of partial names could be eliminated. When dealing with user input, the fuzzy matching is extremely helpful. When dealing with hardcoded names in ASH, the fuzzy matching is still helpful when testing, but for a final release, there's no good reason not to use a complete name.

It would be particularly handy now when updating scripts with multiple $locations[] in them (such as CanAdv or BBB), since many of the old location names still match, despite being incomplete!

ETA: Example warning message:

Warning: Incomplete name given for Yog-Urt, Elder Goddess of Hatred (BatBrain.ash, line 812)

 

[Bale]

I'd like to modify this request.

Please print these warnings only when using the verify command. Otherwise a lot of scripts will become annoying. (Yep, I used the "a" word!) If these warnings are delivered when we do a "verify scriptname", then we can learn about these so that we can fix them if we want, but it won't annoy the users with warnings if we don't want to do so. A lot of people aren't going to fix their scripts (some of whome are not active) and I don't want to be troubled by them. I'll be troubled just by the people posting in the threads with, "Oh noes! The sky is falling because I'm getting cryptic warnings I don't understand!"

Other than that, awesome idea. I want this also.

I understand that line numbers are tricky to give because of the way it is implemented. That's fine, I can always just search and replace. Like I did for the big location name revamp.

 

[zarqon]

The same people who didn't fix their scripts after the location change will be the same people who don't revise their scripts after implementing this. It wouldn't be any more or less annoying.

That said, I love Bale's suggestion and was holding it in reserve myself for if complaints were voiced. Only printing those messages when explicitly verifying a script would be a nice way to avoid complaints. I could see differentiating CLI-specified verification from regular about-to-run-the-script verification as a potential obstacle, however.

Line number would be desirable, of course, but at the very least the script name would be useful. When my CLI became a flood of location messages/errors between combats, it took me quite a while to narrow all of them down (surprisingly few of them were BBB).

ETA: Perhaps wording the message as

Friendly reminder: Incomplete name given for X (script, line)

rather than "Warning" would prevent "bug reports" from confused users. Or perhaps

Not a big deal, but: Incomplete name given for X (script, line)

or

Nothing worth mentioning (certainly not on the forum), but: Incomplete name given for X (script, line)

 

[Veracity]

This is what happens when you "verify" a script:

KoLmafia gets (and caches) an Interpreter to execute the script.

This is what happens when you execute a script:

KoLmafia gets (and caches) an Interpreter to execute the script.
KoLmafia invokes the Interpreter

In other words, "verifying" a script is nothing more or less than building an Interpreter which can be used to execute the script; it is not a special mode of parsing a script. Which is to say, we cannot distinguish at parse time between "getting an Interpreter which will not be invoked right away" vs. "getting an Interpreter which will be immediately used".

How about this:

item it1 = $item[ snorkel ];
item it2 = $item[ snork ];
print( "it1 = " + it1 + " it2 = " + it2 );
location loc1 = $location[ (Top Floor) ];
location loc2 = $location[ Giant's Castle (Top Floor) ];

> nnn.ash

Warning: Name "snork" is incomplete or deprecated for $item[snorkel] (nnn.ash, line 2)
Warning: Name "(Top Floor)" is incomplete or deprecated for $location[Castle in the Clouds in the Sky (Top Floor)] (nnn.ash, line 4)
The string "Giant's Castle (Top Floor)" no longer matches a location name; use "Castle in the Clouds in the Sky (Top Floor)" instead
Warning: Name "Giant's Castle (Top Floor)" is incomplete or deprecated for $location[Castle in the Clouds in the Sky (Top Floor)] (nnn.ash, line 5)
it1 = snorkel it2 = snorkel

This is coded and working. All I need to do is submit it.

(Perhaps "Sloppy code" would be better than "Warning" )

If you want to try it, insert:

				// If validating script, give warning if fuzzy matching kicked in
				String fullName = value.toString();
				if ( !input.equals( fullName ) )
				{
					ScriptException ex = this.parseException( "Warning: Name \"" + input + "\" is incomplete or deprecated for $" + type.toString() + "[" + fullName + "]" );
					String message = ex.getMessage();
					RequestLogger.printLine( message );
				}

at line 3053 of Parser.java

 

[zarqon]

Yeah, I suspected there wasn't a way to tell verifying vs. running. I'd be a fan of implementing this as you have it, but I know for certain it would cause a pretty big flurry of activity, not to mention minor headaches for users of scripts where the author isn't updating them (not a small number of people).

I also do seriously think that we should use something gentler than the word "Warning" for this particular message. Users are accustomed to warnings meaning that they need to change something, and they wouldn't know what to change and would go report it somewhere, believing the error to be more serious than it actually is.

Some message options:

• Recommended: change $item[snork] to use exact name $item[snorkel] (nnn.ash, line 2)
• Changing "snork" to "snorkel" would get rid of this message (nnn.ash, line 2)
• Sloppy code: Name "snork" is incomplete or deprecated for $item[snorkel] (nnn.ash, line 2)
• Oh by the way: Name "snork" is incomplete or deprecated for $item[snorkel] (nnn.ash, line 2)

I kinda like that second one best. Although your suggestion (#3) is also nice.

Or, a "deslop" command that uses a stripped-down interpreter that only evaluates typed constants?

 

[jasonharper]

It seems like this would cause problems with things that have Unicode characters in their names, and therefore can only be referred to via a fuzzy match in the script's source code.

 

[Veracity]

ASH parses the character entities for Unicode characters correctly. Look at item #3465.

Bash-Ōs cereal

print( $item[ Bash-&[b][/b]#332;s cereal ].plural );

> uni.ash

Bash-Ōs cereals

 

[zarqon]

In that case, the warning message should probably include typed-out character entities (including the ampersand and number sign) for convenient copy-pasting. That's the main purpose of these messages after all, to provide for convenient fixing of laziness. Ha!

 

[Veracity]

Unfortunately, since the gCLI displays HTML, it will translate character entities into unicode for you. I imagine there is a way, but right off hand, I'm not sure how to escape them so the characters that make up the character entity are displayed.

 

[lostcalpolydude]

Using the character entity for & would work, right?

 

[Alhifar]

Assuming you have the entity already, just change & to &

EDIT: Got ninja'd in the time it took me to type that on my phone >.<

 

[Veracity]

So, I escape the string which contains unicode to get a string which has character entities.
Then I escape it again so you can actually see the character entities.

Edit: well, it turns out that I already have the string with character entities - and we have a method CharacterEntities.encode() which will escape it.

print( $item[ Bash-&[b][/b]#332;s cereal ].plural );
print( $item[ Bash cereal ].plural );

yields

> uni.ash

Warning: Name "Bash cereal" is incomplete or deprecated for $item[Bash-&#332;s cereal] (uni.ash, line 2)
Bash-Ōs cereals
Bash-Ōs cereals

 

[Veracity]

item it1 = $item[ snorkel ];
item it2 = $item[ snork ];
print( "it1 = " + it1 + " it2 = " + it2 );
location loc1 = $location[ (Top Floor) ];
location loc2 = $location[ Giant's Castle (Top Floor) ];
print( "loc1 = " + loc1 + " loc2 = " + loc2 );
print( $item[ Bash-&[b][/b]#332;s cereal ].plural );
print( $item[ Bash cereal ].plural );

yields

> nnn.ash

Changing "snork" to "snorkel" would get rid of this message (nnn.ash, line 2)
Changing "(Top Floor)" to "Castle in the Clouds in the Sky (Top Floor)" would get rid of this message (nnn.ash, line 5)
The string "Giant's Castle (Top Floor)" no longer matches a location name; use "Castle in the Clouds in the Sky (Top Floor)" instead
Changing "Giant's Castle (Top Floor)" to "Castle in the Clouds in the Sky (Top Floor)" would get rid of this message (nnn.ash, line 6)
Changing "Bash cereal" to "Bash-&#332;s cereal" would get rid of this message (nnn.ash, line 9)
it1 = snorkel it2 = snorkel
loc1 = Castle in the Clouds in the Sky (Top Floor) loc2 = Castle in the Clouds in the Sky (Top Floor)
Bash-Ōs cereals
Bash-Ōs cereals

Revision 12273

 

[Bale]

Whoa. The warnings are case sensitive?!

Changing "song of the glorious lunch" to "Song of the Glorious Lunch" would get rid of this message (eatdrink.ash, line 1318)

 

[Veracity]

Does seem excessively pedantic - although it does catch something I'd consider to be sloppy.

Revision 12274 ignores case.

 

[zarqon]

Perfect, thank you Veracity! Somehow I suspected you would take interest in implementing this, hehe.

 

[fronobulax]

Thank you.

 

[zarqon]

One further concern as I'm correcting my scripts one by one (I'm pleased with the number of scripts so far that don't incite any warnings, I'd kind of assumed they all would): this doesn't appear to work for plural typed constants.

Question: should it? I tend to think that if we're doing it for single ones, we should also do it for plural ones.

 

[Veracity]

plural whats? Example?

 

[zarqon]

> ash foreach f in $familiars[hobo mon, glutton, slime] print(f)

Hobo Monkey
Gluttonous Green Ghost
Slimeling
Returned: void

I'd halfway expected to see messages pop up for those, too.