Bug - Fixed Can the zap command return a list of possible matches when it can't find a unique one

[ereinion]

I just had this happen in the gCLI:

[COLOR=olive]> inv zombie maria[/COLOR]

zombie mariachi pants (2)

[COLOR=olive]> zap zombie maria[/COLOR]

[COLOR=red][zombie maria] has no matches.[/COLOR]

[COLOR=olive]> zap zombie mariachi[/COLOR]

[COLOR=red][zombie mariachi] has no matches.[/COLOR]

[COLOR=olive]> zap zombie mariachi p[/COLOR]

Zapping zombie mariachi pants...
You acquire an item: zombie accordion
zombie mariachi pants has been transformed.

I assume the reason I get different results is because mafia could find an unique match in my inventory using the substring I provided, but not in the list of zappable items? However, the error message can be a bit confusing. For instance I'd say it wouldn't be too farfetched of me to assume that I didn't have any zombie mariachi pants in my inventory, if I hadn't checked how many of them I had before doing the zap request. So what I'd like to request is that the zap command returns a list of possible matches in those cases where it can't figure out the exact item you want to zap.

 

[xKiv]

I assume the reason I get different results is because mafia could find an unique match in my inventory using the substring I provided

"inv $BAZ" just lists everything in yout inventory that has $BAZ as a substring - doesn't do any fuzzy matching, or uniqueness testing. And it most definitely doesn't print any items you don't have, even if they have $BAZ as a substring in their name.

However, the error message can be a bit confusing.

I have also sometimes noticed that I would prefer something that would list possibilities (unless there's too many of them - now, what's "too many", 10?, 20?, 50?).
For any command that wants to uniquely fuzzy match an item (and possibly skill, location, element, slot, did I leave out anything important?).

 

[roippi]

ItemFinder.java could in general stand a bit of refactoring/reworking. I'm not a fan of a number of things in there, most notably the setMatchType method which just seems like a rather arcane way of doing things. Not to mention buggy in a multithreaded environment.

Also there are a ton of special-cased things which can just give totally unexpected results. For example, did you know

		// Candy hearts, snowcones and cupcakes take precedence over
		// all the other items in the game, IF exactly one such item
		// matches.

or

		// First, check to see if there are an HP/MP restores
		// in the list of matches.  If there are, only return
		// the restorative items (the others are irrelevant).

..and some others.

ItemFinder code is used in a lot of places at this point so there are liable to be many side-effects of a revamp, which is presumably why nobody has felt up to it. Might be time for me to take a swipe at it though.

 

[roippi]

Okay, so in general, the zap command does list ambiguous matches-

> zap red

extra-strength red potion
red paisley oyster egg
red pixel potion
red polka-dot oyster egg
red potion
red striped oyster egg

[red] has too many matches.

The issue here is this block in ItemFinder#filterNameList:

		if ( nameList.size() == 1 )
		{
			return;
		}

		// Never match against (non-quest) untradeable items not available
		// in NPC stores when other items are possible.
		// This can be overridden by adding "matchable" as a secondary
		// use; this is needed for untradeables that do need to be
		// explicitly referred to, and have names similar to other items
		// (such as the NS Tower keys).

		nameIterator = nameList.iterator();

		while ( nameIterator.hasNext() )
		{
			itemName = nameIterator.next();
			itemId = ItemDatabase.getItemId( itemName );

			conditionalRemove( nameIterator, itemId != -1 &&
				!ItemDatabase.getAttribute( itemId,
					ItemDatabase.ATTR_TRADEABLE | ItemDatabase.ATTR_MATCHABLE | ItemDatabase.ATTR_QUEST ) &&
				!NPCStoreDatabase.contains( itemName ) );
		}

Basically: this is the last thing that happens when filtering. It only does something if there are still multiple matches. The idea is to strip out the non-tradable things, leaving the tradable ones - usually a good idea. However in this case the list consists of only non-tradable things:

zombie mariachi hat
zombie mariachi pants

Both get removed before ever getting to the step where ambiguous matches get printed.

I have to think about which angle I want to attack this from. Adding the "matchable" attribute to the set in items.txt is one approach, but maybe that block's logic needs tweaking.

 

[roippi]

r12868 should fix the issue. It's possible that this has side-effects where someone was depending on this filtering behavior, but I doubt it.