Bug - Fixed Sushi no longer creatable via CLI

[AtlanteanScion]

Sushi is no longer creatable via the CLI as of r16354 ("Item Finder checks Mixing Method for creatables based on itemId rather than item name. Should fix ash create issue.").

For example, the command "create magical giant dragon roll" succeeds with r16353, but fails with r16354.

Looking at the code, I'm guessing that the call to ItemDatabase.getItemId() (ItemFinder.java Line 227) fails for sushi because sushi doesn't have an item ID.

 

[Bullson]

Make/Eat Sushi still broken as of r16394.

 

[Veracity]

As somebody who non-optimally likes to get Fishy via eating sushi, the arrival of the Ice Hole means I'll want to look at this soon...

 

[Darzil]

It's all related to ItemFinder and item filtering. I'm currently torn between adding functions for getting Consumption type by name for items with no item id, turning the get Consumption type to send two parameters, first itemId, then itemName, and using the second only if the first doesn't match, and getting that to work, or doing the whole assigning item numbers to everything that we have talked about and not done for a while.

 

[Veracity]

I've been meaning to comment on the pseudo-item thread. Assigning item numbers doesn't feel right to me - but if letting everything have item numbers will result in dramatic simplifications of code all over the place, it could be worth it. If we leave -1 as the single distinguished "not really an item" item number, we could make that a constant and compare against it. I wonder how many places there are in the code where we check "itemId <0" rather than "itemId == -1" to mean "not an item" - and how many of those really don't care, since they don't want to deal with pseudo items?

I trust you to do the right thing.

 

[Darzil]

I may do the right thing, eventually. I have no idea when. Life has got very busy these last few months.

I also noticed another issue crop up yesterday, though I don't care much. Ghost Dog has 4 in inventory according to Maximizer, as I have 4 Ghost Dog Chow.

 

[Veracity]

I'm not too concerned about sushi, at the moment, since they show up in the Item Manager food panel and can be consumed perfectly well from there. So, the issue is that you cannot use the "create" (or perhaps the "eat"?) command to make them.

However, since my new VYKEA companion creations are also pseudo items, this issue means I cannot "create level 5 couch", for example. So, I'll be looking at this tomorrow, I expect. Maybe even today; I don't need to actually create the item to test whether the create command will figure out what it is.

 

[Veracity]

Revision 16454 fixes ItemFinder for both Sushi and VYKEA Companions: rather than asking COncoctionDatabase to look up the mixing method via itemId, it uses the AdventureResult version, which incorporates both the item ID and the name and uses whicherver is appropriate.

 

[Bale]

Can this be done with an ash command? create(1, $item[level 5 couch]) ?

 

[Veracity]

No. The create() function turns into a "create" CLI command specifying the item via itemId using pilcrow notation. I could make it pass in the item name if the itemId is -1 but:

[color=green]> ash $item[ level 5 couch ][/color]

[color=red]Bad item value: "level 5 couch" ()[/color]
Returned: void

However, this works:

[color=green]> ash cli_execute( "create level 5 couch" )[/color]

[color=red]You can only build one VYKEA Companion a day.[/color]
Returned: false

 

[Theraze]

So the messy workaround would be to double-up and do something like:
main(string item_to_make) {
it = to_item(item_to_make);
print("We are "+(it == $item[none] ? "" : "NOT ")+"using CLI to create this time.");
if (it == $item[none]) cli_execute("create 1 "+item_to_make);
else create(1, it);
}

 

[Veracity]

If you insist on rolling your own function to create both real items and pseudo-items, I suppose you could do that. Considering that the only pseudo-items you need to "create" are sushi and VYKEA Companions, I don't see the benefit of trying to force them into such a function.

In any case, there is another Feature thread discussing pseudo-items. We could assign negative item numbers to all the pseudo-items which you might want to refer to but which cannot enter your inventory, which would allow ASH to treat them as "item" objects.

That's not a trivial change to KoLmafia, as you, personally, should know: I well remember the bug report you submitted a number of years ago that I spent a number of hours investigating until I eventually figured out that the code had to be passing around negative item numbers, at which point you admitted that you were running your own patched code which gave negative item numbers to sushi. Which is to say, you submitted a bug report about a problem with your own unreleased patched code.

 

[Theraze]

True. I'd submitted the patch for people to consider, but it didn't get looked at much because of how many other issues might be related to it - as you discovered related to that bug report. Sorry about that.

That bit of code was just to try to note a short example of how to do each. A longer check would do something like if the item is a real item, return the results of create. If it's not a real item, parse inventory and meat and, if the amount of anything changes, count it as a success. If everything - meat and items - stays static, then the creation attempt was probably a failure.

 

[xKiv]

It (the messy workaround) is also different in that it might fail for some values of item_to_make for which a simple cli_execute("create 1" + item_to_make) won't fail - because (ash) "to_item" just finds *any* item, whereas (cli) "create" filters only creatable items.

It's also different (and definitely worse) in that it will interpret commas (and maybe other characters?) differently, based entirely on whether item_to_make matches exactly one item.

 

[Theraze]

What comma bug?

> ash string testing = "staff of ed, almost"; to_item(testing).to_string();

Returned: Staff of Ed, almost

> ash string testing = "staff of ed almost"; to_item(testing).to_string();

Returned: Staff of Ed, almost

> ash string testing = "staff of ed, al"; to_item(testing).to_string();

Returned: Staff of Ed, almost

> ash string testing = "staff of ed al"; to_item(testing).to_string();

Returned: Staff of Ed, almost

That's why we use to_item on a string. If it matches, exactly or partially, to a single item, then we make it. If we fail to resolve to a single item, whether because of duplicate items or because it's a pseudo item, fail over to the cli create. As you can't easily test for cli results, we'd need to run it through a simple wrapper to snapshot our inventory and meat and see if those are still the same afterwards, to test our result.

As far as I'm aware, the only danger with to_item is the usual regarding pseudo-items...

> ash $item[ghost dog].to_string()

Changing "ghost dog" to "Ghost Dog Chow" would get rid of this message ()
Returned: Ghost Dog Chow

 

[xKiv]

Returned: Staff of Ed, almost[/code]That's why we use to_item on a string. If it matches, exactly or partially, to a single item, then we make it. If we fail to resolve to a single item, whether because of duplicate items or because it's a pseudo item, fail over to the cli create.

Which will then split on commas, and attempt to create several items.

 

[lostcalpolydude]

Which will then split on commas, and attempt to create several items.

A while ago I made mafia check to see if the entire argument matches an item before splitting on strings, so that shouldn't actually be an issue. Commas should only be an issue if you list multiple items at once.

 

[xKiv]

A while ago I made mafia check to see if the entire argument matches an item before splitting on strings, so that shouldn't actually be an issue. Commas should only be an issue if you list multiple items at once.

It probably wouldn't match an item in this case, because it would only get there if to_item(input) == $item[none].

 

[Theraze]

Just as a note - all of those examples I gave were matched as items. I used .to_string() to make it only output one line rather than the whole mess, but each of those has an item ID. If you hit a non-matched item that has commas, you might have issues. But that's on your input. Anything that gets matched with to_item will get manipulated through its item ID and commas are irrelevant.

So yes, if you give a non-specific creation request that matches !1 item but where your request has a comma in it, then you might end up with problems. But that's not a comma issue. That's a bad request issue.

 

[xKiv]

So yes, if you give a non-specific creation request that matches !1 item but where your request has a comma in it, then you might end up with problems. But that's not a comma issue. That's a bad request issue.

Then your code is not useful as much anything other than an example of what could technically be coded. It's no useful as something to use, because any caller must already know whether the input is a valid item or a valid argument to CLI create.
You can probably guarantee that now in your scripts.
And then you will forget about it, implement something that breaks that guarantee a year later, and post a "weird bug" report.