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

Gen 8 support #26

Closed
wants to merge 82 commits into from
Closed

Gen 8 support #26

wants to merge 82 commits into from

Conversation

szymonWojdat
Copy link
Contributor

@szymonWojdat szymonWojdat commented Mar 25, 2020

As discussed in #20

Copying over a checklist by @hsahovic that he mentioned here in order to keep track of what's left

Things to update:

  • src/data/pokedex.json
  • src/data/moves.json
  • src/environment/Battle.battle_parse_message
  • src/environment/Battle.battle_parse_request
  • src/player/Player.choose_random_move

Things to add:

  • src/environment/Battle.can_dynamax
  • src/environment/Pokemon._dynamax

Things to add if necessary:

  • src/environment/Battle.can_gigantamax
  • src/environment/Pokemon._gigantamax
  • src/environment/Pokemon.is_dynamaxed
  • src/environment/Pokemon.is_gigantamaxed

hsahovic and others added 30 commits February 4, 2020 14:27
- should be stored now takes into account z moves correctly
- is_z detect z versions of status moves
Fix create battle and get battle methods in player
Add warning for username and login issues
Bumps [pyre-check](https://github.com/facebook/pyre-check) from 0.0.41 to 0.0.43.
- [Release notes](https://github.com/facebook/pyre-check/releases)
- [Commits](facebook/pyre-check@v0.0.41...v0.0.43)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
- Ignore error [45] in Enumeration classes as it arises from the
behavior of the standard library's Enum objects
- Add default values for numerous attributes
This allows more flexibility in defining species on pokemon creation,
which solves problems related to alternate forms
0.0.43 is not available with python 3.7.
@szymonWojdat
Copy link
Contributor Author

Alright, now that I think about it, this information (can_dynamax) actually belongs in the Battle state because it tells whether it is possible to dynamax or not, while Pokemon instance only knows if it's currently dynamaxed or not. When it comes to available_moves I think I'll just keep it simple for now. I was considering adding it to Battle state to have it dynamically change between max moves, regular moves and both depending on the dynamax state of the Battle (which would also make Player.choose_random_move() a bit simpler. But for now I think it's going to be the best to do it in a similar way to max moves/megas and maybe optimize later on. Of course I won't be making such changes without discussing them with you first.

@szymonWojdat
Copy link
Contributor Author

I just managed to rerun tests locally, adjusted a few methods, nothing too big, just pushed out the changes. A couple of issues:

  • Move.can_z_move doesn't work as expected as moves.json no longer contains zMovePower for all damage moves, my guess is that PS has switched to some sort of dynamic calculation of the z-move power. The only place where it's used in code is at Pokemon.available_z_moves(). I assume the easiest workaround would be to modify Move.can_z_move so that it checks if given moves appears on a small list of moves that cannot z-move (test_can_z_move() implies as if Metronome could not z-move but it actually can, it just doesn't have any extra effect, moves that can't z-moves would be eg. Struggle)
  • for the same reason as mentioned above, test_z_move_power is failing as well. I'll try taking a look at their codebase to see what's the formula that they're using

@hsahovic
Copy link
Owner

@szymonWojdat thanks for bringing that up, I'll look into it :)

@szymonWojdat
Copy link
Contributor Author

Just added:

  • Battle.opponent_can_dynamax - as discussed
  • Battle.dynamax_turns_left - as a way of determining how many turns of dynamax are left for player, I figured this would be crucial information for any agent as it wouldn't have a way of figuring out how many turns of dynamax are left otherwise based on the current Battle state
  • Battle.opponent_dynamax_turns_left - same as above but for the opponent

I have tested both locally manually. Aside from writing tests for the new functionalities and fixing the failing ones, I believe we're done?

@hsahovic
Copy link
Owner

We'll have to update docs too and be sure that z-moves still work as expected. Other than that, yes :)

@szymonWojdat
Copy link
Contributor Author

Just added a simple test for dynamax. Do you mind explaining what do you mean by "be sure that z-moves work as expected"? Were you refering to the fact that those were gone for a while and then moved back in? If so, then those commits aren't even in the tree anymore, so I'd expect them to work normally. I can also test those manually against my local PS instance or write a simple unit test if necessary.

@hsahovic
Copy link
Owner

I was referring to this:

  • Move.can_z_move doesn't work as expected as moves.json no longer contains zMovePower for all damage moves, my guess is that PS has switched to some sort of dynamic calculation of the z-move power. The only place where it's used in code is at Pokemon.available_z_moves(). I assume the easiest workaround would be to modify Move.can_z_move so that it checks if given moves appears on a small list of moves that cannot z-move (test_can_z_move() implies as if Metronome could not z-move but it actually can, it just doesn't have any extra effect, moves that can't z-moves would be eg. Struggle)
  • for the same reason as mentioned above, test_z_move_power is failing as well. I'll try taking a look at their codebase to see what's the formula that they're using

@szymonWojdat
Copy link
Contributor Author

Thanks! I fixed both of those. Also added two pyre-ignores.

Now integration tests are failing. test_max_concurrent_battle_works is timing out. Do you mind taking a look? I guess we'll also need to add an integration test for gen8randombattle

@hsahovic
Copy link
Owner

@szymonWojdat Yes, sure! I just need to finish a couple of uni-related things; I'll get back into poke-env by the end of the week

@hsahovic hsahovic added documentation Improvements or additions to documentation enhancement New feature or request labels May 2, 2020
@hsahovic hsahovic added this to In progress in Poke-env - general via automation May 2, 2020
@hsahovic hsahovic linked an issue May 2, 2020 that may be closed by this pull request
@hsahovic hsahovic closed this May 5, 2020
Poke-env - general automation moved this from In progress to Done May 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
Development

Successfully merging this pull request may close these issues.

Gen 8 random battle results in no available moves
3 participants