mockingbirdnest · Mar 6, 2017 · Mar 6, 2017 · Mar 6, 2017 · Mar 6, 2017 · Mar 6, 2017
diff --git a/ksp_plugin/interface.cpp b/ksp_plugin/interface.cpp
@@ -708,7 +708,7 @@ void principia__RenderedPredictionApsides(Plugin const* const plugin,
   std::unique_ptr<DiscreteTrajectory<World>> rendered_apoapsides;
   std::unique_ptr<DiscreteTrajectory<World>> rendered_periapsides;
   plugin->ComputeAndRenderApsides(celestial_index,
-                                  prediction.Fork(),
+                                  prediction.Begin(),
                                   prediction.End(),
                                   q_sun,
                                   rendered_apoapsides,

diff --git a/ksp_plugin/part.cpp b/ksp_plugin/part.cpp
@@ -24,7 +24,8 @@ Part::Part(
       deletion_callback_(std::move(deletion_callback)) {}
 
 Part::~Part() {
-  clear_pile_up();
+  LOG(INFO) << "Destroying part " << part_id_;
+  CHECK(!is_piled_up());
   if (deletion_callback_ != nullptr) {
     deletion_callback_();
   }

diff --git a/ksp_plugin/part.hpp b/ksp_plugin/part.hpp
@@ -43,8 +43,8 @@ class Part final {
        DegreesOfFreedom<Barycentric> const& degrees_of_freedom,
        std::function<void()> deletion_callback);
 
-  // Calls the deletion callback passed at construction, if any.  Calls
-  // |clear_pile_up|.
+  // Calls the deletion callback passed at construction, if any.  This part must
+  // not be piled up.
   ~Part();
 
   virtual PartId part_id() const;

diff --git a/ksp_plugin/plugin.cpp b/ksp_plugin/plugin.cpp
@@ -141,6 +141,21 @@ void Plugin::InsertCelestialAbsoluteCartesian(
                                                   initial_state);
 }
 
+Plugin::~Plugin() {
+  // If we simply let the destructor of |Part| deal with removing itself from
+  // pile-ups, by calling |clear_pile_up|, it will try to access the other
+  // parts, which may have been already destroyed...
+  for (auto const& pair : vessels_) {
+    pair.second->ForAllParts([](Part& part) {
+      part.clear_pile_up();
+    });
+  }
+  // We must manually destroy the vessels, triggering the destruction of the
+  // parts, which have callbacks to remove themselves from |part_id_to_vessel_|,
+  // which must therefore still exist.
+  vessels_.clear();
+}
+
 void Plugin::InsertCelestialJacobiKeplerian(
     Index const celestial_index,
     std::experimental::optional<Index> const& parent_index,
@@ -472,6 +487,8 @@ void Plugin::FreeVesselsAndPartsAndCollectPileUps() {
     } else {
       CHECK(!is_loaded(vessel));
       LOG(INFO) << "Removing vessel with GUID " << it->first;
+      // See the destructor.
+      vessel->ForAllParts([](Part& part) { part.clear_pile_up(); });
       it = vessels_.erase(it);
     }
   }
@@ -688,7 +705,7 @@ not_null<std::unique_ptr<DiscreteTrajectory<World>>> Plugin::RenderedPrediction(
     Position<World> const& sun_world_position) const {
   CHECK(!initializing_);
   Vessel const& vessel = *find_vessel_by_guid_or_die(vessel_guid);
-  return RenderedTrajectoryFromIterators(vessel.prediction().Fork(),
+  return RenderedTrajectoryFromIterators(vessel.prediction().Begin(),
                                          vessel.prediction().End(),
                                          sun_world_position);
 }

diff --git a/ksp_plugin/plugin.hpp b/ksp_plugin/plugin.hpp
@@ -84,7 +84,7 @@ class Plugin {
   Plugin(Plugin&&) = delete;
   Plugin& operator=(Plugin const&) = delete;
   Plugin& operator=(Plugin&&) = delete;
-  virtual ~Plugin() = default;
+  virtual ~Plugin();
 
   // Constructs a |Plugin|. The current time of that instance is
   // |solar_system_epoch|.  The angle between the axes of |World| and

diff --git a/ksp_plugin/vessel.cpp b/ksp_plugin/vessel.cpp
@@ -52,11 +52,13 @@ void Vessel::set_parent(not_null<Celestial const*> const parent) {
 
 void Vessel::AddPart(not_null<std::unique_ptr<Part>> part) {
   PartId const id = part->part_id();
-  parts_.emplace(id, std::move(part));
+  LOG(INFO) << "Adding part " << id << " to vessel at " << this;
   kept_parts_.insert(part.get());
+  parts_.emplace(id, std::move(part));
 }
 
 not_null<std::unique_ptr<Part>> Vessel::ExtractPart(PartId const id) {
+  LOG(INFO) << "Extracting part " << id << " from vessel at " << this;
   auto const it = parts_.find(id);
   CHECK(it != parts_.end()) << id;
   auto result = std::move(it->second);
@@ -69,7 +71,6 @@ void Vessel::KeepPart(PartId const id) {
 }
 
 void Vessel::FreeParts() {
-  dummy_part_.reset();
   for (auto it = parts_.begin(); it != parts_.end();) {
     not_null<Part const*> part = it->second.get();
     if (kept_parts_.count(part) == 0) {
@@ -163,6 +164,8 @@ void Vessel::AdvanceTime(Instant const& time) {
 }
 
 void Vessel::ForgetBefore(Instant const& time) {
+  // TODO(egg): do something about the psychohistory. I think bad things happen
+  // if it becomes empty though.
   prediction_->ForgetBefore(time);
   if (flight_plan_ != nullptr) {
     flight_plan_->ForgetBefore(time, [this]() { flight_plan_.reset(); });
@@ -209,9 +212,6 @@ void Vessel::WriteToMessage(
   body_.WriteToMessage(message->mutable_body());
   prediction_adaptive_step_parameters_.WriteToMessage(
       message->mutable_prediction_adaptive_step_parameters());
-  if (dummy_part_ != nullptr) {
-    dummy_part_->WriteToMessage(message->mutable_dummy_part());
-  }
   for (auto const& pair : parts_) {
     auto const& part = pair.second;
     part->WriteToMessage(message->add_parts());
@@ -241,10 +241,6 @@ not_null<std::unique_ptr<Vessel>> Vessel::ReadFromMessage(
       ephemeris,
       Ephemeris<Barycentric>::AdaptiveStepParameters::ReadFromMessage(
           message.prediction_adaptive_step_parameters()));
-  if (message.has_dummy_part()) {
-    vessel->dummy_part_ = Part::ReadFromMessage(message.dummy_part(),
-                                                /*deletion_callback=*/nullptr);
-  }
   for (auto const& serialized_part : message.parts()) {
     PartId const part_id = serialized_part.part_id();
     auto part = Part::ReadFromMessage(

diff --git a/ksp_plugin/vessel.hpp b/ksp_plugin/vessel.hpp
@@ -76,9 +76,9 @@ class Vessel {
   // |AddPart|.
   virtual not_null<Part*> part(PartId id) const;
 
-  // Calls |action| on one (non-dummy) part.
+  // Calls |action| on one part.
   virtual void ForSomePart(std::function<void(Part&)> action) const;
-  // Calls |action| on all non-dummy parts.
+  // Calls |action| on all parts.
   virtual void ForAllParts(std::function<void(Part&)> action) const;
 
   virtual DiscreteTrajectory<Barycentric> const& prediction() const;
@@ -145,13 +145,14 @@ class Vessel {
   not_null<Celestial const*> parent_;
   not_null<Ephemeris<Barycentric>*> const ephemeris_;
 
-  std::unique_ptr<Part> dummy_part_;
   std::map<PartId, not_null<std::unique_ptr<Part>>> parts_;
   std::set<not_null<Part const*>> kept_parts_;
 
   // The new implementation of history, also encompasses the prolongation.
   not_null<std::unique_ptr<DiscreteTrajectory<Barycentric>>> psychohistory_;
-  bool psychohistory_is_authoritative_;
+  // TODO(egg): this is nonsensical, we start with an empty psychohistory, how
+  // can that be authoritative?  This class needs saner invariants.
+  bool psychohistory_is_authoritative_ = true;
 
   not_null<std::unique_ptr<DiscreteTrajectory<Barycentric>>> prediction_;
 

diff --git a/ksp_plugin_adapter/ksp_plugin_adapter.cs b/ksp_plugin_adapter/ksp_plugin_adapter.cs
@@ -233,12 +233,13 @@ private void UpdateBody(CelestialBody body, double universal_time) {
   }
 
   private void UpdateVessel(Vessel vessel, double universal_time) {
-    QP from_parent = plugin_.VesselFromParent(vessel.id.ToString());
-    vessel.orbit.UpdateFromStateVectors(
-        pos     : (Vector3d)from_parent.q,
-        vel     : (Vector3d)from_parent.p,
-        refBody : vessel.orbit.referenceBody,
-        UT      : universal_time);
+     if (plugin_.HasVessel(vessel.id.ToString())) {
+       QP from_parent = plugin_.VesselFromParent(vessel.id.ToString());
+       vessel.orbit.UpdateFromStateVectors(pos : (Vector3d)from_parent.q,
+                                           vel : (Vector3d)from_parent.p,
+                                           refBody : vessel.orbit.referenceBody,
+                                           UT : universal_time);
+     }
   }
 
   private bool is_in_space(Vessel vessel) {
@@ -353,6 +354,7 @@ public override void OnAwake() {
 
   public override void OnSave(ConfigNode node) {
     base.OnSave(node);
+#if THE_SERIALIZATION_WORKS_AGAIN
     if (PluginRunning()) {
       String serialization;
       IntPtr serializer = IntPtr.Zero;
@@ -364,13 +366,15 @@ public override void OnSave(ConfigNode node) {
         node.AddValue(principia_key, serialization);
       }
     }
+#endif
   }
 
   public override void OnLoad(ConfigNode node) {
     base.OnLoad(node);
     if (must_record_journal_) {
       Log.ActivateRecorder(true);
     }
+#if THE_SERIALIZATION_WORKS_AGAIN
     if (node.HasValue(principia_key)) {
       Cleanup();
       SetRotatingFrameThresholds();
@@ -403,9 +407,12 @@ public override void OnLoad(ConfigNode node) {
       plugin_construction_ = DateTime.Now;
       plugin_source_ = PluginSource.SAVED_STATE;
     } else {
+#endif
       Log.Warning("No principia state found, creating one");
       ResetPlugin();
+#if THE_SERIALIZATION_WORKS_AGAIN
     }
+#endif
   }
 
   #endregion
@@ -759,9 +766,6 @@ private System.Collections.IEnumerator
 
      plugin_.FreeVesselsAndPartsAndCollectPileUps();
 
-     var apparent_world_positions = new Dictionary<Vessel, Vector3d>();
-     var apparent_world_velocities = new Dictionary<Vessel, Vector3d>();
-
      foreach (Vessel vessel in FlightGlobals.VesselsLoaded) {
        if (vessel.packed || !plugin_.HasVessel(vessel.id.ToString())) {
          continue;
@@ -801,16 +805,25 @@ private System.Collections.IEnumerator
              plugin_.GetPartActualDegreesOfFreedom(part.flightID);
          // TODO(egg): use the centre of mass.  Here it's a bit tedious, some
          // transform nonsense must probably be done.
+         Log.Error(
+             "q correction " +
+             ((Vector3d)part_actual_degrees_of_freedom.q - part.rb.position));
+         Log.Error(
+             "v correction " +
+             ((Vector3d)part_actual_degrees_of_freedom.p - part.rb.velocity));
          part.rb.position = (Vector3d)part_actual_degrees_of_freedom.q;
          part.rb.velocity = (Vector3d)part_actual_degrees_of_freedom.p;
        }
      }
      if (has_active_vessel_in_space() && FlightGlobals.ActiveVessel.loaded) {
        QP main_body_dof = plugin_.CelestialWorldDegreesOfFreedom(
            FlightGlobals.ActiveVessel.mainBody.flightGlobalsIndex);
-       krakensbane.FrameVel = (Vector3d)main_body_dof.p;
+       Log.Error("change in framevel: " +
+                 (-(Vector3d)main_body_dof.p - krakensbane.FrameVel));
+       krakensbane.FrameVel = -(Vector3d)main_body_dof.p;
        Vector3d offset = (Vector3d)main_body_dof.q -
                          FlightGlobals.ActiveVessel.mainBody.position;
+       Log.Error("shifting the world by " + offset);
        // We cannot use FloatingOrigin.SetOffset to move the world here, because
        // as far as I can tell, that does not move the bubble relative to the
        // rest of the universe.
@@ -854,11 +867,14 @@ private void ReportVesselsAndParts() {
     if (PluginRunning()) {
       foreach (Vessel vessel in FlightGlobals.Vessels.Where(is_in_space)) {
         bool inserted;
+        // WTF? or should I use !packed?
+        bool actually_loaded_with_real_parts =
+            vessel.loaded && !vessel.parts.TrueForAll(part => part.rb == null);
         plugin_.InsertOrKeepVessel(vessel.id.ToString(),
                                    vessel.mainBody.flightGlobalsIndex,
-                                   vessel.loaded,
+                                   actually_loaded_with_real_parts,
                                    out inserted);
-        if (vessel.loaded) {
+        if (actually_loaded_with_real_parts) {
           QP main_body_degrees_of_freedom =
               new QP{q = (XYZ)vessel.mainBody.position,
                      p = (XYZ)(-krakensbane.FrameVel)};

diff --git a/serialization/ksp_plugin.proto b/serialization/ksp_plugin.proto
@@ -119,7 +119,6 @@ message Vessel {
   required MasslessBody body = 1;
   required Ephemeris.AdaptiveStepParameters 
       prediction_adaptive_step_parameters = 10;
-  optional Part dummy_part = 13;
   repeated Part parts = 14;
   repeated uint32 kept_parts = 15;
   required DiscreteTrajectory psychohistory = 16;