From c85947f187caf5a414db4a2074fb94514b25362b Mon Sep 17 00:00:00 2001
From: Sara <sara@saragerretsen.nl>
Date: Sun, 17 Mar 2024 14:49:35 +0100
Subject: [PATCH] feat: refactored how players are removed

---
 game_root.cpp | 39 +++++++++++++++++++++++++--------------
 game_root.hpp | 14 +++++++++-----
 2 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/game_root.cpp b/game_root.cpp
index 88a3b85..22b5143 100644
--- a/game_root.cpp
+++ b/game_root.cpp
@@ -50,19 +50,39 @@ void GameRoot::player_input_connected() {
     this->emit_signal(StringName("player_connected"), input);
 }
 
-void GameRoot::player_removed(uint32_t player_id) {
+void GameRoot::remove_player(uint32_t player_id) {
     if(!this->players.has(player_id))
         return;
+    // convert player object to node
+    Node *node = this->players.get(player_id).second->to_node();
+    if(node == nullptr) {
+        UtilityFunctions::push_error("IPlayer::to_node failed for player with id '", player_id, "'");
+        return;
+    }
+    node->queue_free();
     this->players.get(player_id).second = nullptr;
 }
 
+void GameRoot::remove_all_players() {
+    // free all player instances in use
+    for(KeyValue<uint32_t, Pair<PlayerInput*, IPlayer*>> &pair : this->players) {
+        // skip unused player slots
+        if(pair.value.second == nullptr)
+            continue;
+        else
+            this->remove_player(pair.key);
+    }
+}
+
 bool GameRoot::initialize_player(IPlayer *player, uint32_t id) {
     if(!this->players.has(id))
         return false;
+    // register the player
     Pair<PlayerInput*, IPlayer*> &found{this->players.get(id)};
-    this->add_child(player->to_node());
-    player->player_id = id;
     found.second = player;
+    // set player id
+    player->player_id = id;
+    this->add_child(player->to_node());
     player->setup_player_input(found.first);
     return true;
 }
@@ -98,18 +118,9 @@ uint32_t GameRoot::find_empty_player_slot() const {
 }
 
 void GameRoot::set_game_mode(Ref<GameMode> prototype) {
-    // free all player instances in use
-    for(KeyValue<uint32_t, Pair<PlayerInput*, IPlayer*>> &pair : this->players) {
-        if(pair.value.second == nullptr) continue;
-        Node *node = dynamic_cast<Node*>(pair.value.second);
-        if(node == nullptr) {
-            UtilityFunctions::push_error("Attempt to cast player '", pair.key, "' to node failed");
-        } else {
-            node->queue_free();
-        }
-    }
+    this->remove_all_players();
     // allow "unsetting" the gamemode by passing an invalid gamemode
-    //  shorthand for this behaviour is reset_game_mode
+    // shorthand for this behaviour is reset_game_mode
     if(prototype.is_null() || !prototype.is_valid()) {
         this->game_mode.unref();
         return;
diff --git a/game_root.hpp b/game_root.hpp
index b632b6b..e34bba0 100644
--- a/game_root.hpp
+++ b/game_root.hpp
@@ -1,15 +1,15 @@
 #ifndef GAME_ROOT_HPP
 #define GAME_ROOT_HPP
 
-#include <godot_cpp/classes/packed_scene.hpp>
+#include "game_mode.hpp"
+#include "level.hpp"
 #include <godot_cpp/classes/node.hpp>
+#include <godot_cpp/classes/packed_scene.hpp>
 #include <godot_cpp/templates/hash_map.hpp>
 #include <godot_cpp/templates/hash_set.hpp>
 #include <godot_cpp/templates/pair.hpp>
-#include <godot_cpp/templates/vector.hpp>
 #include <godot_cpp/templates/pair.hpp>
-#include "game_mode.hpp"
-#include "level.hpp"
+#include <godot_cpp/templates/vector.hpp>
 
 namespace godot {
 class PlayerInput;
@@ -31,8 +31,12 @@ public:
     void player_input_connected();
     // force-disconnect a player
     // calls queue_free on the IPlayer instance
-    void player_removed(uint32_t player_id);
+    void remove_player(uint32_t player_id);
+    // calls remove_player for every used player input slot
+    void remove_all_players();
     // initialize and register a player instance
+    // the player will be added to the tree and AFTER setup_player_input will be called
+    // this way the player can initialize before setting up input
     bool initialize_player(IPlayer *player, uint32_t id);
 
     // shorthand for set_game_mode(Ref<GameMode>())