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

Putting "pass" in message when battle order is None in DoubleBattleOrder #659

Open
wants to merge 26 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 7 additions & 35 deletions src/poke_env/environment/double_battle.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,13 @@ def parse_request(self, request: Dict[str, Any]) -> None:
if side["pokemon"]:
self._player_role = side["pokemon"][0]["ident"][:2]
self._update_team_from_request(side)
if self.player_role is not None:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this change in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, this was a little while ago, but if I remember right, this was to try and simplify the code and fix Zoroark bugs on our side of the battle specifically (since that's all the request tells us about, is our own side).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to be clear, it's a replacement for the code being removed a few lines further below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I definitely could test taking it out real quick and see what happens, whether that breaks stuff.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I remember it all now. So, yes, this is a necessary fix, and it is for Zoroark. This is also just a much easier code chunk to look at than what it's replacing. Basically, the request will always put the active pokemon in the first and second positions in the pokemon list, and although you could be worried that we aren't correctly filtering out fainted pokemon, if you look at the _get_active_pokemon method in double_battle.py (which is used to give the active_pokemon property), you'll see that it is filtered there, so we were actually double-filtering before.

self._active_pokemon[f"{self.player_role}a"] = self.team[
request["side"]["pokemon"][0]["ident"]
]
self._active_pokemon[f"{self.player_role}b"] = self.team[
request["side"]["pokemon"][1]["ident"]
]

if "active" in request:
for active_pokemon_number, active_request in enumerate(request["active"]):
Expand All @@ -139,41 +146,6 @@ def parse_request(self, request: Dict[str, Any]) -> None:
force_self_team=True,
details=pokemon_dict["details"],
)
if self.player_role is not None:
if (
active_pokemon_number == 0
and f"{self.player_role}a" not in self._active_pokemon
):
self._active_pokemon[f"{self.player_role}a"] = active_pokemon
elif f"{self.player_role}b" not in self._active_pokemon:
self._active_pokemon[f"{self.player_role}b"] = active_pokemon
elif (
active_pokemon_number == 0
and self._active_pokemon[f"{self.player_role}a"].fainted
and self._active_pokemon[f"{self.player_role}b"]
== active_pokemon
):
(
self._active_pokemon[f"{self.player_role}a"],
self._active_pokemon[f"{self.player_role}b"],
) = (
self._active_pokemon[f"{self.player_role}b"],
self._active_pokemon[f"{self.player_role}a"],
)
elif (
active_pokemon_number == 1
and self._active_pokemon[f"{self.player_role}b"].fainted
and not active_pokemon.fainted
and self._active_pokemon[f"{self.player_role}a"]
== active_pokemon
):
(
self._active_pokemon[f"{self.player_role}a"],
self._active_pokemon[f"{self.player_role}b"],
) = (
self._active_pokemon[f"{self.player_role}b"],
self._active_pokemon[f"{self.player_role}a"],
)

if active_pokemon.fainted:
continue
Expand Down
6 changes: 3 additions & 3 deletions src/poke_env/player/baselines.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import random
from typing import List
from typing import List, Optional

from poke_env.environment.abstract_battle import AbstractBattle
from poke_env.environment.double_battle import DoubleBattle
Expand Down Expand Up @@ -29,7 +29,7 @@ def choose_singles_move(self, battle: AbstractBattle):
return self.choose_random_move(battle)

def choose_doubles_move(self, battle: DoubleBattle):
orders: List[BattleOrder] = []
orders: List[Optional[BattleOrder]] = []
switched_in = None

if any(battle.force_switch):
Expand All @@ -51,7 +51,7 @@ def choose_doubles_move(self, battle: DoubleBattle):
switches = [s for s in switches if s != switched_in]

if not mon or mon.fainted:
orders.append(DefaultBattleOrder())
orders.append(None)
continue
elif not moves and switches:
mon_to_switch_in = random.choice(switches)
Expand Down
11 changes: 6 additions & 5 deletions src/poke_env/player/battle_order.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ def message(self) -> str:
+ self.second_order.message.replace("/choose ", "")
)
elif self.first_order:
return self.first_order.message + ", default"
return self.first_order.message + ", pass"
elif self.second_order:
return self.second_order.message + ", default"
return "/choose pass, " + self.second_order.message.replace("/choose ", "")
else:
return self.DEFAULT_ORDER

Expand All @@ -95,10 +95,11 @@ def join_orders(first_orders: List[BattleOrder], second_orders: List[BattleOrder
if orders:
return orders
elif first_orders:
return [DoubleBattleOrder(first_order=order) for order in first_orders]
return [DoubleBattleOrder(order, None) for order in first_orders]
elif second_orders:
return [DoubleBattleOrder(first_order=order) for order in second_orders]
return [DefaultBattleOrder()]
return [DoubleBattleOrder(None, order) for order in second_orders]
else:
return [DoubleBattleOrder(None, None)]


class ForfeitBattleOrder(BattleOrder):
Expand Down
4 changes: 1 addition & 3 deletions src/poke_env/player/player.py
Original file line number Diff line number Diff line change
Expand Up @@ -532,9 +532,7 @@ def choose_random_doubles_move(battle: DoubleBattle) -> BattleOrder:
second_switch_in = random.choice(available_switches)
second_order = BattleOrder(second_switch_in)

if first_order and second_order:
return DoubleBattleOrder(first_order, second_order)
return DoubleBattleOrder(first_order or second_order, None)
return DoubleBattleOrder(first_order, second_order)

for (
orders,
Expand Down
14 changes: 6 additions & 8 deletions unit_tests/player/test_battle_orders.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,8 @@ def test_double_orders():
DoubleBattleOrder(mon, move).message
== "/choose switch lugia, move selfdestruct 2"
)
assert DoubleBattleOrder(mon).message == "/choose switch lugia, default"
assert (
DoubleBattleOrder(None, move).message == "/choose move selfdestruct 2, default"
)
assert DoubleBattleOrder(mon).message == "/choose switch lugia, pass"
assert DoubleBattleOrder(None, move).message == "/choose pass, move selfdestruct 2"
assert DoubleBattleOrder().message == "/choose default"

orders = [move, mon]
Expand All @@ -53,12 +51,12 @@ def test_double_orders():
"/choose switch lugia, move selfdestruct 2",
}
assert first == {
"/choose move selfdestruct 2, default",
"/choose switch lugia, default",
"/choose move selfdestruct 2, pass",
"/choose switch lugia, pass",
}
assert second == {
"/choose move selfdestruct 2, default",
"/choose switch lugia, default",
"/choose pass, move selfdestruct 2",
"/choose pass, switch lugia",
}
assert none == {"/choose default"}

Expand Down
8 changes: 4 additions & 4 deletions unit_tests/player/test_doubles_baselines.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ def test_doubles_max_damage_player():
battle._active_pokemon["p1a"] = active_pikachu

# calls player.choose_random_doubles_move(battle)
assert player.choose_move(battle).message == "/choose default, default"
assert player.choose_move(battle).message == "/choose default, pass"

# calls player.choose_random_doubles_move(battle)
battle._available_switches[0].append(Pokemon(species="ponyta", gen=8))
assert player.choose_move(battle).message == "/choose switch ponyta, default"
assert player.choose_move(battle).message == "/choose switch ponyta, pass"

active_raichu = Pokemon(species="raichu", gen=8)
active_raichu.switch_in()
Expand Down Expand Up @@ -65,12 +65,12 @@ def test_doubles_max_damage_player():
# forced switch
battle._force_switch = [True, False]
assert player.choose_move(battle).message in [
"/choose switch ponyta, default",
"/choose switch ponyta, pass",
]

battle._force_switch = [False, True]
assert player.choose_move(battle).message in [
"/choose switch rapidash, default",
"/choose pass, switch rapidash",
]

battle._force_switch = [True, True]
Expand Down
Loading