Skip to content

fix for #2104 #2136

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.

Sign up for GitHub

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

Closed
bigkraig wants to merge 1 commit into from
Closed

fix for #2104 #2136

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pokemongo_bot/cell_workers/handle_soft_ban.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def work(self):
)

if fort_distance > Constants.MAX_DISTANCE_FORT_IS_REACHABLE:
MoveToFort(self.bot).work()
MoveToFort(self.bot, self.config).work()
Copy link
Copy Markdown
Contributor

@elicwhite elicwhite Jul 31, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If MoveToFort takes configuration, this will cause problems since it won't be getting the correct config values. self.config is specific to the task. This is why tasks shouldn't be calling other tasks.

@douglascamata Any ideas on a fix?

Copy link
Copy Markdown
Member

@douglascamata douglascamata Jul 31, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the config here needs to have the same structure of the config hash in JSON and SoftBan.config clearly won't have this structure.

Copy link
Copy Markdown
Contributor

@elicwhite elicwhite Jul 31, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, I'm not sure how we fix this though. HandleSoftBan doesn't (and shouldn't) know what the config is for MoveToFort. Is there any way we can refactor HandleSoftBan to not require anything about MoveToFort? Or do we just say screw it create the config that MoveToFort wants (ignoring however the user might have configured that task) and use it?

Copy link
Copy Markdown
Member

@douglascamata douglascamata Aug 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HandleSoftBan will decide how it wants to move to the fort.

Copy link
Copy Markdown

@SMWARREN SMWARREN Aug 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See Issue: #1522

I just replaced (self.bot, self.config) with (self, self.bot).

self.bot.recent_forts = self.bot.recent_forts[0:-1]
if forts[0]['id'] in self.bot.fort_timeouts:
del self.bot.fort_timeouts[forts[0]['id']]
Expand Down