SteamDatabase / BrowserExtension

💻 SteamDB's extension for Steam websites
https://steamdb.info/extension/
BSD 3-Clause "New" or "Revised" License
749 stars 57 forks source link

Attempt to get a sale item (sticker) after successful queue auto-discovery #184

Closed ge-ku closed 2 weeks ago

ge-ku commented 2 weeks ago

This should work. Item name will be displayed if it was gotten successfully. If for some reason there is no title for that item we'll show item ID (I don't think this should ever happen). Otherwise just show queue exploring result again with no mention of trying to get an item and log the reason of not getting it (HTTP error code or response with no item ID).

I'm also assuming that if there is data-store_user_config attribute of #application_config element, it contains valid JSON.

xPaw commented 2 weeks ago

The only problem is this will override the queue explored message. Maybe it needs to be appended as new element. Also errors don't seem to update the result message.

Do we know if this api accepts language param to localize the item name?

ge-ku commented 2 weeks ago

Do you mean the "Trying to claim a sale item if there is one…" message? It's being show for a split second when connection is fine. But then, when attempt is over, queue explored message is brought back along with info about new sticker (or nothing if no item was obtained).

Do you want to keep queue message at all times and a separate element for item messages? Right now if error occurs (http error or when we get response with no item ID) it shows previous message about explored queue and nothing about error (they are logged into console), do you want errors to be shown? So something like "No sale item was claimed." And HTTP code or "Perhaps you already claimed it today." message if response was OK but with no item ID?

Don't know about language param, I'll check when new item is available.

xPaw commented 2 weeks ago

Seems like there is https://steamapi.xpaw.me/#ISaleItemRewardsService/ClaimItem

As for the errors, it might be better to display the claiming item separately? Do we also need some retry loop?

ge-ku commented 2 weeks ago

Added separate element for claiming status, changed some styles to accommodate it. Added some retrying logic in case of HTTP errors (5 retries and uses same delay as you do for queue exploring). Now sets up language param as you linked.

Also on the page you linked I noticed CanClaimItem call, so now it starts by sending this request first and if can_claim is true we try to claim an item, otherwise give info that no item is available and show time of the next item. This call also uses the same retrying logic in case of HTTP errors.

I haven't tested it much, will try to use once a new item is available.

xPaw commented 2 weeks ago

Do we really need the can claim call? That's one extra req that can fail if steam is under load. If the direct claim call has no issues, just use it directly. I assume the can claim is for the countdown or something they use.

ge-ku commented 2 weeks ago

Do we really need the can claim call? That's one extra req that can fail if steam is under load. If the direct claim call has no issues, just use it directly. I assume the can claim is for the countdown or something they use.

I like it because ClaimItem returns an empty object when no item is claimed and we can only assume what it means while CanClaimItem shows us exact status (and time of the next item that we can show). I'll remove it you think it's not worth it.

ge-ku commented 2 weeks ago

Or maybe we can reverse it, start by claiming item and if it returns an empty object call CanClaimItem? This way it'll use one request in the best case scenario.

xPaw commented 2 weeks ago

Eh probably fine since you are showing the date. Can you reduce the amount of strings for this though? I don't think we really need all the attempts (I'll probably reduce them for explore too) so it's less stuff to translate.

Sorry I can't do a proper review on phone at the moment.

ge-ku commented 2 weeks ago

Sure, removed attempts messages and use one message for any of the 2 requests failing.

ge-ku commented 2 weeks ago

Done, I also tried to actually use it when item is available and it seems to work fine.

xPaw commented 2 weeks ago

I've merged it so people can start translating, I'll give the code a closer look later, thanks!