Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added caching of fort timeouts similar to caching recent forts. #3897

Closed
wants to merge 2 commits into from

Conversation

Quantra
Copy link
Contributor

@Quantra Quantra commented Aug 14, 2016

Short Description:

Caching of fort timeouts similar to how recent_forts are cached and with option to disable this feature.

@douglascamata please review =]

Fixes (provide links to github issues if you can):

-Prevents bot from trying to spin fort still on cool down after restarting bot

@julienlavergne
Copy link
Contributor

Look like it could replace entirely the recent fort cache. right ?

@Quantra
Copy link
Contributor Author

Quantra commented Aug 14, 2016

@anakin5 they perform slightly different tasks. fort_timeouts are all forts you have on cooldown. recent_forts is a list of forts you just been to that you don't want to revisit with user configurable length.

So caching fort_timeouts stops that little bug when you restart next to a fort you just spun.

Caching recent_forts means if a user has a long recent_forts list and restarts their bot regularly then that list isn't started over every time. It ensures the max_circle_size feature works as intended across restarts.

@julienlavergne
Copy link
Contributor

hum, OK but you get the timeout of fort when getting the cell info from the API. So why do you need to keep them in a cache ?
If the spin_fort is spinning a fort on cool down, it is because it is not checking the timeout.

@Quantra
Copy link
Contributor Author

Quantra commented Aug 14, 2016

@anakin5 if you find the answer please let me know! I've been scratching my head on this one....

recent_forts if used by MoveToFort not SpinFort so let's ignore it for now.

When SpinFort works the first thing it does is call self.get_forts_in_range()

    def get_forts_in_range(self):
        forts = self.bot.get_forts(order_by_distance=True)

        for fort in forts:
            if 'cooldown_complete_timestamp_ms' in fort:
                self.bot.fort_timeouts[fort["id"]] = fort['cooldown_complete_timestamp_ms']
                forts.remove(fort)

        forts = filter(lambda fort: fort["id"] not in self.bot.fort_timeouts, forts)
        forts = filter(lambda fort: distance(
            self.bot.position[0],
            self.bot.position[1],
            fort['latitude'],
            fort['longitude']
        ) <= Constants.MAX_DISTANCE_FORT_IS_REACHABLE, forts)

        return forts

That first for loop looks like it throws out anything with a timeout to me but it doesn't work right after a restart.

So my ideas...

Maybe that for loop is badly coded and forts should be cloned before looping... Pretty sure it's NOT ok to be removing things from what you are iterating over whilst you are iterating over it. I could be wrong though.

      for fort in forts.copy():
            if 'cooldown_complete_timestamp_ms' in fort:
                self.bot.fort_timeouts[fort["id"]] = fort['cooldown_complete_timestamp_ms']
                forts.remove(fort)

OR We're not storing the timeouts data correctly.

OR The server isn't returning the timeouts right after relogging.

@julienlavergne
Copy link
Contributor

The server is returning the timeout, but at the start of the bot, the fort_timeouts list is not filled.
The fort_timeouts list is only filled after spinning a fort, that is why you have an issue when restarting the bot.

The first part of the solution, I think, is to fill the fort_timeouts list when the bot start. Which should be easy thing since we get the list of forts in the get_meta_cell function.

After done with that, this loop in spin_forts can be removed because there will never be any fort on timeout that is not in the fort_timeouts list.

Last thing, once the 2 previous thing are done, is to fix the behavior when the inventory is full. When spinning a fort and getting the inventory full response, that fort is not added to the fort_timeout list. This cause the bot to loop over forts when inventory is full.

@Quantra
Copy link
Contributor Author

Quantra commented Aug 14, 2016

I've not looked too closely at what requests the bot makes at start up but if you can find where it gets that data and pop it in fort_timeouts it sounds like it would make things cleaner.

Cacheing the timeouts on exit shouldn't be necessary but I'll leave this PR open for the discussion.

I'll test the forloop fix if i get some time later even if it's just a stop gap fix at least it's a 1 line stop gap fix ;)

@TheSavior
Copy link
Contributor

@Quantra I'm going to close this to reduce clutter, but feel free to open a new PR when you guys are happy with the approach.

We calculate the meta cell here, perhaps this is what you are looking for: https://github.com/PokemonGoF/PokemonGo-Bot/blob/master/pokemongo_bot/__init__.py#L485

@TheSavior TheSavior closed this Aug 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants