Feature - Implemented boolean reprice_shop( price, item ) to change the price of an item already on sale

Currently, "repricing" an item in my shop actually requires removing it, then re-adding it at the new price.

Code:
int shopAmount = shop_amount(item);
int newPrice = 12345;
take_shop(item);
put_shop(newPrice, 0, shopAmount, item);

It'd be lovely if this could be accomplished with one command. Easier for the scripter, and friendlier in terms of server hits.

So, requesting:
boolean reprice_shop( int price, int limit, item it )
or the overloaded boolean reprice_shop( int price, item it )
returns true if repricing succeeds;
returns false if repricing fails (for instance, if the item isn't currently stocked).
 
Last edited:

Hellno

Member
I'll throw my support at this suggestion, I have a script that does repricing the 'hard' way, but never felt good about having to do that.
 

Veracity

Developer
Staff member
Or perhaps we automatically remove limits? ;)

Seriously, if there are many things we can change for an item currently in your store with the same server hit, we should let you change them all with a single server hit.
 

heeheehee

Developer
Staff member
Thoughts regarding something like boolean update_shop_data(item it, int limit, int price) ?

The relevant request looks to be something like backoffice.php?price%5B5246%5D=9%2C100&limit%5B5246%5D=3&ajax=1&action=updateinv&pwd (unencoded, it's backoffice.php?price[5246]=9,100&limit[5246]=3&ajax=1&action=updateinv&pwd )

This looks like something that could be batch processed for multiple items at a time.

edit: comment in the response source code when I tried this out on two items at a time:
HTML:
<!-- U:{"603567625":{"price":9100,"lim":0},"213065981":{"price":7500,"lim":0}} -->

Only items that have their metadata actually updated show up in that comment.
 
Last edited:
Sure, including a per-day limit update sounds fine to me. I would personally vote for a three-parameter function as well as an overloaded two-parameter function that does not change the per-day limit, but I could live with just the more complex function.

One small note: ordering the parameters as ( item first, other parameters last ) makes intuitive sense to me, but the other multiple-parameter functions that deal with the shop (eg. put_shop()) list the item parameter last. So, solely for consistency's sake, that might be a better ordering.
 

heeheehee

Developer
Staff member
The other functions also specify quantity first (put_shop, etc; presumably for consistency with use(int, item) to mimic the CLI "use X item"). There is no quantity parameter involved with repricing, so that seems fine to me.
 

fronobulax

Developer
Staff member
I have a vague recollection this was previously considered and rejected in the past. The reason for doing so was that KoL did not support a single call that would be doing repricing so a KoLmafia implementation would be the same as the ash implementation, take everything out and then put it all back. Kol's implementation of "backoffice" addresses that so this now seems like a pretty good idea.
 

Ryo_Sangnoir

Developer
Staff member
I was also interested in this feature, so I've whipped up a patch that provides it, attached. The patch provides the ash function specified in the first post (reprice_shop(price, limit, item) and reprice_shop(price, item)) and a CLI command "shop reprice <item> @ price [limit <num>] [, <another>]".

I put the associated CLI command in as "shop reprice", because I felt that having "reprice" undercut all max-priced items that weren't at mall minimum and "reprice <item> @ <price>" reprice a single item at a given price made for some strange semantics. I tried to hew as closely as possible to the way things were done in other similar functions.

This patch includes a bugfix for ManageStoreRequest that lead to items almost always not being skipped in the request, even if they were unchanged. I also modified the shop CLI help text which indicated that the @ character was optional when providing a price, but it doesn't seem to be.
 

Attachments

  • reprice_command.txt
    6.8 KB · Views: 54

heeheehee

Developer
Staff member
Bugfix looks good to me; I'm surprised nobody reported it in the 4 months it's been active.

I haven't stepped through the rest of the patch in detail yet (although I'm wondering why we still have utilities.IntegerArray -- it's clearly a holdover from when we didn't want to migrate the whole codebase to Java 1.5 and use generics, but... there are now generics used in that same file).
 

heeheehee

Developer
Staff member
Looks like IntegerArray exists so we can have an int[] toArray() rather than the Integer[] toArray() of an ArrayList<Integer>.

Why do we (and consequently, you) cast the return value (int[]) of IntegerArray.toArray() to int[]? Seems redundant these days.

Other than various quirks that are already present in our codebase, stylistically, the patch looks fine (and probably does what it's supposed to -- I haven't gotten around to applying and testing).
 

Veracity

Developer
Staff member
I'll be working next on this. I'm curious to see the bug in ManageStoreRequest, since I was working in that file today. :)
 

Veracity

Developer
Staff member
Yeah, OK. I tweaked the code and related code and submitted it in revision 17673.

1) I didn't understand why the item was at the end of the price and optional limit in the ASH function. Then I saw that put_shop did that too. Confusing API,in my opinion, but, it is correct for reprice_shop to follow the lead of put_shop.

2) Item names can contain commas, so I thought the "split" on commas in the "shop reprice" command was dubious - and then I saw that you took the code for parsing the command straight from "shop put". I tweaked both of those, but otherwise, all is well.

Thanks for the patch.
 
I've been using this since it was implemented, and just realized I never came back to say (1) it works great, and (2) thanks!

So... both of those things! I love this feature.
 
Top