Repository: hsahovic/poke-env

Issue:
Temporary Effects
There are temporary `Effect`s that only last a turn; for example:
- Quick Claw
- After You
- Custap Berry
- After You
- Flinch
- Helping Hand

But end silently in Showdown (eg they don't have a `['', '-end',...]` message). This means that once a `Pokemon` has the `Effect` applied to them, this effect incorrectly persists for the rest of the game in poke-env. Some end after a pokemon moves, and some end after the turn, and some end when switched out (among other reasons). Would you be open to a PR that adds logic for these various effects? I don't want AI that encodes the battle state to think that once you Helping Hand a mon, it will always be helping handed. It will also make things like damage calculations with poke-env very difficult

I'm not sure what the best way to maintain/test implementation logic is though, given that it is almost necessarily custom. Curious if you have thoughts?

Discussion:
@caymansimpson: @hsahovic I'm thinking of implementing by:
1. Adding a list of effects that go away after moves (e.g. glaiverush)
2. Adding a list of effects that go away after switches (e.g. all volatile statuses)
3. Adding a list of effects that go away after turns (e.g. After You)

I will add these as curated lists in effects.py and implement the checks in abstract_battle.py for the next PR. I'm not going to get 100% coverage of every effect, but I should be able to get at least 95%. LMK if you have any opinions on (1) implementation plan, (2) how I should test or (3) anything else!

Solution patch:
diff --git a/src/poke_env/environment/abstract_battle.py b/src/poke_env/environment/abstract_battle.py
index bc054ba98..d7d5b60ef 100644
--- a/src/poke_env/environment/abstract_battle.py
+++ b/src/poke_env/environment/abstract_battle.py
@@ -47,6 +47,7 @@ class AbstractBattle(ABC):
         "gametype",
         "html",
         "immune",
+        "inactiveoff",
         "init",
         "j",
         "join",
@@ -64,7 +65,6 @@ class AbstractBattle(ABC):
         "uhtml",
         "zbroken",
         "",
-        "inactiveoff",
     }
 
     __slots__ = (
diff --git a/src/poke_env/environment/effect.py b/src/poke_env/environment/effect.py
index c4b9b31e2..0fe111a39 100644
--- a/src/poke_env/environment/effect.py
+++ b/src/poke_env/environment/effect.py
@@ -299,6 +299,22 @@ def breaks_protect(self):
         """
         return self in _PROTECT_BREAKING_EFFECTS
 
+    @property
+    def ends_on_switch(self) -> bool:
+        """
+        :return: Whether this effect ends when the pokemon switches out.
+        :rtype: bool
+        """
+        return self.is_volatile_status or self in _ENDS_ON_SWITCH_EFFECTS
+
+    @property
+    def ends_on_turn(self) -> bool:
+        """
+        :return: Whether this effect ends at the end of the turn.
+        :rtype: bool
+        """
+        return self in _ENDS_ON_TURN_EFFECTS
+
     @property
     def is_turn_countable(self) -> bool:
         """
@@ -688,6 +704,42 @@ def is_from_move(self) -> bool:
     Effect.WRAP,
 }
 
+_ENDS_ON_MOVE_EFFECTS = {
+    Effect.GLAIVE_RUSH,
+    Effect.CHARGE,
+}
+
+_ENDS_ON_SWITCH_EFFECTS = {
+    Effect.MIND_READER,
+    Effect.MUMMY,
+    Effect.SPEED_SWAP,
+}
+
+_ENDS_ON_TURN_EFFECTS = {
+    Effect.AFTER_YOU,
+    Effect.BANEFUL_BUNKER,
+    Effect.BURNING_BULWARK,
+    Effect.CRAFTY_SHIELD,
+    Effect.FEINT,
+    Effect.FLINCH,
+    Effect.FOLLOW_ME,
+    Effect.KINGS_SHIELD,
+    Effect.CUSTAP_BERRY,
+    Effect.MIND_READER,
+    Effect.MAGIC_COAT,
+    Effect.OBSTRUCT,
+    Effect.PROTECT,
+    Effect.QUASH,
+    Effect.QUICK_CLAW,
+    Effect.QUICK_DRAW,
+    Effect.QUICK_GUARD,
+    Effect.RAGE_POWDER,
+    Effect.ROOST,
+    Effect.SPIKY_SHIELD,
+    Effect.SPOTLIGHT,
+    Effect.WIDE_GUARD,
+}
+
 _ACTION_COUNTER_EFFECTS: Set[Effect] = {Effect.RAGE, Effect.STOCKPILE}
 
 _FROM_DATA: Dict[str, Effect] = {
diff --git a/src/poke_env/environment/pokemon.py b/src/poke_env/environment/pokemon.py
index 9724f2c4d..2840442b3 100644
--- a/src/poke_env/environment/pokemon.py
+++ b/src/poke_env/environment/pokemon.py
@@ -216,9 +216,11 @@ def end_item(self, item: str):
     def end_turn(self):
         if self._status == Status.TOX:
             self._status_counter += 1
-        for effect in self.effects:
+        for effect in list(self.effects.keys()):
             if effect.is_turn_countable:
                 self.effects[effect] += 1
+            if effect.ends_on_turn:
+                self.end_effect(effect.name)
 
     def faint(self):
         self._current_hp = 0
@@ -278,6 +280,16 @@ def moved(self, move_id: str, failed: bool = False, use: bool = True):
 
             self._moves = new_moves
 
+        # Handle silent effect ending
+        if Effect.GLAIVE_RUSH in self.effects:
+            self.end_effect("Glaive Rush")
+        if (
+            Effect.CHARGE in self.effects
+            and isinstance(move, Move)
+            and move.type == PokemonType.ELECTRIC
+        ):
+            self.end_effect("Charge")
+
     def prepare(self, move_id: str, target: Optional[Pokemon]):
         self.moved(move_id, use=False)
 
@@ -314,6 +326,10 @@ def set_hp_status(self, hp_status: str, store=False):
         if " " in hp_status:
             hp, status = hp_status.split(" ")
             self._status = Status[status.upper()]
+
+            # Handle silent effect ending
+            if self._status == Status.SLP and Effect.YAWN in self.effects:
+                self.end_effect("yawn")
         else:
             hp = hp_status
 
@@ -362,6 +378,10 @@ def switch_out(self):
         if self._status == Status.TOX:
             self._status_counter = 0
 
+        for effect in self.effects:
+            if effect.ends_on_switch or effect.is_volatile_status:
+                self.end_effect(effect.name)
+
     def terastallize(self, type_: str):
         self._terastallized_type = PokemonType.from_name(type_)
         self._terastallized = True
diff --git a/src/poke_env/player/env_player.py b/src/poke_env/player/env_player.py
index eeb5d515f..405b144ce 100644
--- a/src/poke_env/player/env_player.py
+++ b/src/poke_env/player/env_player.py
@@ -3,7 +3,8 @@
 
 from abc import ABC
 from threading import Lock
-from typing import Dict, List, Optional, Union
+from typing import List, Optional, Union
+from weakref import WeakKeyDictionary
 
 from poke_env.environment.abstract_battle import AbstractBattle
 from poke_env.player.battle_order import BattleOrder, ForfeitBattleOrder
@@ -81,7 +82,9 @@ def __init__(
             or leave it inactive.
         :type start_challenging: bool
         """
-        self._reward_buffer: Dict[AbstractBattle, float] = {}
+        self._reward_buffer: WeakKeyDictionary[AbstractBattle, float] = (
+            WeakKeyDictionary()
+        )
         self._opponent_lock = Lock()
         self._opponent = opponent
         b_format = self._DEFAULT_BATTLE_FORMAT


Test patch:
diff --git a/unit_tests/environment/test_enumerations.py b/unit_tests/environment/test_enumerations.py
index 73ee4cae5..b28235d9e 100644
--- a/unit_tests/environment/test_enumerations.py
+++ b/unit_tests/environment/test_enumerations.py
@@ -7,6 +7,7 @@
     Field,
     Move,
     MoveCategory,
+    Pokemon,
     PokemonGender,
     SideCondition,
     Status,
@@ -90,6 +91,36 @@ def test_effect_build():
             )
 
 
+def test_effect_end():
+    furret = Pokemon(gen=9, species="furret")
+    furret._effects = {
+        Effect.GLAIVE_RUSH: 1,
+        Effect.CHARGE: 1,
+        Effect.QUASH: 1,
+        Effect.MUMMY: 1,
+    }
+    furret.moved("followme")
+
+    assert Effect.GLAIVE_RUSH not in furret.effects
+    assert Effect.CHARGE in furret.effects
+
+    furret.moved("zapcannon")
+    assert Effect.CHARGE not in furret.effects
+
+    # Test end on turn
+    furret.end_turn()
+    assert Effect.QUASH not in furret.effects
+
+    # Test end on switch
+    furret.switch_out()
+    assert Effect.MUMMY not in furret.effects
+
+    # Test the definition of Volatile Status
+    for effect in list(Effect):
+        if effect.is_volatile_status:
+            assert effect.ends_on_switch
+
+
 def test_field_str():
     assert str(Field["ELECTRIC_TERRAIN"])