Decoupling and refactoring

[mjmadsen]

Our task order is currently static. Some of our tasks need to be performed on a state change. My thoughts that turn into ramblings:

• We should always try to collect level up rewards on exp change
• We should always check for badge responses (on each heartbeat?)
• Our API requests should be detached from bot decisions (store what we want locally, but only send recent when a response is required)
• Move emit calls to centralized place (store emit requests from each task). Allows for easier configuration, decouples task config from log config.
• Move delay times and sleep features to centralized place. Same as above, we repeat too many (similar kinds of) waits and make our config entirely too long.
• Too many parts of code that are trying to accomplish the same thing. Between catch_pokemon, pokemon_catch_worker, map_move_to_pokemon we repeat much of the code. Plus map_move_to_pokemon changes position and initiates capture attempts (and checks balls). All capture attempts should be force through our catch_pokemon task first (with refactor to implement order logic of map_move_to_pokemon). pokemon_catch_work should be the only one checking balls, our walker should be the entry point for all changes to position (with etc logic changes).

I'll stop for now. My idea is, some files do too much that other files should and some files just too much in general.

[Gobberwart]

Re api calls.. In an ideal universe the bot would only call the api when absolutely necessary, ie. To perform an action for the most part. As far as things like player stats and inventory, the bot should grab that info at start, store it in a database, then only refer to the database for future info. Sanity check every so often just to make sure nothing is wrong.

Just my ramble

[duttonw]

We should always check for badge responses (on each heartbeat?) check awarded badges is currently in the heartbeat.

My PR #4746 stored the awarded badge in the bot, and also updated the player_data on every heartbeat. so we could remove the get player_data api calls from some tasks if last 10 second (give or take delays) is good enough.

[mjmadsen]

@Gobberwart @duttonw I love it.

[sohje]

I make some debug over api calls. There is a lot of calls like _get_player/getinventory one after another without delay or delay <= 1s. Looks like:

bot_task()
-> function get_inventory at 0x107a1bc80
-> function get_inventory at 0x107a1bc80
-> function get_player at 0x107a1bc08
-> function get_inventory at 0x107a1bc80
bot_task()
-> function get_player at 0x107a1bc08
-> function get_inventory at 0x107a1bc80
-> function get_inventory at 0x107a1bc80
-> function get_inventory at 0x107a1bc80

Do we really need this calls every time on bot_task switch?

[sohje]

Ok, some logs here:

[2016-08-29 13:51:17] [MoveToFort] 
2016-08-29 13:51:18 (<function get_inventory at 0x10c9e5c80>)
2016-08-29 13:51:18 (<function get_inventory at 0x10c9e5c80>)
2016-08-29 13:51:18 (<function get_inventory at 0x10c9e5c80>)
2016-08-29 13:51:18 (<function get_inventory at 0x10c9e5c80>)
[2016-08-29 13:51:18] [UpdateLiveStats]

[duttonw]

get_player should really be removed from tasks and pull from the bot as the heartbeat keeps it up to date every 10 seconds. (other than the tutorial task).

tasks calling get_inventory should be using the centralized inventory which should only refresh every so often. This should be ready to go forward since recycle now updates internal inventory.

On 29 August 2016 at 20:53, Nikolay Spiridonov notifications@github.com wrote:

Ok, some logs here:

[2016-08-29 13:51:17] [MoveToFort] 2016-08-29 13:51:18 (<function get_inventory at 0x10c9e5c80>) 2016-08-29 13:51:18 (<function get_inventory at 0x10c9e5c80>) 2016-08-29 13:51:18 (<function get_inventory at 0x10c9e5c80>) 2016-08-29 13:51:18 (<function get_inventory at 0x10c9e5c80>) [2016-08-29 13:51:18] [UpdateLiveStats]

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/PokemonGoF/PokemonGo-Bot/issues/4754#issuecomment-243093276, or mute the thread https://github.com/notifications/unsubscribe-auth/AB7XcARCNXD-T2G49I7JHoFYpdhjhpHtks5qkro-gaJpZM4Jtx9x .

[sohje]

Seems we have to refactor all api.get_inventory() calls to refresh_inventory() with decorated cache object. But what TTL we should use for api call?

[Gobberwart]

I've just submitted a PR which goes some way towards refactoring to reduce api calls. Metrics and UpdateLiveStats now share the same cached inventory information (note that "inventory" from an API perspective includes player_stats so they're now all kept together in inventory)

PR #4916

[Gobberwart]

Removed another api call from incubate_eggs. As far as I can see, GET_INVENTORY is now only called as part of inventory_refresh, which is called by heartbeat. Everything else should just use whatever is cached.

[k4n30]

@mjmadsen close?