Merge pull request Revolutionary-Games#178 from Revolutionary-Games/c…

…leaning-up-transfer-system

Cleanup of entity transfer system and crash-fixes.
Fixed crashes related to gamestate changes in certain orderings. This fixes Revolutionary-Games#162 and Revolutionary-Games#163.

scripts/microbe_editor/microbe_editor.lua

@@ -27,12 +27,14 @@ function MicrobeEditor:__init(hudSystem)
 end
 
 function MicrobeEditor:activate()
-    playerEntity = Entity(Engine:playerData():activeCreature(), GameState.MICROBE)
-    self.lockedMap = Engine:playerData():lockedMap()
-    self.nextMicrobeEntity = playerEntity:transfer(GameState.MICROBE_EDITOR)
-    self.nextMicrobeEntity:stealName("working_microbe")
-    Engine:playerData():setBool("edited_microbe", true)
-    Engine:playerData():setActiveCreature(self.nextMicrobeEntity.id)
+    if Engine:playerData():activeCreatureGamestate():name() == GameState.MICROBE:name() then 
+        microbeStageMicrobe = Entity(Engine:playerData():activeCreature(), GameState.MICROBE)
+        self.lockedMap = Engine:playerData():lockedMap()
+        self.nextMicrobeEntity = microbeStageMicrobe:transfer(GameState.MICROBE_EDITOR)
+        self.nextMicrobeEntity:stealName("working_microbe")
+        Engine:playerData():setBool("edited_microbe", true)
+        Engine:playerData():setActiveCreature(self.nextMicrobeEntity.id, GameState.MICROBE_EDITOR)
+    end
 end
 
 function MicrobeEditor:update(milliseconds)
@@ -148,7 +150,7 @@ function MicrobeEditor:loadMicrobe(entityId)
     self.currentMicrobe.sceneNode.transform.orientation = Quaternion(Radian(Degree(180)), Vector3(0, 0, 1))-- Orientation
     self.currentMicrobe.sceneNode.transform:touch()
     self.currentMicrobe.collisionHandler:addCollisionGroup("powerupable")
-    Engine:playerData():setActiveCreature(entityId)
+    Engine:playerData():setActiveCreature(entityId, GameState.MICROBE_EDITOR)
 end
 
 function MicrobeEditor:createNewMicrobe()

scripts/microbe_stage/microbe_replacement.lua

@@ -20,7 +20,7 @@ function MicrobeReplacementSystem:activate()
         newPlayerMicrobe = newMicrobeEntity:transfer(GameState.MICROBE)
         newPlayerMicrobe:stealName(PLAYER_NAME)
         global_newEditorMicrobe = false
-        Engine:playerData():setActiveCreature(newPlayerMicrobe.id)
+        Engine:playerData():setActiveCreature(newPlayerMicrobe.id, GameState.MICROBE)
     end
 
 end

scripts/microbe_stage/setup.lua

@@ -374,7 +374,7 @@ local function setupPlayer()
     microbe = createStarterMicrobe(PLAYER_NAME, false)
     microbe.collisionHandler:addCollisionGroup("powerupable")
     Engine:playerData():lockedMap():addLock("Toxin")
-    Engine:playerData():setActiveCreature(microbe.entity.id)
+    Engine:playerData():setActiveCreature(microbe.entity.id, GameState.MICROBE)
 end
 
 local function setupSound()

src/engine/component_collection.cpp

@@ -147,22 +147,6 @@ ComponentCollection::removeComponent(
     return false;
 }
 
-std::unique_ptr<Component>
-ComponentCollection::extractComponent(
-    EntityId entityId
-) {
-    auto iter = m_impl->m_components.find(entityId);
-    if (iter != m_impl->m_components.end()) {
-        iter->second->setOwner(NULL_ENTITY);
-        std::unique_ptr<Component> component = std::move(iter->second);
-        m_impl->m_components.erase(iter);
-        return std::move(component);
-    }
-    else{
-        throw std::invalid_argument("No component for the provided entityId existed.");
-    }
-}
-
 
 ComponentTypeId
 ComponentCollection::type() const {

src/engine/component_collection.h

@@ -169,22 +169,6 @@ class ComponentCollection {
         EntityId entityId
     );
 
-    /**
-    * @brief Removes a component from this collection returning an owning pointer instead of destroying it
-    *
-    * Does not call any callbacks
-    *
-    * @param entityId
-    *   The entity the component belongs to
-    *
-    * @return
-    *   A unique_ptr containing the component
-    */
-    std::unique_ptr<Component>
-    extractComponent(
-        EntityId entityId
-    );
-
     struct Implementation;
     std::unique_ptr<Implementation> m_impl;
 

src/engine/entity_manager.cpp

@@ -41,8 +41,6 @@ struct EntityManager::Implementation {
 
     std::list<EntityId> m_entitiesToRemove;
 
-    std::list<std::tuple<EntityId, EntityId, GameState*>> m_entitiesToTransfer;
-
     std::unordered_map<std::string, EntityId> m_namedIds;
 
     std::unordered_set<EntityId> m_volatileEntities;
@@ -173,44 +171,6 @@ EntityManager::stealName(
 }
 
 
-
-
-EntityId
-EntityManager::transferEntity(
-    EntityId entityId,
-    GameState* gameState
-) {
-    //Get a new id, this initial id will not be used and therefore wasted if we find a name mapping.
-    EntityId newEntity = gameState->entityManager().generateNewId();
-    // Rough way of checking for a name mapping. But performance should not matter much here.
-    for (auto& pair : m_impl->m_namedIds){
-        if (pair.second == entityId){
-            newEntity = gameState->entityManager().getNamedId(pair.first);
-            break;
-        }
-    }
-    m_impl->m_entitiesToTransfer.emplace_back(std::tuple<EntityId, EntityId, GameState*>(entityId, newEntity, gameState));
-    return newEntity;
-}
-
-
-void
-EntityManager::processTransfers() {
-    for (auto& tuple : m_impl->m_entitiesToTransfer){
-        for (const auto& pair : m_impl->m_collections) {
-            if (pair.second->get(std::get<0>(tuple)) != nullptr){
-                std::get<2>(tuple)->entityManager().addComponent(std::get<1>(tuple), pair.second->extractComponent(std::get<0>(tuple)));
-                auto iter = m_impl->m_entities.find(std::get<0>(tuple));
-                iter->second -= 1;
-                if (iter->second == 0) {
-                    m_impl->m_entities.erase(iter);
-                }
-            }
-        }
-    }
-}
-
-
 std::unordered_set<ComponentTypeId>
 EntityManager::nonEmptyCollections() const {
     std::unordered_set<ComponentTypeId> collections;
@@ -281,9 +241,10 @@ EntityManager::transferEntity(
             ) {
                 auto newComponent = componentFactory.load(componentFactory.getTypeName(pair.first), component->storage());
                 newComponent->setOwner(newEntityId);
-                this->removeComponent(oldEntityId, pair.first);
+
                 newEntityManager.addComponent(newEntityId, std::move(newComponent));
             }
+            this->removeComponent(oldEntityId, pair.first);
         }
     }
 }

src/engine/entity_manager.h

@@ -270,23 +270,6 @@ class EntityManager {
         const std::string& name
     );
 
-    /**
-    * @brief Transfers an entity to a different gamestate removing it from the current one
-    *
-    * @param entityId
-    *   The entity to transfer
-    *
-    * @param gameState
-    *  The new gamestate to own the entity
-    *
-    * @return
-    *  The new entity id in the new gamestate
-    */
-    EntityId
-    transferEntity(
-        EntityId entityId,
-        GameState* gameState
-    );
 
     /**
     * @brief Stores a single entity

src/engine/game_state.cpp

@@ -262,6 +262,5 @@ GameState::update(
             system->update(milliseconds);
         }
     }
-    m_impl->m_entityManager.processTransfers(); // Transfer requests trumps remove request with this ordering
     m_impl->m_entityManager.processRemovals();
 }

src/engine/player_data.cpp

@@ -1,5 +1,6 @@
 #include "engine/player_data.h"
 
+#include "engine/game_state.h"
 #include "engine/serialization.h"
 #include "general/locked_map.h"
 #include "scripting/luabind.h"
@@ -17,6 +18,7 @@ struct PlayerData::Implementation {
     }
 
     EntityId m_activeCreature = NULL_ENTITY;
+    GameState* m_activeCreatureGamestate = nullptr;
 
     std::string m_playerName;
 
@@ -35,6 +37,7 @@ PlayerData::luaBindings() {
         .def("lockedMap", &PlayerData::lockedMap)
         .def("activeCreature", &PlayerData::activeCreature)
         .def("setActiveCreature", &PlayerData::setActiveCreature)
+        .def("activeCreatureGamestate", &PlayerData::activeCreatureGamestate)
         .def("isBoolSet", &PlayerData::isBoolSet)
         .def("setBool", &PlayerData::setBool)
     ;
@@ -65,9 +68,16 @@ PlayerData::activeCreature(){
 
 void
 PlayerData::setActiveCreature(
-    EntityId creatureId
+    EntityId creatureId,
+    GameState& gamestate
 ){
     m_impl->m_activeCreature = creatureId;
+    m_impl->m_activeCreatureGamestate = &gamestate;
+}
+
+GameState&
+PlayerData::activeCreatureGamestate(){
+    return *m_impl->m_activeCreatureGamestate;
 }
 
 bool

src/engine/player_data.h

@@ -11,10 +11,10 @@ namespace luabind {
 
 namespace thrive {
 
+class GameState;
 class StorageContainer;
 class LockedMap;
 
-
 class PlayerData {
 
 public:
@@ -42,6 +42,7 @@ class PlayerData {
     * - PlayerData::lockedMap
     * - PlayerData::activeCreature
     * - PlayerData::setActiveCreature
+    * - PlayerData::activeCreatureGamestate
     * - PlayerData::isBoolSet
     * - PlayerData::setBool
     *
@@ -51,23 +52,23 @@ class PlayerData {
     luaBindings();
 
     /**
-    * @brief getter for the players name
+    * @brief Getter for the players name
     *
     * @return
     */
     const std::string&
     playerName();
 
     /**
-    * @brief getter for the map of locked concepts
+    * @brief Getter for the map of locked concepts
     *
     * @return
     */
     LockedMap&
     lockedMap();
 
     /**
-    * @brief getter for the id of the players currently active creature entity
+    * @brief Getter for the id of the players currently active creature entity
     *
     * @return
     */
@@ -79,12 +80,24 @@ class PlayerData {
     *
     * @param creatureId
     *  Entity id of the creature
+    *
+    * @param gamestate
+    *  The gamestate that the players new active creature belongs to
     */
     void
     setActiveCreature(
-        EntityId creatureId
+        EntityId creatureId,
+        GameState& gamestate
     );
 
+    /**
+    * @brief Getter for the players currently active creatures gamestate
+    *
+    * @return
+    */
+    GameState&
+    activeCreatureGamestate();
+
     /**
     * @brief Returns whether a key has a true bool set to it
     *