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

Force Switch Battle Property Tracking Bugfix #478

Merged
merged 4 commits into from
May 29, 2024
Merged

Force Switch Battle Property Tracking Bugfix #478

merged 4 commits into from
May 29, 2024

Conversation

cameronangliss
Copy link
Contributor

@cameronangliss cameronangliss commented Dec 26, 2023

I found that force_switch is not being tracked correctly, as it should always be of type bool, but I have found that sometimes it returns [True]. Here is an example request message that shows why the error is happening:

2023-12-26 16:45:56,358 - ShowdownEnv 1 - ERROR - Unhandled exception raised while handling message:
>battle-gen4randombattle-80371
|request|{"forceSwitch":[true],"side":{"name":"ShowdownEnv 1","id":"p1","pokemon":[{"ident":"p1: Glaceon","details":"Glaceon, L88, M","condition":"0 fnt","active":true,"stats":{"atk":113,"def":244,"spa":278,"spd":217,"spe":165},"moves":["protect","wish","hiddenpowerground","icebeam"],"baseAbility":"snowcloak","item":"leftovers","pokeball":"pokeball"},{"ident":"p1: Hitmonchan","details":"Hitmonchan, L87, M","condition":"229/229","active":false,"stats":{"atk":232,"def":187,"spa":111,"spd":241,"spe":182},"moves":["rapidspin","stoneedge","icepunch","drainpunch"],"baseAbility":"ironfist","item":"leftovers","pokeball":"pokeball"},{"ident":"p1: Raikou","details":"Raikou, L76","condition":"262/262","active":false,"stats":{"atk":135,"def":157,"spa":219,"spd":196,"spe":219},"moves":["aurasphere","thunderbolt","hiddenpowerice","calmmind"],"baseAbility":"pressure","item":"lifeorb","pokeball":"pokeball"},{"ident":"p1: Gyarados","details":"Gyarados, L78, F","condition":"276/276","active":false,"stats":{"atk":240,"def":168,"spa":139,"spd":201,"spe":171},"moves":["dragondance","earthquake","substitute","waterfall"],"baseAbility":"intimidate","item":"leftovers","pokeball":"pokeball"},{"ident":"p1: Magmortar","details":"Magmortar, L83, M","condition":"129/260","active":false,"stats":{"atk":162,"def":159,"spa":255,"spd":205,"spe":185},"moves":["fireblast","thunderbolt","focusblast","substitute"],"baseAbility":"flamebody","item":"leftovers","pokeball":"pokeball"},{"ident":"p1: Meganium","details":"Meganium, L90, M","condition":"52/290","active":false,"stats":{"atk":152,"def":231,"spa":201,"spd":231,"spe":195},"moves":["energyball","aromatherapy","synthesis","leechseed"],"baseAbility":"overgrow","item":"leftovers","pokeball":"pokeball"}]},"noCancel":true,"rqid":20}

Notice that we have "forceSwitch":[true]. I fix this bug by indexing into the first element of the request JSON's forceSwitch attribute.

Copy link

codecov bot commented Dec 26, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.20%. Comparing base (f458350) to head (3822386).
Report is 41 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #478      +/-   ##
==========================================
+ Coverage   83.38%   84.20%   +0.82%     
==========================================
  Files          39       42       +3     
  Lines        3918     4116     +198     
==========================================
+ Hits         3267     3466     +199     
+ Misses        651      650       -1     

@cameronangliss cameronangliss changed the title bugfix Force Switch Battle Property Tracking Bugfix Dec 26, 2023
@hsahovic
Copy link
Owner

Good catch - thanks for the fix. Can you add a unit test checking that the message is parsed properly?

@hsahovic
Copy link
Owner

@cameronangliss let me know if you need a hand with this :)

@cameronangliss
Copy link
Contributor Author

cameronangliss commented Jan 16, 2024

Hi @hsahovic, sorry, I saw what you wrote but I've just not gotten around to adding the unit tests yet. If you feel like you have the time and want to get this through quickly I'd have no issue with you taking it from here :) If not though, I can't say for sure when I'll be able to look into that myself, as I'm getting rather busy as of late.

@hsahovic
Copy link
Owner

No problem - i'll take a look whenever I have some time for poke-env :)

@hsahovic
Copy link
Owner

I'm closing this for now - let me know if you want to reopen it!

@hsahovic hsahovic closed this Apr 21, 2024
@cameronangliss
Copy link
Contributor Author

cameronangliss commented May 17, 2024

Hi @hsahovic! I'm done with my semester now so I have time to finish this PR. Open it back up and I'll get this done.

@cameronangliss
Copy link
Contributor Author

Update: I've got this branch working with updated unit tests up, so it should be good as soon as you reopen it.

@hsahovic hsahovic reopened this May 28, 2024
@hsahovic hsahovic merged commit d1ddcaf into hsahovic:master May 29, 2024
4 of 8 checks passed
@hsahovic
Copy link
Owner

Thanks @cameronangliss !

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.

2 participants