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

'all' in 'release' config discards all pokemon except N best (Closes #4552) #4615

Merged
merged 2 commits into from
Aug 23, 2016

Conversation

LitRidl
Copy link
Contributor

@LitRidl LitRidl commented Aug 23, 2016

Closes #4552 , Resolves #4468

Added keyword 'all' in 'release' config which allows to discards all pokemon in bag except N best. Updated docs and added commented examples in configs.

  • "release": {"all": keep_best_cp": 100} globally keeps 100 best CP pokemon;
  • fixed description of 'any' in docs and configs;
  • added 'all' example in config files;
  • added example of 'all' to docs;
  • accidentally replaced some tabs in configs to spaces (but hey, that's good - spaces are everywhere in the project).

Changes in code are exactly as described in #4552 solution suggestion (refactored contents of for in a separate method and added 3 new lines, that's all).

…N best ones

* "release": {"all": keep_best_cp": 100} globally keeps 100 best CP pokemon
* fixed description of 'any' in docs and configs
* added 'all' example in config files
* added example of 'all' to docs
@mention-bot
Copy link

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

@supercourgette
Copy link
Contributor

Thanks for your commit. :)

English is not my first language, and I have some difficulty to understand the meaning of your PR (and your proposition in #4552). From my understanding, the actual configuration already allow to discard any Pokemons which does not met customs criteria, to only keep the N best.

In the documentation you said:

+```"release": {"any": {"keep_best_cp": 2}}```, ```"release": {"any": {"keep_best_cp": 10}}``` - can be any number. In the latter case for every pokemon type bot will keep no more that 10 best CP pokemon.
+
+If you wish to limit your pokemon bag as a whole, not type-wise, use `all`:
+```"release": {"all": {"keep_best_cp": 200}}```. In this case bot looks for 200 best CP pokemon in bag independently of their type. For example, if you have 150 Snorlax with 1500 CP and 100 Pidgeys with CP 100, bot will keep 150 Snorlax and 50 Pidgeys for a total of 200 best CP pokemon. 

For your example with the Snorlax, wouldn't the same output will be done with:

release": {"any": {"keep_best_cp": 200}} ?

Thanks you in advance for your explanation. :)

@LitRidl
Copy link
Contributor Author

LitRidl commented Aug 23, 2016

@supercourgette well, you think exactly as I did before I got into this problem. I though that keep with 'any' works by keeping N best pokemon in bag. But sadly, it doesn't: it tries to replicate deduplication mechanism from Necrobot. For example, if you have 50 pidgeys, 50 rattatas and 50 weedles and run them through "any": { "keep_best_cp": 10 }, you will end up having 10 best pidgeys, 10 best rattatas and 10 best weedle.
If we use new 'all' instead of 'any' in this example we will have 10 global best pokemon no matter of what type. Maybe it would be 5 rattatas, 1 pidgey and 4 weedles -- who knows. Just 10 strongest in total, no more.

_release_pokemon_get_groups(...) called in TransferPokemon.work creates groups of duplicate pokemon (== pokemon of 1 type) and returns it. Filtering logic in for is applied always to 1 group. So, 'any' works by deleting worst duplicate pokemon and keeping only N best ones.

With all things are different: there is only 1 big group of all pokemon which gets filtered as a whole.

@solderzzc
Copy link
Contributor

This branch has conflicts that must be resolved
OOPs.

@LitRidl
Copy link
Contributor Author

LitRidl commented Aug 23, 2016

@solderzzc And what to do? When I published PR there were no conflict. Few hours later -- well, of course they happened to be ) If I will create new PR, time delays will be the same. So conflicts will happen again?

@solderzzc
Copy link
Contributor

@LitRidl Since there're several PRs I merged before yours. You can just on your branch, run git fetch, git merge origin/dev, then solve the conflict.

@LitRidl
Copy link
Contributor Author

LitRidl commented Aug 23, 2016

@solderzzc fixed

@solderzzc
Copy link
Contributor

Hmm, I haven't read the code carefully. The release task is out date since PokemonOptimizer did much better on XP.

@mjmadsen
Copy link
Contributor

"any" is releasing all but the top as a whole. @solderzzc @LitRidl #4621

@LitRidl
Copy link
Contributor Author

LitRidl commented Aug 24, 2016

@mjmadsen Strange, how does it come that I have broken 'any'? Sorry, going to look for this

@mjmadsen
Copy link
Contributor

@LitRidl Yeah, I haven't found it yet.

@LitRidl
Copy link
Contributor Author

LitRidl commented Aug 24, 2016

@mjmadsen I have found it -- if 'all' is not present in release config, it falls into 'any'. 'any' logics is organised as default value: if pokemon name is not found in release config, bot gets 'any'. And well, if someone haven't specified 'all'... 'any' runs on the whole pokemon bag :)
Will fix it in 5 minutes, should I open new PR?

@mjmadsen
Copy link
Contributor

mjmadsen commented Aug 24, 2016

Yes please. Nice work!

My self included, we had a few folks lose most of their bags. Doh!

@LitRidl
Copy link
Contributor Author

LitRidl commented Aug 24, 2016

I am really sorry. Should have tested it better.
I have created PR with fix, please check it. Don't want to break anything again

@mjmadsen
Copy link
Contributor

No problem. It happens!

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