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

Add option to separate usage of incubator unbreakable from breakable (2) #4564

Merged
merged 14 commits into from
Aug 22, 2016
Merged

Add option to separate usage of incubator unbreakable from breakable (2) #4564

merged 14 commits into from
Aug 22, 2016

Conversation

supercourgette
Copy link
Contributor

Short Description:

  • The 2, 5 and 10km eggs can be dispatched between either the infinite or the breakables incubators.

I wasn't able to test this option in real-game, since well... My bot is sleeping. From the tests I ran, the option should behave as excepted.

Fixes/Resolves/Closes (please use correct syntax):

Not sure why it failed before (cf #4556)

@mention-bot
Copy link

@supercourgette, thanks for your PR! By analyzing the annotation information on this pull request, we identified @mjmadsen, @DeXtroTip and @TheSavior to be potential reviewers

@mjmadsen
Copy link
Contributor

CI passed this time. Woot.

@mjmadsen mjmadsen merged commit 75535c3 into PokemonGoF:dev Aug 22, 2016
@supercourgette supercourgette deleted the EggIncubate branch August 22, 2016 15:55
@alexyaoyang
Copy link
Contributor

This is a great feature! Just a quick question. Will the default values on line 27 and 28 of incubate_eggs.py break old configs which doesn't have the new fields?

@mjmadsen mjmadsen mentioned this pull request Aug 22, 2016
@mjmadsen
Copy link
Contributor

@alexyaoyang #4564 is a fix for that.

@supercourgette
Copy link
Contributor Author

It would have work without the fix though :)

On Aug 23, 2016 1:39 AM, "Matt J Madsen" [email protected] wrote:

@alexyaoyang #4564 is a fix for that.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@mjmadsen
Copy link
Contributor

@supercourgette From the looks of it, if they did not update their config, both would be empty when we try them.

@supercourgette
Copy link
Contributor Author

Oh, yes I see, It would not have been broken thought but I see a case I
didn't account for. If an user put an empty array then my code just ignore
the option (which now I think about it is not the indented behavior). I
don't have my computer right now, I'll check this case tomorrow. In the
mean time it is a bit broken in case of empty array. I'm sorry for the
inconvenience :(

On Aug 23, 2016 1:45 AM, "Matt J Madsen" [email protected] wrote:

@supercourgette From the looks of it, if they did not update their
config, both would be empty when we try them.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@alexyaoyang
Copy link
Contributor

Don't worry about it, I'll submit a PR :)

@supercourgette
Copy link
Contributor Author

Thanks :). And sorry for the mistake :)

On Aug 23, 2016 1:53 AM, "Alex Yao" [email protected] wrote:

Don't worry about it, I'll submit a PR :)


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#4564 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AD31cRPg1-3IVg-bT6zmamNkUXn5VME7ks5qidP9gaJpZM4JqANG
.

@alexyaoyang
Copy link
Contributor

No problem! Thank @mjmadsen too, he pushed & merged the fix already :)

@mjmadsen
Copy link
Contributor

I am getting drained. Either I'll fall asleep soon or have more coffee and keep rocking PRs.

@alexyaoyang
Copy link
Contributor

@mjmadsen Very productive stretch! Great job!

@supercourgette
Copy link
Contributor Author

Developers sleep? :P

On Aug 23, 2016 2:03 AM, "Matt J Madsen" [email protected] wrote:

I am getting drained. Either I'll fall asleep soon or have more coffee and
keep rocking PRs.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#4564 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AD31cQf6qGfTYZMrPC0CqSuhc3gE0n1vks5qidZigaJpZM4JqANG
.

@k4n30
Copy link
Contributor

k4n30 commented Aug 23, 2016

@supercourgette what about the other config files being update? Happy the doco got done though :)

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.

5 participants