Goals for a new Inventory

BlueRaja commented 9 years ago

Alright, I think it's time to finally sit down and rewrite the inventory system. The current one has a lot of issues. Here's a list of goals I'd like to accomplish:

Consolidate Item and Inventory into single classes. No more of this "grabbing the description for an item" crap. No more separate Inventory classes for TF2 vs. other games. No more "Item may have a Schema.Item, Inventory.Item, or both, depending on circumstances".

Note that we may still have multiple classes for different Inventory/Item types, but they should all be derived from a common base class.
Inventory should work with "friends-only" inventories, and private inventories during a trade.
Bot should not block the trade while waiting for inventories to load. Probably the easiest way to make this happen is to not call OnTradeItemAdded until all Inventories have been loaded.
Inventory should work with more than 2500 items.
Inventory should be loaded using the correct URL (if that's even possible)
Should be compatible with StormReaper's TradeOffer PR, since a lot of people are using that. Add partnerInventory API support.
New: Should work with items that have amounts, like gems.
As an optional goal, should work with Spiral Knights (does anyone even care about this?)

Can anyone think of anything else?

This is going to be a major breaking change, but I think that's acceptable. This has been a long time coming.

Bottswana commented 9 years ago

What's the plan on this, sounds like a bit of work, who's going to stay implementing it? On 11 Oct 2014 15:38, "BlueRaja" notifications@github.com wrote:

Alright, I think it's time to finally sit down and rewrite the inventory system. The current one has a lot of issues. Here's a list of goals I'd like to accomplish:

Consolidate Item and Inventory into single classes. No more of this "grabbing the description for an item" crap. No more separate Inventory classes for TF2 vs. other games. No more "Item may have a Schema.Item, Inventory.Item, or both, depending on circumstances". Note that we may still have multiple classes for different Inventory/ Item types, but they should all be derived from a common base class.

Inventory should work with "friends-only" inventories https://github.com/Jessecar96/SteamBot/issues/639, and private inventories https://github.com/Jessecar96/SteamBot/issues/560 during a trade.

Bot should not block the trade while waiting for inventories to load. Probably the easiest way to make this happen is to not call OnTradeItemAdded until all Inventories have been loaded.

Inventory should work with more than 2500 items https://github.com/Jessecar96/SteamBot/issues/640.

Inventory should be loaded using the correct URL https://github.com/Jessecar96/SteamBot/issues/641 (if that's even possible)

Should be compatible with StormReaper's TradeOffer PR https://github.com/Jessecar96/SteamBot/pull/633, since a lot of people are using that.

As an optional goal, should work with Spiral Knights https://github.com/Jessecar96/SteamBot/issues/575 (does anyone even care about this?)

Can anyone think of anything else?

This is going to be a major breaking change, but I think that's acceptable. This has been a long time coming.

— Reply to this email directly or view it on GitHub https://github.com/Jessecar96/SteamBot/issues/642.

scholtzm commented 9 years ago

Another goal, add support for partnerinventory web interface which is used by Trade Offers.

For reference: /tradeoffer/<TRADE_OFFER_ID>/partnerinventory/?sessionid=<SESSION_ID>&partner=<STEAMID64>&appid=<HERP>&contextid=<DERP>

glen-84 commented 9 years ago

No more separate Inventory classes for TF2 vs. other games

Will it be possible to specify an appid in the settings file for each bot, which is used to determine which inventory to load?

BTW, it would be nice if releases were tagged, so that it's easy to merge changes without worrying about BC breaks.

scholtzm commented 9 years ago

@glen-84 appid and contextid will have to be specified, but most likely just in your code and not a settings file.

glen-84 commented 9 years ago

@scholtzm I was hoping to avoid code changes, like we currently do with replacing 440 with 570 in a bunch of places. This would also allow you to have multiple bots with inventories from different games (f.e. one Dota 2 bot and one CS:GO bot).

scholtzm commented 9 years ago

I have separate config for "my" stuff, you could probably do the same. Nonetheless it's too soon to tell how the things will go.

BlueRaja commented 9 years ago

This Inventory change is going to be a major breaking change, so you will definitely need some code changes regardless.

I think it makes more sense to specify which inventories to load in code rather than config files (it's something that varies by user-handler, not by bot-instance), but I'm open changing my mind.

WildCard65 commented 9 years ago

Can Generic Schemas be of use? Also for inventory loading, how about a call to the userhandler with a list that the userhandler can fill with appids of inventories to load.

BlueRaja commented 9 years ago

I'm not sure if Schemas are necessary, or if we can get everything from the inventory/json API

Bottswana commented 9 years ago

I thought Valve have been trying to kill the community api on steamcommunity.com for a while now. I don't know if that is relevant, but if it is, is it something to consider during this rewrite?

If the information is old my apologies, haven't worked on the inventory API since dabbling with a dota2 site a few years ago. Actually it might only be the XML based api on steamcommunity they have 'depreciated'.

WildCard65 commented 9 years ago

Well I started on a generic schema system.

On Sun, Oct 12, 2014 at 4:11 PM, Bottswana notifications@github.com wrote:

I thought Valve have been trying to kill the json api on steamcommunity.com for a while now. I don't know if that is relevant, but if it is, is it something to consider during this rewrite?

If the information is old my apologies, haven't worked on the inventory API since dabbling with a dota2 site a few years ago.

— Reply to this email directly or view it on GitHub https://github.com/Jessecar96/SteamBot/issues/642#issuecomment-58820503.

scholtzm commented 9 years ago

@Bottswana Right now regular trades and trade offers use the API provided by steamcommunity.com so imo it's the best to use that. Considering they have recently updated it (partner inventory in trade offers), I don't think they are going to kill it any time soon.

Bottswana commented 9 years ago

Yeah looks like the Inventory (json) API isn't on the hate list, only the ?xml=1 API's. For the reference, these are the ones they want to depreciate: https://partner.steamgames.com/documentation/community_data

scholtzm commented 9 years ago

Also one thing I noticed is type inconsistency. Sometimes, item IDs are declared as long, sometimes as ulong etc. Since we are doing breaking changes, this could be also looked into.

aleksamagicka commented 8 years ago

Any news? It's been almost a year.

crunchyintheory commented 7 years ago

I know this is around two years old, but I'd just like to ask if anyone has found a way to get a user's Spiral Knights inventory? I know you can find the context ids of the inventories manually but have we found a way to do it programamtically yet?

ghost commented 6 years ago

I dislike bumping old issues such as this. However, after watching the Steam API recently, I feel as though we need to have some of this pushed/updated as the API has become more and more unstable recently causing the bot to fail to login which if this was to be pushed, this would remove some of these issues.

Jessecar96 / SteamBot

Goals for a new Inventory #642