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

Doubles support #51

Closed
wants to merge 81 commits into from
Closed

Doubles support #51

wants to merge 81 commits into from

Conversation

szymonWojdat
Copy link
Contributor

@szymonWojdat szymonWojdat commented May 20, 2020

Closes #49

Drafted this real quick for now, will keep working on this in the following days.

For now the plan was to create AbstractBattle, use DoubleBattle for doubles and Battle for singles, move all common logic from Battle to AbstractBattle and implement the missing functionalities of DoubleBattle.

@codecov
Copy link

codecov bot commented May 20, 2020

Codecov Report

Merging #51 into master will increase coverage by 3.88%.
The diff coverage is 84.12%.

@@            Coverage Diff             @@
##           master      #51      +/-   ##
==========================================
+ Coverage   81.34%   85.22%   +3.88%     
==========================================
  Files          31       33       +2     
  Lines        2455     2809     +354     
==========================================
+ Hits         1997     2394     +397     
+ Misses        458      415      -43     

@hsahovic hsahovic added the enhancement New feature or request label May 20, 2020
@hsahovic hsahovic added this to In progress in Doubles support via automation May 20, 2020
@hsahovic hsahovic mentioned this pull request May 21, 2020
@szymonWojdat szymonWojdat changed the title Create a draft of AbstractBattle Doubles support May 24, 2020
@szymonWojdat
Copy link
Contributor Author

I think most of the work on DoubleBattle is complete, stuff that's left:

  • isolating a DoublesPlayer class as I think it's the cleanest way of integrating DoubleBattle with Player. Similarly to DoubleBattle, it will inherit from an abstract class, Player class will do so too
  • implementing DoublesPlayer.choose_random_move()
  • writing a ton of unit tests for doubles
  • creating some simple baselines for doubles (?)

@szymonWojdat
Copy link
Contributor Author

Also I don't quite understand why is flake8 (line length over 88) failing for double_battle.py while it clearly should be failing for other files as well (eg. battle.py).

@hsahovic
Copy link
Owner

hsahovic commented Jun 4, 2020

Good job! Are you using black? I suspect it might solve most of these issues (apart from docstrings, which might need to be manually reformatted).

@szymonWojdat
Copy link
Contributor Author

@hsahovic thanks for the advice, gonna use it from now on, for some reason I've never noticed that max line length was so small.

Just fixed all tests and stuff, going to work on DoublesPlayer from tomorrow. Mind taking a look at the last step that keeps failing? Looks like there's some issue on the CI machine, does it not reset its state after each run?

@hsahovic
Copy link
Owner

hsahovic commented Jun 4, 2020

Oh, I think that I know what the problem is. I updated the CI process on another branch last week, and I think that my first updates might have used the same cache name - which was an error on my end. I think that merging the current state of master into this branch should solve the issue.

On the subject of tests, you should also install pre-commit: it will help you catch this kind of errors before committing your work.

@dmizr
Copy link
Contributor

dmizr commented Aug 26, 2020

Hey @hsahovic @szymonWojdat, I noticed some effects that occur in double battles are absent from the src/poke-env/environment/effect.py file.
Here's a version of the Effect class with several extra effects that can happen in battles: https://gist.github.com/dmizr/1f1d6911ea4cc20203dbd0d73f417f49

@szymonWojdat
Copy link
Contributor Author

@hsahovic i have no idea why flake is failing, should i reformat docstrings in excetptions.py so that they're even shorter?

@hsahovic
Copy link
Owner

I'll check it out tonight.

@hsahovic hsahovic closed this Sep 22, 2020
Doubles support automation moved this from In progress to Done Sep 22, 2020
@hsahovic hsahovic mentioned this pull request Sep 22, 2020
@szymonWojdat szymonWojdat deleted the doubles-support branch November 6, 2020 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Doubles support
3 participants