Bug - Fixed file_to_map fails to restore items with commas

ereinion

ereinion

Member
There seems to be some issues with categorizing some items in OCD, namely "Ouija board, ouija board" and "A Crimbo Carol, Ch. 3 (used)". I know I at least had the ouija board categorized earlier, and probably the crimbo charol too, but not too long ago they started showing up as wanting to be categorized whenever I ran OCD. Trying to recategorize them doesn't help, they just show up again, right afterwards.

I did find the ouija board, but not the carol, in the datafile:
Code: 
[7025]Ouija Board, Ouija Board    MALL    1
A wild guess would be that there is some problem with the parsing, perhaps with the commas as that is the only uncommon punctuation I see between the two?

I'm using kolmafia r16145, in case it matters.
 
H

heeheehee

Developer
Staff member
IIRC, OCD uses cli_execute to do some heavy lifting. Maybe it should be modified to use pilcrow notation (@Bale)?
 
fronobulax

fronobulax

Developer
Staff member
ereinion said:
I'm using kolmafia r16145, in case it matters.
Click to expand...

For a while scripts that used items as an index to maps behaved differently. I don't remember which version stabilized things but in case it was after 16145 I would run with the latest, categorize everything, shut down, restart and see whether the problem reoccurs.
 
Bale

Bale

Minion
heeheehee said:
IIRC, OCD uses cli_execute to do some heavy lifting. Maybe it should be modified to use pilcrow notation (@Bale)?
Click to expand...

I thought that I was using pilcrow notation. I'll have to check the program later when I can to see if my memory is wrong.
 
H

heeheehee

Developer
Staff member
Okay, well, I did some preliminary debugging.
Code: 
> ashq string [string] map; file_to_map("OCDdata_heeheehee.txt", map); print(map contains "[7025]Ouija Board, Ouija Board")

true

> ashq string [item] map; file_to_map("OCDdata_heeheehee.txt", map); print(map contains $item[ouija board, ouija board])

false

I might investigate more later.
 
H

heeheehee

Developer
Staff member
Some more testing:
Code: 
> ashq item[item] map; map[$item[7025]] = $item[7025]; print(to_json(map)); map_to_file(map, "test.txt"); file_to_map("test.txt", map); print(to_json(map));

{ "Ouija Board, Ouija Board" : "Ouija Board, Ouija Board" }
{ "none" : "none" }

Seems that, for some reason, items with commas in their names are being restored as $item[none].
 
Veracity

Veracity

Developer
Staff member
Yeah... Datatypes.parseItemValue() ends up calling ItemFinder.getFirstMatchingItem, which does this:

Code: 
		if ( parameters.contains( "\u00B6" ) || parameters.contains( "[" ) )
		{
			// At least one item is specified by item ID
			if ( parameters.contains( "," ) )
			{
				// We can't parse multiple items of this sort
				if ( errorOnFailure )
				{
					KoLmafia.updateDisplay( MafiaState.ERROR, "More than one item specified by item ID." );
				}
				return null;
			}

			int spaceIndex = parameters.indexOf( ' ' );
			if ( spaceIndex != -1 )
			{
				String itemCountString = parameters.substring( 0, spaceIndex );

				if ( StringUtilities.isNumeric( itemCountString ) )
				{
					itemCount = StringUtilities.parseInt( itemCountString );
					parameters = parameters.substring( spaceIndex + 1 ).trim();
				}
			}
Later in the method, it does this:

Code: 
			else if ( name.startsWith( "[" ) )
			{
				int index = name.indexOf( "]" );
				if ( index == -1 )
				{
					return null;
				}
				itemId = StringUtilities.parseInt( name.substring( 1, index ) );
			}
which is the code to parse the item ID wrapped in [].

I think that method is suspect, but I think that DataTypes.parseItemValue could handle it itself.
 
H

heeheehee

Developer
Staff member
Veracity said:
Yeah... Datatypes.parseItemValue() ends up calling ItemFinder.getFirstMatchingItem, which does this:

Code: 
		if ( parameters.contains( "\u00B6" ) || parameters.contains( "[" ) )
		{
			// At least one item is specified by item ID
			if ( parameters.contains( "," ) )
			{
				// We can't parse multiple items of this sort
				if ( errorOnFailure )
				{
					KoLmafia.updateDisplay( MafiaState.ERROR, "More than one item specified by item ID." );
				}
				return null;
			}
Click to expand...
That makes perfect sense, actually. I am surprised by the inability to parse multiple items specified by item ID, though.
 
Bale

Bale

Minion
Just for the record, this script don't sell using cli_execute() at all, not even with pilcrow notation. It uses put_shop(). The script only uses cli_execute in conjunction with pilcrow only for untinkering and pulverizing. I don't think that there are ash commands for those things.

It's definitely a mafia bug that is causing the issue.

Veracity and heeheehee, should I make a bug report for the problem, or do you have it in hand?
 
H

heeheehee

Developer
Staff member
I'm not entirely sure if the proper fix would be to overhaul how Mafia matches items, or if there's a simpler alternative. I can't see a good reason why, for instance, Mafia shouldn't be able to match something like "sell 1 [1], 1 [2]".
 
Veracity

Veracity

Developer
Staff member
When Darzil introduced [NUM]NAME notation for items and effects - with the NAME being essentially a comment - he added recognition for that syntax into ItemFinder.getFirstMatchingItem, since that is used all over the place for accepting user input for item "names". We are grateful that he did so.

ItemDatabase.getItemId has its own recognition of that syntax. This is intended for programmatic use when you know that the "name" you pass in refers to a single item. It has an optional parameter - substringMatch - which controls whether fuzzy matching will be in used.

ASH's Datatypes.parseItemValue uses ItemFinder.getFirstMatchingItem. I'm not sure why. It seems like it could use ItemDatabase.getItemId with fuzzy matching. That seems like the "simpler alternative" for this specific bug. I will experiment.

Regarding the "sell" command that you give as an example - that's an example of parsing user input which might have multiple items in it. That uses ItemDatabase.getMatchingItemList, which first attempts to see if the string refers to a single item - which might have a comma in its name - and if not, splits the string over "\s*,\s*" and uses ItemFinder.getFirstMatchingItem on each one. Making the "sell" command work the way you want would require an "overhaul" - or, at least, some work in ItemFinder.getFirstMatchingItem.
 
Veracity

Veracity

Developer
Staff member
The simple fix to Datatypes.parseItemVale seems to work:

> ashq item[item] map; map[$item[7025]] = $item[7025]; print(to_json(map)); map_to_file(map, "test.txt"); file_to_map("test.txt", map); print(to_json(map));

{ "Ouija Board, Ouija Board" : "Ouija Board, Ouija Board" }
{ "Ouija Board, Ouija Board" : "Ouija Board, Ouija Board" }
Click to expand...

test.txt:

Code: 
[7025]Ouija Board, Ouija Board	[7025]Ouija Board, Ouija Board
and this continues to work correctly:

> ash to_item( "potion of object detection" ).to_string()

Returned: smoky potion
Click to expand...
Revision 16155.
 
You must log in or register to reply here.
Top