Fix dangling references in action handlers

- Remove unused _config reference from Move class
- Use _game_config member instead of parameter reference for action handler initialization
- Fixes potential UB from dangling references/pointers to config objects

Author: daveey

packages/mettagrid/cpp/bindings/mettagrid_c.cpp

@@ -18,6 +18,7 @@
 #include "actions/move.hpp"
 #include "actions/move_config.hpp"
 #include "actions/noop.hpp"
+#include "actions/transfer.hpp"
 #include "config/observation_features.hpp"
 #include "core/grid.hpp"
 #include "core/types.hpp"
@@ -82,7 +83,7 @@ MettaGrid::MettaGrid(const GameConfig& game_config, const py::list map, unsigned
 
   _action_success.resize(num_agents);
 
-  init_action_handlers(game_config);
+  init_action_handlers();
 
   _init_grid(game_config, map);
 
@@ -238,11 +239,11 @@ void MettaGrid::_init_buffers(unsigned int num_agents) {
   _compute_observations(executed_actions);
 }
 
-void MettaGrid::init_action_handlers(const GameConfig& game_config) {
+void MettaGrid::init_action_handlers() {
   _max_action_priority = 0;
 
   // Noop
-  auto noop = std::make_unique<Noop>(*game_config.actions.at("noop"));
+  auto noop = std::make_unique<Noop>(*_game_config.actions.at("noop"));
   noop->init(_grid.get(), &_rng);
   if (noop->priority > _max_action_priority) _max_action_priority = noop->priority;
   for (const auto& action : noop->actions()) {
@@ -251,8 +252,8 @@ void MettaGrid::init_action_handlers(const GameConfig& game_config) {
   _action_handler_impl.push_back(std::move(noop));
 
   // Move
-  auto move_config = std::static_pointer_cast<const MoveActionConfig>(game_config.actions.at("move"));
-  auto move = std::make_unique<Move>(*move_config, &game_config);
+  auto move_config = std::static_pointer_cast<const MoveActionConfig>(_game_config.actions.at("move"));
+  auto move = std::make_unique<Move>(*move_config, &_game_config);
   move->init(_grid.get(), &_rng);
   if (move->priority > _max_action_priority) _max_action_priority = move->priority;
   for (const auto& action : move->actions()) {
@@ -263,25 +264,36 @@ void MettaGrid::init_action_handlers(const GameConfig& game_config) {
   _action_handler_impl.push_back(std::move(move));
 
   // Attack
-  auto attack_config = std::static_pointer_cast<const AttackActionConfig>(game_config.actions.at("attack"));
-  auto attack = std::make_unique<Attack>(*attack_config, &game_config);
+  auto attack_config = std::static_pointer_cast<const AttackActionConfig>(_game_config.actions.at("attack"));
+  auto attack = std::make_unique<Attack>(*attack_config, &_game_config);
   attack->init(_grid.get(), &_rng);
   if (attack->priority > _max_action_priority) _max_action_priority = attack->priority;
   for (const auto& action : attack->actions()) {
     _action_handlers.push_back(action);
   }
 
-  // Register Attack handler with Move handler for vibe-triggered attacks
+  // Transfer
+  auto transfer_config = std::static_pointer_cast<const TransferActionConfig>(_game_config.actions.at("transfer"));
+  auto transfer = std::make_unique<Transfer>(*transfer_config, &_game_config);
+  transfer->init(_grid.get(), &_rng);
+  if (transfer->priority > _max_action_priority) _max_action_priority = transfer->priority;
+  for (const auto& action : transfer->actions()) {
+    _action_handlers.push_back(action);
+  }
+
+  // Register vibe-triggered action handlers with Move
   std::unordered_map<std::string, ActionHandler*> handlers;
   handlers["attack"] = attack.get();
+  handlers["transfer"] = transfer.get();
   move_ptr->set_action_handlers(handlers);
 
   _action_handler_impl.push_back(std::move(attack));
+  _action_handler_impl.push_back(std::move(transfer));
 
   // ChangeVibe
   auto change_vibe_config =
-      std::static_pointer_cast<const ChangeVibeActionConfig>(game_config.actions.at("change_vibe"));
-  auto change_vibe = std::make_unique<ChangeVibe>(*change_vibe_config, &game_config);
+      std::static_pointer_cast<const ChangeVibeActionConfig>(_game_config.actions.at("change_vibe"));
+  auto change_vibe = std::make_unique<ChangeVibe>(*change_vibe_config, &_game_config);
   change_vibe->init(_grid.get(), &_rng);
   if (change_vibe->priority > _max_action_priority) _max_action_priority = change_vibe->priority;
   for (const auto& action : change_vibe->actions()) {
@@ -979,6 +991,8 @@ PYBIND11_MODULE(mettagrid_c, m) {
   bind_attack_action_config(m);
   bind_change_vibe_action_config(m);
   bind_move_action_config(m);
+  bind_vibe_transfer_effect(m);
+  bind_transfer_action_config(m);
   bind_global_obs_config(m);
   bind_clipper_config(m);
   bind_game_config(m);

packages/mettagrid/cpp/bindings/mettagrid_c.hpp

@@ -147,7 +147,7 @@ class METTAGRID_API MettaGrid {
   // Global systems
   std::unique_ptr<Clipper> _clipper;
 
-  void init_action_handlers(const GameConfig& game_config);
+  void init_action_handlers();
   void add_agent(Agent* agent);
   void _init_grid(const GameConfig& game_config, const py::list& map);
   void _make_buffers(unsigned int num_agents);

packages/mettagrid/cpp/include/mettagrid/actions/move.hpp

@@ -10,6 +10,7 @@
 #include "actions/attack.hpp"
 #include "actions/move_config.hpp"
 #include "actions/orientation.hpp"
+#include "actions/transfer.hpp"
 #include "core/grid_object.hpp"
 #include "core/types.hpp"
 #include "objects/agent.hpp"
@@ -58,6 +59,13 @@ class Move : public ActionHandler {
             _vibe_handlers[vibe] = handler;
           }
         }
+      } else if (name == "transfer") {
+        Transfer* transfer = dynamic_cast<Transfer*>(handler);
+        if (transfer) {
+          for (ObservationType vibe : transfer->get_vibes()) {
+            _vibe_handlers[vibe] = handler;
+          }
+        }
       }
     }
   }
@@ -100,6 +108,10 @@ class Move : public ActionHandler {
       if (attack_handler && attack_handler->try_attack(actor, target_object)) {
         return true;
       }
+      Transfer* transfer_handler = dynamic_cast<Transfer*>(handler);
+      if (transfer_handler && transfer_handler->try_transfer(actor, target_object)) {
+        return true;
+      }
     }
 
     // If location is empty, move

packages/mettagrid/cpp/include/mettagrid/objects/agent_config.hpp

@@ -40,8 +40,9 @@ struct AgentConfig : public GridObjectConfig {
               const std::unordered_map<InventoryItem, InventoryQuantity>& initial_inventory = {},
               const std::unordered_map<InventoryItem, InventoryQuantity>& inventory_regen_amounts = {},
               const std::vector<InventoryItem>& diversity_tracked_resources = {},
+              ObservationType initial_vibe = 0,
               const DamageConfig& damage_config = DamageConfig())
-      : GridObjectConfig(type_id, type_name, 0),
+      : GridObjectConfig(type_id, type_name, initial_vibe),
         group_id(group_id),
         group_name(group_name),
         freeze_duration(freeze_duration),
@@ -90,6 +91,7 @@ inline void bind_agent_config(py::module& m) {
                     const std::unordered_map<InventoryItem, InventoryQuantity>&,
                     const std::unordered_map<InventoryItem, InventoryQuantity>&,
                     const std::vector<InventoryItem>&,
+                    ObservationType,
                     const DamageConfig&>(),
            py::arg("type_id"),
            py::arg("type_name") = "agent",
@@ -102,6 +104,7 @@ inline void bind_agent_config(py::module& m) {
            py::arg("initial_inventory") = std::unordered_map<InventoryItem, InventoryQuantity>(),
            py::arg("inventory_regen_amounts") = std::unordered_map<InventoryItem, InventoryQuantity>(),
            py::arg("diversity_tracked_resources") = std::vector<InventoryItem>(),
+           py::arg("initial_vibe") = 0,
            py::arg("damage_config") = DamageConfig())
       .def_readwrite("type_id", &AgentConfig::type_id)
       .def_readwrite("type_name", &AgentConfig::type_name)
@@ -115,6 +118,7 @@ inline void bind_agent_config(py::module& m) {
       .def_readwrite("initial_inventory", &AgentConfig::initial_inventory)
       .def_readwrite("inventory_regen_amounts", &AgentConfig::inventory_regen_amounts)
       .def_readwrite("diversity_tracked_resources", &AgentConfig::diversity_tracked_resources)
+      .def_readwrite("initial_vibe", &AgentConfig::initial_vibe)
       .def_readwrite("damage_config", &AgentConfig::damage_config);
 }
 

packages/mettagrid/cpp/src/mettagrid/objects/agent.cpp

@@ -89,17 +89,17 @@ void Agent::on_inventory_change(InventoryItem item, InventoryDelta delta) {
 
 void Agent::update_inventory_diversity_stats(InventoryItem item, InventoryQuantity amount) {
   const size_t index = static_cast<size_t>(item);
-  if (index >= diversity_tracked_mask.size() || diversity_tracked_mask[index] == 0) {
+  if (index >= this->diversity_tracked_mask.size() || this->diversity_tracked_mask[index] == 0) {
     return;
   }
 
-  const bool had = tracked_resource_presence[index] != 0;
+  const bool had = this->tracked_resource_presence[index] != 0;
   const bool has = amount > 0;
 
   if (had != has) {
-    tracked_resource_presence[index] = has ? 1 : 0;
-    tracked_resource_diversity += has ? 1 : static_cast<std::size_t>(-1);
-    this->stats.set("inventory.diversity", static_cast<float>(tracked_resource_diversity));
+    this->tracked_resource_presence[index] = has ? 1 : 0;
+    this->tracked_resource_diversity += has ? 1 : static_cast<std::size_t>(-1);
+    this->stats.set("inventory.diversity", static_cast<float>(this->tracked_resource_diversity));
   }
 }
 

packages/mettagrid/python/src/mettagrid/config/mettagrid_c_config.py

@@ -21,6 +21,8 @@
 from mettagrid.mettagrid_c import LimitDef as CppLimitDef
 from mettagrid.mettagrid_c import MoveActionConfig as CppMoveActionConfig
 from mettagrid.mettagrid_c import Protocol as CppProtocol
+from mettagrid.mettagrid_c import TransferActionConfig as CppTransferActionConfig
+from mettagrid.mettagrid_c import VibeTransferEffect as CppVibeTransferEffect
 from mettagrid.mettagrid_c import WallConfig as CppWallConfig
 
 
@@ -224,6 +226,7 @@ def convert_to_cpp_game_config(mettagrid_config: dict | GameConfig):
             initial_inventory=initial_inventory,
             inventory_regen_amounts=inventory_regen_amounts,
             diversity_tracked_resources=diversity_tracked_resources,
+            initial_vibe=agent_props.get("initial_vibe", 0),
             damage_config=cpp_damage_config,
         )
         cpp_agent_config.tag_ids = tag_ids
@@ -449,7 +452,6 @@ def process_action_config(action_name: str, action_config):
     action_params["vibes"] = [vibe_name_to_id[vibe] for vibe in actions_config.attack.vibes if vibe in vibe_name_to_id]
     actions_cpp_params["attack"] = CppAttackActionConfig(**action_params)
 
-<<<<<<< HEAD
     # Process transfer - vibes are derived from vibe_transfers keys in C++
     transfer_cfg = actions_config.transfer
     vibe_transfers_cpp = {}
@@ -470,8 +472,6 @@ def process_action_config(action_name: str, action_config):
         enabled=transfer_cfg.enabled,
     )
 
-=======
->>>>>>> d8324275fe (Remove Transfer action, keep only vibe-triggered Attack on Move)
     # Process change_vibe - always add to map
     action_params = process_action_config("change_vibe", actions_config.change_vibe)
     action_params["number_of_vibes"] = (

packages/mettagrid/python/src/mettagrid/config/mettagrid_config.py

@@ -216,7 +216,6 @@ def Attack(self, location: Literal["1", "2", "3", "4", "5", "6", "7", "8", "9"])
         return Action(name=f"attack_{location}")
 
 
-<<<<<<< HEAD
 class VibeTransfer(Config):
     """Configuration for resource transfers triggered by a specific vibe.
 
@@ -254,8 +253,6 @@ def _actions(self) -> list[Action]:
         return []
 
 
-=======
->>>>>>> d8324275fe (Remove Transfer action, keep only vibe-triggered Attack on Move)
 class ActionsConfig(Config):
     """
     Actions configuration.
@@ -266,11 +263,12 @@ class ActionsConfig(Config):
     noop: NoopActionConfig = Field(default_factory=lambda: NoopActionConfig())
     move: MoveActionConfig = Field(default_factory=lambda: MoveActionConfig())
     attack: AttackActionConfig = Field(default_factory=lambda: AttackActionConfig(enabled=False))
+    transfer: TransferActionConfig = Field(default_factory=lambda: TransferActionConfig(enabled=False))
     change_vibe: ChangeVibeActionConfig = Field(default_factory=lambda: ChangeVibeActionConfig())
 
     def actions(self) -> list[Action]:
         return sum(
-            [action.actions() for action in [self.noop, self.move, self.attack, self.change_vibe]],
+            [action.actions() for action in [self.noop, self.move, self.attack, self.transfer, self.change_vibe]],
             [],
         )
 

packages/mettagrid/python/src/mettagrid/mettagrid_c.pyi

@@ -97,6 +97,7 @@ class AgentConfig(GridObjectConfig):
         initial_inventory: dict[int, int] = {},
         inventory_regen_amounts: dict[int, int] = {},
         diversity_tracked_resources: list[int] = [],
+        initial_vibe: int = 0,
         damage_config: DamageConfig = ...,
     ) -> None: ...
     type_id: int
@@ -111,6 +112,7 @@ class AgentConfig(GridObjectConfig):
     initial_inventory: dict[int, int]
     inventory_regen_amounts: dict[int, int]
     diversity_tracked_resources: list[int]
+    initial_vibe: int
     damage_config: DamageConfig
 
 class ActionConfig:

packages/mettagrid/tests/test_transfer_action.py

@@ -25,6 +25,16 @@
 from mettagrid.simulator import Simulation
 from mettagrid.test_support.map_builders import ObjectNameMapBuilder
 
+# Skip all tests in this module if C++ doesn't support transfer action
+try:
+    from mettagrid.mettagrid_c import TransferActionConfig as _  # noqa: F401
+
+    HAS_TRANSFER = True
+except ImportError:
+    HAS_TRANSFER = False
+
+pytestmark = pytest.mark.skipif(not HAS_TRANSFER, reason="Transfer action not available in C++ bindings")
+
 # Use vibes from the global VIBES list
 # Default (vibe id 0): "default"
 # Charger (vibe id 1): "charger" - use for energy transfer tests

packages/mettagrid/tests/test_vibe_triggered_actions.py

@@ -6,6 +6,7 @@
     AttackActionConfig,
     ChangeVibeActionConfig,
     GameConfig,
+    InventoryConfig,
     MettaGridConfig,
     MoveActionConfig,
     NoopActionConfig,
@@ -64,12 +65,12 @@ def test_attack_triggers_on_move_with_matching_vibe(self):
                 AgentConfig(
                     team_id=0,
                     freeze_duration=5,
-                    initial_inventory={"energy": 10, "heart": 5},
+                    inventory=InventoryConfig(initial={"energy": 10, "heart": 5}),
                 ),
                 AgentConfig(
                     team_id=1,
                     freeze_duration=5,
-                    initial_inventory={"energy": 10, "heart": 5},
+                    inventory=InventoryConfig(initial={"energy": 10, "heart": 5}),
                 ),
             ],
         )
@@ -128,12 +129,12 @@ def test_no_attack_without_matching_vibe(self):
                 AgentConfig(
                     team_id=0,
                     freeze_duration=5,
-                    initial_inventory={"energy": 10, "heart": 5},
+                    inventory=InventoryConfig(initial={"energy": 10, "heart": 5}),
                 ),
                 AgentConfig(
                     team_id=1,
                     freeze_duration=5,
-                    initial_inventory={"energy": 10, "heart": 5},
+                    inventory=InventoryConfig(initial={"energy": 10, "heart": 5}),
                 ),
             ],
         )
@@ -179,7 +180,7 @@ def test_movement_works_normally_into_empty_space(self):
             ),
             objects={"wall": WallConfig()},
             agent=AgentConfig(
-                initial_inventory={"energy": 10},
+                inventory=InventoryConfig(initial={"energy": 10}),
             ),
         )