Fix circular dependency during deserialization

Author: ottml

libs/s25main/figures/noFigure.cpp

@@ -48,7 +48,8 @@ const unsigned short WANDER_RADIUS_SOLDIERS = 15;
 noFigure::noFigure(const Job job, const MapPoint pos, const unsigned char player, noRoadNode* const goal)
     : noMovable(NodalObjectType::Figure, pos), fs(FigureState::GotToGoal), job_(job), player(player), cur_rs(nullptr),
       rs_pos(0), rs_dir(false), on_ship(false), goal_(goal), waiting_for_free_node(false), wander_way(0),
-      wander_tryings(0), flagPos_(MapPoint::Invalid()), flag_obj_id(0), burned_wh_id(0xFFFFFFFF), last_id(0xFFFFFFFF)
+      wander_tryings(0), flagPos_(MapPoint::Invalid()), flag_obj_id(0), burned_wh_id(0xFFFFFFFF), last_id(0xFFFFFFFF),
+      armor(false)
 {
     // Haben wir ein Ziel?
     // Gehen wir in ein Lagerhaus? Dann dürfen wir da nicht unsere Arbeit ausführen, sondern
@@ -62,7 +63,8 @@ noFigure::noFigure(const Job job, const MapPoint pos, const unsigned char player
 noFigure::noFigure(const Job job, const MapPoint pos, const unsigned char player)
     : noMovable(NodalObjectType::Figure, pos), fs(FigureState::Job), job_(job), player(player), cur_rs(nullptr),
       rs_pos(0), rs_dir(false), on_ship(false), goal_(nullptr), waiting_for_free_node(false), wander_way(0),
-      wander_tryings(0), flagPos_(MapPoint::Invalid()), flag_obj_id(0), burned_wh_id(0xFFFFFFFF), last_id(0xFFFFFFFF)
+      wander_tryings(0), flagPos_(MapPoint::Invalid()), flag_obj_id(0), burned_wh_id(0xFFFFFFFF), last_id(0xFFFFFFFF),
+      armor(false)
 {}
 
 void noFigure::Destroy()
@@ -85,6 +87,7 @@ void noFigure::Serialize(SerializedGameData& sgd) const
     sgd.PushUnsignedShort(rs_pos);
     sgd.PushBool(rs_dir);
     sgd.PushBool(on_ship);
+    sgd.PushBool(armor);
 
     if(fs == FigureState::GotToGoal || fs == FigureState::GoHome)
         sgd.PushObject(goal_);
@@ -104,7 +107,7 @@ void noFigure::Serialize(SerializedGameData& sgd) const
 noFigure::noFigure(SerializedGameData& sgd, const unsigned obj_id)
     : noMovable(sgd, obj_id), fs(sgd.Pop<FigureState>()), job_(sgd.Pop<Job>()), player(sgd.PopUnsignedChar()),
       cur_rs(sgd.PopObject<RoadSegment>(GO_Type::Roadsegment)), rs_pos(sgd.PopUnsignedShort()), rs_dir(sgd.PopBool()),
-      on_ship(sgd.PopBool()), last_id(0xFFFFFFFF)
+      on_ship(sgd.PopBool()), last_id(0xFFFFFFFF), armor(sgd.GetGameDataVersion() >= 12 ? sgd.PopBool() : false)
 {
     if(fs == FigureState::GotToGoal || fs == FigureState::GoHome)
         goal_ = sgd.PopObject<noRoadNode>();

libs/s25main/figures/noFigure.h

@@ -74,6 +74,31 @@ class noFigure : public noMovable
     /// Speichert letzten Animationsframes (zum Abspielen von Sounds)
     unsigned last_id;
 
+    /*
+       The armor variable should be a member of nofArmored. But this is not possible due to a circular dependency
+       between objects during object deserialization. If a soldier is sent to a military building from a warehouse, the
+       soldier is inserted into the ordered_troops list of the military building. This soldier is also in the leave
+       queue of the warehouse after some time. When the game is stored in this state a circular dependency exists.
+
+       What happens during deserialization?
+
+       The warehouse is deserialized first. And therefore deserialization of the soldier in the leave queue will start.
+       The deserialization is done by going up the class hierarchy. In the top base class GameObject the new obejct is
+       inserted into the líst of already deserialized objects. But at this time the object is not yet fully constructed.
+       In the base class noFigure the goal for the soldier is deserialized. The goal is a military building and holds
+       the ordered_troops list. When this list is deserialized we try to deserialize the soldier, we are currently
+       creating, again. Because the soldier is in the líst of already deserialized objects we get back a reference to
+       this instance from the list instead of creating it new. The soldier is inserted into the ordered_troops list.
+       Because this list is a sorted list a comparator is called to do the insertion. The comparator in this case is the
+       ComparatorSoldiersByRank. This comparator uses the rank of the soldier, the objectId and the armor for
+       comparison. But at this time the armor variable in the nofArmored subclass of the soldier is not yet deserialized
+       and has a random value. This leads to wrong insertion into the sorted list. And later on to a wrong application
+       state. Because after loading, when in the game the ordered soldier reaches the building the removing from the
+       ordered_troops list will fail. This problem does not exist for the rank of the soldier. Because the rank of a
+       soldier depends on the job id, which is deserialized in noFigure before the goal is deserialized.
+    */
+    bool armor;
+
     explicit noFigure(const noFigure&) = default;
 
 private:

libs/s25main/figures/nofArmored.cpp

@@ -14,26 +14,22 @@
 #include "gameTypes/JobTypes.h"
 
 nofArmored::nofArmored(Job job, MapPoint pos, unsigned char player, noRoadNode* goal, bool armor)
-    : noFigure(job, pos, player, goal), armor(armor)
-{}
+    : noFigure(job, pos, player, goal)
+{
+    this->armor = armor;
+}
 
-nofArmored::nofArmored(Job job, MapPoint pos, unsigned char player, bool armor)
-    : noFigure(job, pos, player), armor(armor)
-{}
+nofArmored::nofArmored(Job job, MapPoint pos, unsigned char player, bool armor) : noFigure(job, pos, player)
+{
+    this->armor = armor;
+}
 
 void nofArmored::Serialize(SerializedGameData& sgd) const
 {
     noFigure::Serialize(sgd);
-    sgd.PushBool(armor);
 }
 
-nofArmored::nofArmored(SerializedGameData& sgd, const unsigned obj_id) : noFigure(sgd, obj_id)
-{
-    if(sgd.GetGameDataVersion() >= 12)
-        armor = sgd.PopBool();
-    else
-        armor = false;
-}
+nofArmored::nofArmored(SerializedGameData& sgd, const unsigned obj_id) : noFigure(sgd, obj_id) {}
 
 void nofArmored::DrawArmorWalking(DrawPoint drawPt)
 {

libs/s25main/figures/nofArmored.h

@@ -13,9 +13,6 @@ class SerializedGameData;
 class nofArmored : public noFigure
 {
 protected:
-    /// Armor
-    bool armor;
-
     explicit nofArmored(const nofArmored&) = default;
 
     void DrawArmor(DrawPoint drawPt);