PriceAdvisor: Maximize your profits

You may want to see my implementation of quantities, but I'm not quite there yet.

I present for discussion a beginning of a refactor for PA, in the hopes that those who are scripting with it can tell me if this is the sort of thing they are looking for... or if I'm completely off base. I'm not rewriting advice for concoctions without knowing the base representations are better this time!

Looking at the to_executable_string() or to_readable_string() methods are probably good indications of how I am using the data in a pa_advice to construct everything that used to just be its action member. Having all that information broken out instead of stuck in a string makes a few things more complicated, but I hope it will make most others much easier since it gets rid of string parsing.

PARefactor deals only with autosell, mallsell, and smashing (not malusing, or concoctions, or using, or anything else). Quantity does not yet come into play, but since smashing does include expected results style output you can see what that will look like.

This will also give you a heads-up on many nomenclature changes. I don't actually delight in breaking scripts that use PA, but since I was changing the basic data structure anyway I decided to change a bunch of other stuff that I'd thought better of.

An easy way to visualize the executable-style output is to change print(pa_advice a) (line 284) to call to_executable_string() rather than to_readable_string().

Suggested alias for testing things:
alias par => ashq import <PARefactor.ash> print(all_advice($items[%%], true))

Re: quantities:

I propose that a basic pa_advice does not have a quantity -- i.e. has a quantity of 1 -- although it does store the quantity of its results. This is actually fairly necessary, as it enables caching... and as I discussed in my post about 1.6, PA really really needs its cache. (Okay, this early PARefactor doesn't, because it can't recurse very deeply, but eventually...)

I agree, however, that as much as possible of the mallbot-like silly behavior should be dealt with. Towards this aim, the functions that construct strings from pa_advice accept a quantity argument (although you'll notice they don't have any anti-mallbot-like behavior in them yet). Thus, the pa_advice will be constructed as an abstract, and when it is put into implementable form it will become smarter depending on the quantity it now knows about.

(Notify and version checking have been commented out in PARefactor. Anyone is welcome to download this and tell me what they think of it, but it is not a new version of PA yet!)

Is the new version of SmashLib foreshadowing anything that will happen with this script? (Took a bit of scanning through the code to figure out what the change was... heh. You added... a notify and quantities?)

Back to the subject at hand: I like how in PARefactor you renamed price_advice to pa_advice (Less typing? What's next, pa_adv?). On a more serious note, thanks for separating readable and executable output. (I'm assuming that the former is CLI-executable, and the latter is ASH-executable. Correct me if I'm horribly wrong.)

On a slightly related note:
From your todo list:

in to_readable_string(), deal with expected results (0 quantities) with better output than 0

Click to expand...

Couldn't you add if(quantity!=0) in the front of line 249 (read = read + a.action + " " + quantity + " " + a.it;)?

Well, the update to SmashLib was just to add a wrapper function "smash" so I can use "smash" both as a readable description and a function name. It seemed like something that belonged in SmashLib (given that it's a library of smashy functions) rather than PA, so I put it in there. (There is no extant ASH function smash() or pulverize().)

The readable advice is just supposed to be readable, by people (hence, I want to replace 0 with something like "as many as you got", which is what it actually means). It is not fully CLI-executable. The executable advice is supposed to be ASH executable. I guess I could add a CLI executable output style too?

Re: price_advice -> pa_advice. Yeah, less typing. And for a bit I thought I'd rename price_advisor() to price_advice(), but then I renamed it all_advice() instead. Rename everything! EVERYTHING! MWAHAHAHA!

Ahem.

pretium_consuasor.ash ?

aqualectrix said:

The readable advice is just supposed to be readable, by people (hence, I want to replace 0 with something like "as many as you got", which is what it actually means). It is not fully CLI-executable. The executable advice is supposed to be ASH executable. I guess I could add a CLI executable output style too?

Click to expand...

Mmft, then the bit of code for replacing 0 in ASH would be... exe = "mallsell(item_amount(a.it)-"+item_amount(a.it)+",a.it);"

Or, at least, that's the approach I'd take.

Oh, and regarding renaming: make one ash_adv, and the other cli_adv? Or maybe pa_ash and pa_cli?

Yes, the code for replacing 0 in to_executable_string() is there already.

I'm confused about your renaming suggestions (maybe we're still joking? I can't tell.).

I take it you think there should be a to_cli_string() and to_ash_string() instead of just to_executable_string() (which is what to_ash_string() would be)?

Does anyone want a to_cli_string()? In addition to to_executable_string() / to_ash_string()?

Is to_executable_string() / to_ash_string() actually what people want when they say they want ash output?

Is the new pa_advice record manipulable enough? Is it better than the string-parsing you currently have to do with PA?

These are the things I want to know -- am I completely off base, or is this going in a good direction?

Yeah, I was still joking about the renaming.

I personally don't really care if there's a to_cli_string; the way I see it, it should be easier to create CLI output -- PA already had that, so you could just cannibalize that. But the primary reason I suggested leaving that in there was so that it wouldn't break pre-existing scripts too horribly.

Huh. Does the whole negative integer translate into "all but x" in ASH, as well as CLI? (I'm assuming that I've found the code for replacing 0, and that I'm reading it properly -- probably shouldn't ever be operating under those assumptions. Heh.)

The string parsing was a pain and didn't always work properly, so just about anything's better. It was a big nasty cluster of index_of, matchers, and substrings.

It's always going in a good direction -- forward is good, right? Well, better than backward, at least. Except in the case when you're rolling back to a previous version to clear up some newly created bugs.

Yes, negative integers mean "all but x" in ash as well as cli. It would be awfully confusing if the two differed!

(Which is to say, if more-experienced ash coders know of any place this isn't true, please let me know...)

Actually, no - as far as I can tell, every ASH function that takes a count parameter will do nothing at all if given a zero or negative value.

So I guess you'd have to replace the offending line with the string concatenation that I detailed above here (Seems like I sure do love my string concatenations! Either that, or I need to find some new way of doing things.)

Oh, and that's another reason to preserve the CLI-executable output. The replace_zero problem is solved by just tossing in an
exe = "mallsell -"+item_amount(a.it)+" "+a.it;
somewhere. And several exceptions if you want the output to look prettier (i.e. instead of -0, just 0 or *).

Huh. This makes me sad. First, because it doesn't work as I'd like, and second, because I asserted that it did without actually checking. Which was pretty dumb. Thanks for catching this before I built too much on it, Jason!

I stand by my statement that the difference in behavior is odd. I think it would be useful for ash to behave in the same paradigm as cli. I'll file a feature request, and then at least I'll learn why it isn't so.
Otherwise I will indeed just make some wrapper functions that use the cli version of the commands.

Here's a weird suggestion

heavy D:
autosell 1 heavy D: 135.0 meat
acquire 1 lowercase N; make 1 ND; untinker ND; autosell 1 heavy D; acquire 1 original G; make 1 NG; acquire 1 WA; make 1 wang; mallsell 1 wang: 135.0 meat

Sneaky, sneaky PA!

Halfway-done-rewritten-PA has better silly-result checking. I hope it will catch more of these silly results when I get time to finish it off and polish it up.

aqua! Here's hoping you have some time to polish this off!

And, here's a suggestion or two to help delay you further:

During the IsleWar, there may be a more profitable option for certain Battlefield items than autosell/pulverize: the camp shops, where you can buy/sell items for quarters/dimes, which can in turn be used to buy different items. I've recently modified OCW to only turn in items for coins if the resultant coins are more profitable than the item's mall/autosell value, so it got me thinking that PA should also consider this.

For items that can be turned in for coins, each coin is worth at least 200 meat (you can buy an item for 5 coins at both camps that autosells for 1000 meat), or in certain unpredictable cases it may be worth more to turn in an item for coins and then use those coins to buy a different item. This would involve a couple more maps and a good bit more price checking, but would make this script just that much more awesome. I can attest personally that if PA did this, it would impress the crap out of me. In the words of UR, I would have to go find some more crap.

Once you get that working, then you'll have to add special handling for sand dollars. Then, perhaps it will be time to write something that auto-detects the Traveling Trader...

Have I mentioned I love this script?

Hello, sorry to be clueless. I installed this script (and zlib, and smashlib and the latest KoLmafia exe). I set up the alias command ("pa => ash import <PriceAdvisor.ash> print(price_advisor($items[%%], true))"). When I type, say, "pa cog" into the CLI, what I get back is


It just says Returned: void for everything. Am I missing something? I'm pretty new to .ash files. Thanks for any help!

I get:

(price advisor doesn't give more advicethan that for NPC items)

Interesting. I got the same advice as laplume:

Edit: Running the same alias without any argument after it leads to an entire long list of stuff that says nothing followed by a "Returned: void"

What's your zlib verbosity setting? It has to be 1 or more for the "autosell 1 cog: 10.0 meat" message to be printed (you can check by doing zlib vars in the gCLI).

And you guys are running v1.62 or PriceAdvisor, right?

You probably want "ashq" instead of "ash" in that alias to get rid of the Returned: void line.

slyz said:

What's your zlib verbosity setting? It has to be 1 or more for the "autosell 1 cog: 10.0 meat" message to be printed (you can check by doing zlib vars in the gCLI).

Click to expand...

I set the zlib verbosity to 1, and PA works like a charm now. Thanks a lot!