From 8fac78c3569452e88c40068b4ec314704df1eb42 Mon Sep 17 00:00:00 2001 From: pleroy Date: Sun, 17 Sep 2017 11:59:51 +0200 Subject: [PATCH 1/8] A test that exposes a similar (but distinct) problem. --- ksp_plugin_adapter/ksp_plugin_adapter.cs | 2 +- ksp_plugin_test/plugin_test.cpp | 85 +++++++++++++----------- 2 files changed, 49 insertions(+), 38 deletions(-) diff --git a/ksp_plugin_adapter/ksp_plugin_adapter.cs b/ksp_plugin_adapter/ksp_plugin_adapter.cs index 810009ac75..27ab5a8c56 100644 --- a/ksp_plugin_adapter/ksp_plugin_adapter.cs +++ b/ksp_plugin_adapter/ksp_plugin_adapter.cs @@ -952,7 +952,7 @@ private void FixedUpdate() { // TODO(egg): Set the degrees of freedom of the origin of |World| (by // toying with Krakensbane and FloatingOrigin) here. - // Now we let the game and Unity do their thing. among other things, + // Now we let the game and Unity do their thing. Among other things, // the FashionablyLate callbacks, including ReportNonConservativeForces, // then the FlightIntegrator's FixedUpdate will run, then the Vessel's, // and eventually the physics simulation. diff --git a/ksp_plugin_test/plugin_test.cpp b/ksp_plugin_test/plugin_test.cpp index 93bfd60b08..9037c33628 100644 --- a/ksp_plugin_test/plugin_test.cpp +++ b/ksp_plugin_test/plugin_test.cpp @@ -253,43 +253,6 @@ class PluginTest : public testing::Test { return time + step * plugin_->Δt(); } - // Keeps the vessel with the given |guid| during the next call to - // |AdvanceTime|. The vessel must be present. - void KeepVessel(GUID const& guid) { - bool inserted; - plugin_->InsertOrKeepVessel(guid, - "v" + guid, - SolarSystemFactory::Earth, - /*loaded=*/false, - inserted); - EXPECT_FALSE(inserted) << guid; - } - - // Inserts a vessel with the given |guid| and makes it a satellite of Earth - // with relative position |satellite_initial_displacement_| and velocity - // |satellite_initial_velocity_|. The vessel must not already be present. - // Increments the counter |number_of_new_vessels|. - void InsertVessel(GUID const& guid, - PartId const& part_id, - std::size_t& number_of_new_vessels, - Instant const& time) { - bool inserted; - plugin_->InsertOrKeepVessel(guid, - "v" + guid, - SolarSystemFactory::Earth, - /*loaded=*/false, - inserted); - EXPECT_TRUE(inserted) << guid; - EXPECT_CALL(plugin_->mock_ephemeris(), Prolong(time)).RetiresOnSaturation(); - plugin_->InsertUnloadedPart( - part_id, - "p" + std::to_string(part_id), - guid, - RelativeDegreesOfFreedom(satellite_initial_displacement_, - satellite_initial_velocity_)); - ++number_of_new_vessels; - } - void PrintSerializedPlugin(const Plugin& plugin) { serialization::Plugin message; plugin.WriteToMessage(&message); @@ -1053,6 +1016,7 @@ TEST_F(PluginTest, UpdateCelestialHierarchy) { VanishesBefore(initial_from_parent.velocity().Norm(), 718, 422847)); } } + TEST_F(PluginTest, Navball) { // Create a plugin with planetarium rotation 0. Plugin plugin(initial_time_, @@ -1101,6 +1065,53 @@ TEST_F(PluginTest, Navball) { VanishesBefore(1, 4)); } +TEST_F(PluginTest, NavballTargetVessel) { + GUID const guid = "Target Vessel"; + PartId const part_id = 666; + + Plugin plugin(initial_time_, + initial_time_, + 0 * Radian); + + serialization::GravityModel::Body gravity_model; + CHECK(google::protobuf::TextFormat::ParseFromString( + R"(name : "Sun" + gravitational_parameter : "1 m^3/s^2" + reference_instant : "JD2451545.0" + mean_radius : "1 m" + axis_right_ascension : "0 deg" + axis_declination : "90 deg" + reference_angle : "0 deg" + angular_frequency : "1 rad/s")", + &gravity_model)); + plugin.InsertCelestialAbsoluteCartesian( + SolarSystemFactory::Sun, + /*parent_index=*/std::experimental::nullopt, + gravity_model, + solar_system_->cartesian_initial_state_message( + SolarSystemFactory::name(SolarSystemFactory::Sun))); + plugin.EndInitialization(); + + bool inserted; + plugin.InsertOrKeepVessel(guid, + "v" + guid, + SolarSystemFactory::Sun, + /*loaded=*/false, + inserted); + plugin.InsertUnloadedPart( + part_id, + "part", + guid, + RelativeDegreesOfFreedom(satellite_initial_displacement_, + satellite_initial_velocity_)); + plugin.PrepareToReportCollisions(); + plugin.FreeVesselsAndPartsAndCollectPileUps(20 * Milli(Second)); + + plugin.SetTargetVessel(guid, SolarSystemFactory::Sun); + plugin.AdvanceTime(plugin.CurrentTime() + 12 * Hour, 0 * Radian); + plugin.NavballFrameField(World::origin)->FromThisFrame(World::origin); +} + TEST_F(PluginTest, Frenet) { // Create a plugin with planetarium rotation 0. Plugin plugin(initial_time_, From a25b5a9b750a469996f02c37a6d3dd452bbe309a Mon Sep 17 00:00:00 2001 From: pleroy Date: Sun, 17 Sep 2017 17:26:13 +0200 Subject: [PATCH 2/8] Move the code that extends the prediction to Renderer. --- ksp_plugin/plugin.cpp | 28 ++++----------- ksp_plugin/plugin.hpp | 4 --- ksp_plugin/renderer.cpp | 60 +++++++++++++++++++++---------- ksp_plugin/renderer.hpp | 30 +++++++++++----- ksp_plugin/vessel.cpp | 14 ++++++-- ksp_plugin_test/renderer_test.cpp | 15 ++++++-- 6 files changed, 94 insertions(+), 57 deletions(-) diff --git a/ksp_plugin/plugin.cpp b/ksp_plugin/plugin.cpp index f07be8980b..058724dfcc 100644 --- a/ksp_plugin/plugin.cpp +++ b/ksp_plugin/plugin.cpp @@ -730,11 +730,10 @@ void Plugin::ComputeAndRenderClosestApproaches( Position const& sun_world_position, std::unique_ptr>& closest_approaches) const { CHECK(renderer_->HasTargetVessel()); - UpdatePredictionForRendering(begin.trajectory()->Size()); DiscreteTrajectory apoapsides_trajectory; DiscreteTrajectory periapsides_trajectory; - ComputeApsides(renderer_->GetTargetVessel().prediction(), + ComputeApsides(renderer_->GetTargetVesselPrediction(current_time_), begin, end, apoapsides_trajectory, @@ -754,10 +753,6 @@ void Plugin::ComputeAndRenderNodes( Position const& sun_world_position, std::unique_ptr>& ascending, std::unique_ptr>& descending) const { - if (renderer_->HasTargetVessel()) { - UpdatePredictionForRendering(begin.trajectory()->Size()); - } - auto const trajectory_in_plotting = renderer_->RenderBarycentricTrajectoryInPlotting(begin, end); DiscreteTrajectory ascending_trajectory; @@ -873,12 +868,12 @@ void Plugin::SetTargetVessel(GUID const& vessel_guid, not_null const celestial = FindOrDie(celestials_, reference_body_index).get(); not_null const vessel = FindOrDie(vessels_, vessel_guid).get(); - // Make sure that the current time is covered by the prediction. - if (current_time_ > vessel->prediction().t_max()) { - vessel->UpdatePrediction(current_time_ + prediction_length_); - } - - renderer_->SetTargetVessel(vessel, celestial, ephemeris_.get()); + renderer_->SetTargetVessel(vessel, + celestial, + ephemeris_.get(), + /*prediction_last_time=*/[this]() { + return current_time_ + prediction_length_; + }); } std::unique_ptr> Plugin::NavballFrameField( @@ -1255,15 +1250,6 @@ void Plugin::UpdatePlanetariumRotation() { to_planetarium; } -void Plugin::UpdatePredictionForRendering(std::int64_t const size) const { - auto& vessel = renderer_->GetTargetVessel(); - auto parameters = vessel.prediction_adaptive_step_parameters(); - // Adding one to ensure that we set a strictly positive max_steps. - parameters.set_max_steps(size + 1); - vessel.set_prediction_adaptive_step_parameters(parameters); - vessel.UpdatePrediction(current_time_ + prediction_length_); -} - Velocity Plugin::VesselVelocity( Instant const& time, DegreesOfFreedom const& degrees_of_freedom) const { diff --git a/ksp_plugin/plugin.hpp b/ksp_plugin/plugin.hpp index 7e7ec8631e..3bff77a19f 100644 --- a/ksp_plugin/plugin.hpp +++ b/ksp_plugin/plugin.hpp @@ -406,10 +406,6 @@ class Plugin { // whenever |main_body_| or |planetarium_rotation_| changes. void UpdatePlanetariumRotation(); - // NOTE(egg): this is an ugly hack to try to get a long enough trajectory - // while retaining a timeout. - void UpdatePredictionForRendering(std::int64_t size) const; - Velocity VesselVelocity( Instant const& time, DegreesOfFreedom const& degrees_of_freedom) const; diff --git a/ksp_plugin/renderer.cpp b/ksp_plugin/renderer.cpp index 9b9ac39b1c..5be471690e 100644 --- a/ksp_plugin/renderer.cpp +++ b/ksp_plugin/renderer.cpp @@ -39,12 +39,13 @@ not_null Renderer::GetPlottingFrame() const { void Renderer::SetTargetVessel( not_null const vessel, not_null const celestial, - not_null const*> const ephemeris) { - CHECK(!vessel->prediction().Empty()); + not_null const*> const ephemeris, + std::function prediction_last_time) { if (!target_ || target_->vessel != vessel || target_->celestial != celestial) { - target_.emplace(vessel, celestial, ephemeris); + target_.emplace( + vessel, celestial, ephemeris, std::move(prediction_last_time)); } } @@ -72,6 +73,17 @@ Vessel const& Renderer::GetTargetVessel() const { return *target_->vessel; } +DiscreteTrajectory const& Renderer::GetTargetVesselPrediction( + Instant const& time) const { + CHECK(target_); + target_->vessel->UpdatePrediction( + std::max(time, target_->prediction_last_time())); + // The prediction may not have been prolonged to |time| if we are near a + // singularity. + CHECK_LE(time, target_->vessel->prediction().last().time()); + return target_->vessel->prediction(); +} + not_null>> Renderer::RenderBarycentricTrajectoryInWorld( Instant const& time, @@ -94,19 +106,20 @@ not_null>> Renderer::RenderBarycentricTrajectoryInPlotting( DiscreteTrajectory::Iterator const& begin, DiscreteTrajectory::Iterator const& end) const { - CHECK(!target_ || !target_->vessel->prediction().Empty()); - auto trajectory = make_not_null_unique>(); for (auto it = begin; it != end; ++it) { + Instant const& t = it.time(); if (target_) { - if (it.time() < target_->vessel->prediction().t_min()) { + target_->vessel->UpdatePrediction( + std::max(t, target_->prediction_last_time())); + if (t < target_->vessel->prediction().t_min()) { continue; - } else if (it.time() > target_->vessel->prediction().t_max()) { + } else if (t > target_->vessel->prediction().t_max()) { break; } } trajectory->Append( - it.time(), + t, BarycentricToPlotting(it.time())(it.degrees_of_freedom())); } return trajectory; @@ -158,7 +171,7 @@ Renderer::RenderPlottingTrajectoryInWorld( RigidMotion Renderer::BarycentricToPlotting( Instant const& time) const { - return GetPlottingFrame()->ToThisFrameAtTime(time); + return GetPlottingFrame(time)->ToThisFrameAtTime(time); } RigidTransformation Renderer::BarycentricToWorld( @@ -187,7 +200,6 @@ OrthogonalMap, World> Renderer::FrenetToWorld( NavigationManœuvre const& manœuvre, Rotation const& planetarium_rotation) const { Instant const initial_time = manœuvre.initial_time(); - NavigationFrame const& plotting_frame = *GetPlottingFrame(); return PlottingToWorld(time, planetarium_rotation) * BarycentricToPlotting(initial_time).orthogonal_map() * manœuvre.FrenetFrame(); @@ -204,8 +216,9 @@ OrthogonalMap, World> Renderer::FrenetToWorld( BarycentricToPlotting(time)(barycentric_degrees_of_freedom); Rotation, Navigation> const frenet_frame_to_plotting_frame = - GetPlottingFrame()->FrenetFrame(time, - plotting_frame_degrees_of_freedom); + GetPlottingFrame(time)->FrenetFrame( + time, + plotting_frame_degrees_of_freedom); return PlottingToWorld(time, planetarium_rotation) * frenet_frame_to_plotting_frame.Forget(); @@ -230,7 +243,7 @@ OrthogonalMap, World> Renderer::FrenetToWorld( OrthogonalMap Renderer::PlottingToBarycentric( Instant const& time) const { - return GetPlottingFrame()->FromThisFrameAtTime(time).orthogonal_map(); + return GetPlottingFrame(time)->FromThisFrameAtTime(time).orthogonal_map(); } RigidTransformation Renderer::PlottingToWorld( @@ -238,7 +251,8 @@ RigidTransformation Renderer::PlottingToWorld( Position const& sun_world_position, Rotation const& planetarium_rotation) const { return BarycentricToWorld(time, sun_world_position, planetarium_rotation) * - GetPlottingFrame()->FromThisFrameAtTime(time).rigid_transformation(); + GetPlottingFrame(time)-> + FromThisFrameAtTime(time).rigid_transformation(); } OrthogonalMap Renderer::PlottingToWorld( @@ -287,15 +301,25 @@ not_null> Renderer::ReadFromMessage( Renderer::Target::Target( not_null const vessel, not_null const celestial, - not_null const*> const ephemeris) + not_null const*> const ephemeris, + std::function prediction_last_time) : vessel(vessel), celestial(celestial), target_frame( make_not_null_unique< BodyCentredBodyDirectionDynamicFrame>( - ephemeris, - [this]() -> auto& { return this->vessel->prediction(); }, - celestial->body())) {} + ephemeris, + [this]() -> auto& { return this->vessel->prediction(); }, + celestial->body())), + prediction_last_time(std::move(prediction_last_time)) {} + +not_null Renderer::GetPlottingFrame( + Instant const& time) const { + if (target_) { + GetTargetVesselPrediction(time); + } + return GetPlottingFrame(); +} } // namespace internal_renderer } // namespace ksp_plugin diff --git a/ksp_plugin/renderer.hpp b/ksp_plugin/renderer.hpp index 85ec815f84..f79471cf04 100644 --- a/ksp_plugin/renderer.hpp +++ b/ksp_plugin/renderer.hpp @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include "base/not_null.hpp" @@ -48,12 +49,13 @@ class Renderer { virtual not_null GetPlottingFrame() const; // Overrides the current plotting frame with one that is centred on the given - // |vessel|. When using the operations below with a target vessel, the client - // must ensure that the |time| is covered by the vessel's prediction. + // |vessel|. The time returned by |prediction_last_time| is used to prolong + // the prediction of the |vessel| when needed. virtual void SetTargetVessel( not_null vessel, not_null celestial, - not_null const*> ephemeris); + not_null const*> ephemeris, + std::function prediction_last_time); // Reverts to frame last set by |SetPlottingFrame|. The second version only // has an effect if the given |vessel| is the current target vessel. @@ -65,6 +67,11 @@ class Renderer { virtual Vessel& GetTargetVessel(); virtual Vessel const& GetTargetVessel() const; + // If there is a target vessel, returns its prediction after extending it up + // to |time|. + virtual DiscreteTrajectory const& GetTargetVesselPrediction( + Instant const& time) const; + // Returns a trajectory in |World| corresponding to the trajectory defined by // |begin| and |end|, as seen in the current plotting frame. In this function // and others in this class, |sun_world_position| is the current position of @@ -165,18 +172,25 @@ class Renderer { not_null const*> ephemeris); private: - not_null const sun_; - - not_null> plotting_frame_; - struct Target { Target(not_null vessel, not_null celestial, - not_null const*> ephemeris); + not_null const*> ephemeris, + std::function prediction_last_time); not_null const vessel; not_null const celestial; not_null> const target_frame; + std::function prediction_last_time; }; + + // Returns a plotting frame suitable for evaluation at |time|, possibly by + // extending the prediction if there is a target vessel. + not_null GetPlottingFrame(Instant const& time) const; + + not_null const sun_; + + not_null> plotting_frame_; + std::experimental::optional target_; }; diff --git a/ksp_plugin/vessel.cpp b/ksp_plugin/vessel.cpp index 73e60bc470..79bb1858e8 100644 --- a/ksp_plugin/vessel.cpp +++ b/ksp_plugin/vessel.cpp @@ -229,9 +229,17 @@ void Vessel::DeleteFlightPlan() { } void Vessel::UpdatePrediction(Instant const& last_time) { - prediction_ = make_not_null_unique>(); - auto const last = psychohistory_->last(); - prediction_->Append(last.time(), last.degrees_of_freedom()); + // TODO(phl): The prediction should probably be a fork of the psychohistory. + auto const psychohistory_last = psychohistory_->last(); + auto const prediction_begin = prediction_->Begin(); + if (prediction_->Empty() || + prediction_begin.time() != psychohistory_last.time() || + prediction_begin.degrees_of_freedom() != + psychohistory_last.degrees_of_freedom()) { + prediction_ = make_not_null_unique>(); + prediction_->Append(psychohistory_last.time(), + psychohistory_last.degrees_of_freedom()); + } FlowPrediction(last_time); } diff --git a/ksp_plugin_test/renderer_test.cpp b/ksp_plugin_test/renderer_test.cpp index e5f6091409..2c162486da 100644 --- a/ksp_plugin_test/renderer_test.cpp +++ b/ksp_plugin_test/renderer_test.cpp @@ -100,7 +100,10 @@ TEST_F(RendererTest, TargetVessel) { EXPECT_CALL(vessel, prediction()) .WillRepeatedly(ReturnRef(vessel_trajectory)); - renderer_.SetTargetVessel(&vessel, &celestial_, &ephemeris); + renderer_.SetTargetVessel(&vessel, + &celestial_, + &ephemeris, + /*prediction_last_time=*/[this]() { return t0_; }); EXPECT_TRUE(renderer_.HasTargetVessel()); EXPECT_THAT(renderer_.GetTargetVessel(), Ref(vessel)); MockVessel other_vessel; @@ -108,7 +111,10 @@ TEST_F(RendererTest, TargetVessel) { EXPECT_THAT(renderer_.GetTargetVessel(), Ref(vessel)); renderer_.ClearTargetVesselIf(&vessel); EXPECT_FALSE(renderer_.HasTargetVessel()); - renderer_.SetTargetVessel(&vessel, &celestial_, &ephemeris); + renderer_.SetTargetVessel(&vessel, + &celestial_, + &ephemeris, + /*prediction_last_time=*/[this]() { return t0_; }); renderer_.ClearTargetVessel(); EXPECT_FALSE(renderer_.HasTargetVessel()); } @@ -225,7 +231,10 @@ TEST_F(RendererTest, RenderBarycentricTrajectoryInPlottingWithTargetVessel) { Velocity()))); } - renderer_.SetTargetVessel(&vessel, &celestial_, &ephemeris); + renderer_.SetTargetVessel(&vessel, + &celestial_, + &ephemeris, + /*prediction_last_time=*/[this]() { return t0_; }); auto const rendered_trajectory = renderer_.RenderBarycentricTrajectoryInPlotting( trajectory_to_render.Begin(), From bb888328ac37a1d6127614c772db2e5a78791837 Mon Sep 17 00:00:00 2001 From: pleroy Date: Sun, 17 Sep 2017 17:29:35 +0200 Subject: [PATCH 3/8] Cleanup. --- ksp_plugin/renderer.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ksp_plugin/renderer.cpp b/ksp_plugin/renderer.cpp index 5be471690e..5c496fe7f2 100644 --- a/ksp_plugin/renderer.cpp +++ b/ksp_plugin/renderer.cpp @@ -118,9 +118,7 @@ Renderer::RenderBarycentricTrajectoryInPlotting( break; } } - trajectory->Append( - t, - BarycentricToPlotting(it.time())(it.degrees_of_freedom())); + trajectory->Append(t, BarycentricToPlotting(t)(it.degrees_of_freedom())); } return trajectory; } From b4b7059dfde6e638cb718b51179a236a4cba984e Mon Sep 17 00:00:00 2001 From: pleroy Date: Sun, 17 Sep 2017 18:45:11 +0200 Subject: [PATCH 4/8] Lint. --- ksp_plugin/renderer.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ksp_plugin/renderer.cpp b/ksp_plugin/renderer.cpp index 5c496fe7f2..c1eb927917 100644 --- a/ksp_plugin/renderer.cpp +++ b/ksp_plugin/renderer.cpp @@ -1,6 +1,8 @@  #include "ksp_plugin/renderer.hpp" +#include + #include "geometry/grassmann.hpp" #include "geometry/named_quantities.hpp" #include "physics/apsides.hpp" From c5f59474902bbfc3334134c08ebe9da5221f868e Mon Sep 17 00:00:00 2001 From: pleroy Date: Sun, 17 Sep 2017 23:19:00 +0200 Subject: [PATCH 5/8] Remove prediction length. --- ksp_plugin/interface.cpp | 8 -------- ksp_plugin/plugin.cpp | 15 +++----------- ksp_plugin/plugin.hpp | 3 --- ksp_plugin/renderer.cpp | 22 ++++++++++----------- ksp_plugin/renderer.hpp | 10 +++------- ksp_plugin_adapter/ksp_plugin_adapter.cs | 1 - ksp_plugin_test/interface_test.cpp | 5 ----- ksp_plugin_test/mock_plugin.hpp | 2 -- ksp_plugin_test/plugin_integration_test.cpp | 5 ++--- ksp_plugin_test/plugin_test.cpp | 4 ++-- ksp_plugin_test/renderer_test.cpp | 15 +++----------- serialization/journal.proto | 11 ----------- 12 files changed, 23 insertions(+), 78 deletions(-) diff --git a/ksp_plugin/interface.cpp b/ksp_plugin/interface.cpp index e57b948a27..a2f1181548 100644 --- a/ksp_plugin/interface.cpp +++ b/ksp_plugin/interface.cpp @@ -792,14 +792,6 @@ void principia__SetPartApparentDegreesOfFreedom(Plugin* const plugin, return m.Return(); } -void principia__SetPredictionLength(Plugin* const plugin, - double const t) { - journal::Method m({plugin, t}); - CHECK_NOTNULL(plugin); - plugin->SetPredictionLength(t * Second); - return m.Return(); -} - // Make it so that all log messages of at least |min_severity| are logged to // stderr (in addition to logging to the usual log file(s)). void principia__SetStderrLogging(int const min_severity) { diff --git a/ksp_plugin/plugin.cpp b/ksp_plugin/plugin.cpp index 058724dfcc..69c31ec59f 100644 --- a/ksp_plugin/plugin.cpp +++ b/ksp_plugin/plugin.cpp @@ -51,6 +51,7 @@ namespace principia { namespace ksp_plugin { namespace internal_plugin { +using astronomy::InfiniteFuture; using astronomy::ParseTT; using astronomy::KSPStockSystemFingerprint; using astronomy::KSPStabilizedSystemFingerprint; @@ -682,8 +683,7 @@ RelativeDegreesOfFreedom Plugin::CelestialFromParent( void Plugin::UpdatePrediction(GUID const& vessel_guid) const { CHECK(!initializing_); - FindOrDie(vessels_, vessel_guid)->UpdatePrediction( - current_time_ + prediction_length_); + FindOrDie(vessels_, vessel_guid)->UpdatePrediction(InfiniteFuture); } void Plugin::CreateFlightPlan(GUID const& vessel_guid, @@ -777,10 +777,6 @@ void Plugin::ComputeAndRenderNodes( PlanetariumRotation()); } -void Plugin::SetPredictionLength(Time const& t) { - prediction_length_ = t; -} - void Plugin::SetPredictionAdaptiveStepParameters( Ephemeris::AdaptiveStepParameters const& prediction_adaptive_step_parameters) { @@ -868,12 +864,7 @@ void Plugin::SetTargetVessel(GUID const& vessel_guid, not_null const celestial = FindOrDie(celestials_, reference_body_index).get(); not_null const vessel = FindOrDie(vessels_, vessel_guid).get(); - renderer_->SetTargetVessel(vessel, - celestial, - ephemeris_.get(), - /*prediction_last_time=*/[this]() { - return current_time_ + prediction_length_; - }); + renderer_->SetTargetVessel(vessel, celestial, ephemeris_.get()); } std::unique_ptr> Plugin::NavballFrameField( diff --git a/ksp_plugin/plugin.hpp b/ksp_plugin/plugin.hpp index 3bff77a19f..30deca7035 100644 --- a/ksp_plugin/plugin.hpp +++ b/ksp_plugin/plugin.hpp @@ -312,8 +312,6 @@ class Plugin { std::unique_ptr>& ascending, std::unique_ptr>& descending) const; - virtual void SetPredictionLength(Time const& t); - virtual void SetPredictionAdaptiveStepParameters( Ephemeris::AdaptiveStepParameters const& prediction_adaptive_step_parameters); @@ -451,7 +449,6 @@ class Plugin { Ephemeris::FixedStepParameters history_parameters_; Ephemeris::AdaptiveStepParameters prolongation_parameters_; Ephemeris::AdaptiveStepParameters prediction_parameters_; - Time prediction_length_ = 1 * Hour; // The thread pool for advancing vessels. ThreadPool vessel_thread_pool_; diff --git a/ksp_plugin/renderer.cpp b/ksp_plugin/renderer.cpp index c1eb927917..db04016e26 100644 --- a/ksp_plugin/renderer.cpp +++ b/ksp_plugin/renderer.cpp @@ -41,13 +41,11 @@ not_null Renderer::GetPlottingFrame() const { void Renderer::SetTargetVessel( not_null const vessel, not_null const celestial, - not_null const*> const ephemeris, - std::function prediction_last_time) { + not_null const*> const ephemeris) { if (!target_ || target_->vessel != vessel || target_->celestial != celestial) { - target_.emplace( - vessel, celestial, ephemeris, std::move(prediction_last_time)); + target_.emplace(vessel, celestial, ephemeris); } } @@ -78,8 +76,7 @@ Vessel const& Renderer::GetTargetVessel() const { DiscreteTrajectory const& Renderer::GetTargetVesselPrediction( Instant const& time) const { CHECK(target_); - target_->vessel->UpdatePrediction( - std::max(time, target_->prediction_last_time())); + target_->vessel->UpdatePrediction(time); // The prediction may not have been prolonged to |time| if we are near a // singularity. CHECK_LE(time, target_->vessel->prediction().last().time()); @@ -109,11 +106,14 @@ Renderer::RenderBarycentricTrajectoryInPlotting( DiscreteTrajectory::Iterator const& begin, DiscreteTrajectory::Iterator const& end) const { auto trajectory = make_not_null_unique>(); + if (target_ && begin != end) { + auto last = end; + --last; + target_->vessel->UpdatePrediction(last.time()); + } for (auto it = begin; it != end; ++it) { Instant const& t = it.time(); if (target_) { - target_->vessel->UpdatePrediction( - std::max(t, target_->prediction_last_time())); if (t < target_->vessel->prediction().t_min()) { continue; } else if (t > target_->vessel->prediction().t_max()) { @@ -301,8 +301,7 @@ not_null> Renderer::ReadFromMessage( Renderer::Target::Target( not_null const vessel, not_null const celestial, - not_null const*> const ephemeris, - std::function prediction_last_time) + not_null const*> const ephemeris) : vessel(vessel), celestial(celestial), target_frame( @@ -310,8 +309,7 @@ Renderer::Target::Target( BodyCentredBodyDirectionDynamicFrame>( ephemeris, [this]() -> auto& { return this->vessel->prediction(); }, - celestial->body())), - prediction_last_time(std::move(prediction_last_time)) {} + celestial->body())) {} not_null Renderer::GetPlottingFrame( Instant const& time) const { diff --git a/ksp_plugin/renderer.hpp b/ksp_plugin/renderer.hpp index f79471cf04..f36367e909 100644 --- a/ksp_plugin/renderer.hpp +++ b/ksp_plugin/renderer.hpp @@ -49,13 +49,11 @@ class Renderer { virtual not_null GetPlottingFrame() const; // Overrides the current plotting frame with one that is centred on the given - // |vessel|. The time returned by |prediction_last_time| is used to prolong - // the prediction of the |vessel| when needed. + // |vessel|. virtual void SetTargetVessel( not_null vessel, not_null celestial, - not_null const*> ephemeris, - std::function prediction_last_time); + not_null const*> ephemeris); // Reverts to frame last set by |SetPlottingFrame|. The second version only // has an effect if the given |vessel| is the current target vessel. @@ -175,12 +173,10 @@ class Renderer { struct Target { Target(not_null vessel, not_null celestial, - not_null const*> ephemeris, - std::function prediction_last_time); + not_null const*> ephemeris); not_null const vessel; not_null const celestial; not_null> const target_frame; - std::function prediction_last_time; }; // Returns a plotting frame suitable for evaluation at |time|, possibly by diff --git a/ksp_plugin_adapter/ksp_plugin_adapter.cs b/ksp_plugin_adapter/ksp_plugin_adapter.cs index 27ab5a8c56..1da7f9b3dd 100644 --- a/ksp_plugin_adapter/ksp_plugin_adapter.cs +++ b/ksp_plugin_adapter/ksp_plugin_adapter.cs @@ -936,7 +936,6 @@ private void FixedUpdate() { prediction_length_tolerance_index_]}; plugin_.VesselSetPredictionAdaptiveStepParameters( main_vessel.id.ToString(), adaptive_step_parameters); - plugin_.SetPredictionLength(double.PositiveInfinity); plugin_.UpdatePrediction(main_vessel.id.ToString()); string target_id = FlightGlobals.fetch.VesselTarget?.GetVessel()?.id.ToString(); diff --git a/ksp_plugin_test/interface_test.cpp b/ksp_plugin_test/interface_test.cpp index d527fd63e4..3180f409af 100644 --- a/ksp_plugin_test/interface_test.cpp +++ b/ksp_plugin_test/interface_test.cpp @@ -485,11 +485,6 @@ TEST_F(InterfaceTest, NewNavigationFrame) { EXPECT_EQ(mock_navigation_frame, navigation_frame.get()); } -TEST_F(InterfaceTest, PredictionGettersAndSetters) { - EXPECT_CALL(*plugin_, SetPredictionLength(42 * Second)); - principia__SetPredictionLength(plugin_.get(), 42); -} - TEST_F(InterfaceTest, NavballOrientation) { StrictMock>* const mock_navigation_frame = diff --git a/ksp_plugin_test/mock_plugin.hpp b/ksp_plugin_test/mock_plugin.hpp index 80540b1124..bd4f404f15 100644 --- a/ksp_plugin_test/mock_plugin.hpp +++ b/ksp_plugin_test/mock_plugin.hpp @@ -68,8 +68,6 @@ class MockPlugin : public Plugin { Instant const& final_time, Mass const& initial_mass)); - MOCK_METHOD1(SetPredictionLength, void(Time const& t)); - MOCK_METHOD1(SetPredictionAdaptiveStepParameters, void(Ephemeris::AdaptiveStepParameters const& prediction_adaptive_step_parameters)); diff --git a/ksp_plugin_test/plugin_integration_test.cpp b/ksp_plugin_test/plugin_integration_test.cpp index 4e46e47e56..ed272a41de 100644 --- a/ksp_plugin_test/plugin_integration_test.cpp +++ b/ksp_plugin_test/plugin_integration_test.cpp @@ -704,10 +704,9 @@ TEST_F(PluginIntegrationTest, Prediction) { plugin.renderer().SetPlottingFrame( plugin.NewBodyCentredNonRotatingNavigationFrame(celestial)); - plugin.SetPredictionLength(2 * π * Second); Ephemeris::AdaptiveStepParameters adaptive_step_parameters( DormandElMikkawyPrince1986RKN434FM>(), - /*max_steps=*/1000, + /*max_steps=*/14, /*length_integration_tolerance=*/1 * Milli(Metre), /*speed_integration_tolerance=*/1 * Milli(Metre) / Second); plugin.SetPredictionAdaptiveStepParameters(adaptive_step_parameters); @@ -739,7 +738,7 @@ TEST_F(PluginIntegrationTest, Prediction) { AbsoluteError(rendered_prediction->last().degrees_of_freedom().position(), Displacement({1 * Metre, 0 * Metre, 0 * Metre}) + World::origin), - AllOf(Gt(2 * Milli(Metre)), Lt(3 * Milli(Metre)))); + AllOf(Gt(31 * Milli(Metre)), Lt(32 * Milli(Metre)))); } } // namespace internal_plugin diff --git a/ksp_plugin_test/plugin_test.cpp b/ksp_plugin_test/plugin_test.cpp index 9037c33628..aca49f0ab4 100644 --- a/ksp_plugin_test/plugin_test.cpp +++ b/ksp_plugin_test/plugin_test.cpp @@ -850,13 +850,13 @@ TEST_F(PluginTest, ForgetAllHistoriesBeforeAfterPredictionFork) { .WillOnce(DoAll(SaveArg<0>(&trajectories), Return(ByMove(std::move(instance))))); EXPECT_CALL(plugin_->mock_ephemeris(), t_max()) - .WillRepeatedly(Return(Instant())); + .WillRepeatedly(Return(Instant() + 12 * Hour)); EXPECT_CALL(plugin_->mock_ephemeris(), empty()).WillRepeatedly(Return(false)); EXPECT_CALL(plugin_->mock_ephemeris(), trajectory(_)) .WillOnce(Return(plugin_->trajectory(SolarSystemFactory::Sun))); EXPECT_CALL(plugin_->mock_ephemeris(), Prolong(_)).Times(AnyNumber()); EXPECT_CALL(plugin_->mock_ephemeris(), FlowWithAdaptiveStep(_, _, _, _, _, _)) - .WillRepeatedly(DoAll(AppendToDiscreteTrajectory(dof), Return(true))); + .WillRepeatedly(DoAll(AppendToDiscreteTrajectory(dof), Return(false))); EXPECT_CALL(plugin_->mock_ephemeris(), FlowWithFixedStep(_, _)) .WillRepeatedly(AppendToDiscreteTrajectory2(&trajectories[0], dof)); EXPECT_CALL(plugin_->mock_ephemeris(), planetary_integrator()) diff --git a/ksp_plugin_test/renderer_test.cpp b/ksp_plugin_test/renderer_test.cpp index 2c162486da..e5f6091409 100644 --- a/ksp_plugin_test/renderer_test.cpp +++ b/ksp_plugin_test/renderer_test.cpp @@ -100,10 +100,7 @@ TEST_F(RendererTest, TargetVessel) { EXPECT_CALL(vessel, prediction()) .WillRepeatedly(ReturnRef(vessel_trajectory)); - renderer_.SetTargetVessel(&vessel, - &celestial_, - &ephemeris, - /*prediction_last_time=*/[this]() { return t0_; }); + renderer_.SetTargetVessel(&vessel, &celestial_, &ephemeris); EXPECT_TRUE(renderer_.HasTargetVessel()); EXPECT_THAT(renderer_.GetTargetVessel(), Ref(vessel)); MockVessel other_vessel; @@ -111,10 +108,7 @@ TEST_F(RendererTest, TargetVessel) { EXPECT_THAT(renderer_.GetTargetVessel(), Ref(vessel)); renderer_.ClearTargetVesselIf(&vessel); EXPECT_FALSE(renderer_.HasTargetVessel()); - renderer_.SetTargetVessel(&vessel, - &celestial_, - &ephemeris, - /*prediction_last_time=*/[this]() { return t0_; }); + renderer_.SetTargetVessel(&vessel, &celestial_, &ephemeris); renderer_.ClearTargetVessel(); EXPECT_FALSE(renderer_.HasTargetVessel()); } @@ -231,10 +225,7 @@ TEST_F(RendererTest, RenderBarycentricTrajectoryInPlottingWithTargetVessel) { Velocity()))); } - renderer_.SetTargetVessel(&vessel, - &celestial_, - &ephemeris, - /*prediction_last_time=*/[this]() { return t0_; }); + renderer_.SetTargetVessel(&vessel, &celestial_, &ephemeris); auto const rendered_trajectory = renderer_.RenderBarycentricTrajectoryInPlotting( trajectory_to_render.Begin(), diff --git a/serialization/journal.proto b/serialization/journal.proto index 915114cae1..d078d259e5 100644 --- a/serialization/journal.proto +++ b/serialization/journal.proto @@ -1500,17 +1500,6 @@ message SetPlottingFrame { optional Out out = 2; } -message SetPredictionLength { - extend Method { - optional SetPredictionLength extension = 5042; - } - message In { - required fixed64 plugin = 1 [(pointer_to) = "Plugin", (is_subject) = true]; - required double t = 2; - } - optional In in = 1; -} - message SetTargetVessel { extend Method { optional SetTargetVessel extension = 5121; From 3d86da5798d99f2ec17b6887f1b85725b3402dfc Mon Sep 17 00:00:00 2001 From: pleroy Date: Wed, 20 Sep 2017 21:54:40 +0200 Subject: [PATCH 6/8] Set the parameters for the target vessel. --- ksp_plugin/interface_vessel.cpp | 6 +++--- ksp_plugin/plugin.cpp | 16 ++++++++++++++++ ksp_plugin/plugin.hpp | 5 +++++ 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/ksp_plugin/interface_vessel.cpp b/ksp_plugin/interface_vessel.cpp index 1ae9b40763..acb42c18bf 100644 --- a/ksp_plugin/interface_vessel.cpp +++ b/ksp_plugin/interface_vessel.cpp @@ -70,9 +70,9 @@ void principia__VesselSetPredictionAdaptiveStepParameters( journal::Method m( {plugin, vessel_guid, adaptive_step_parameters}); CHECK_NOTNULL(plugin); - plugin->GetVessel(vessel_guid) - ->set_prediction_adaptive_step_parameters( - FromAdaptiveStepParameters(adaptive_step_parameters)); + plugin->SetPredictionAdaptiveStepParameters( + vessel_guid, + FromAdaptiveStepParameters(adaptive_step_parameters)); return m.Return(); } diff --git a/ksp_plugin/plugin.cpp b/ksp_plugin/plugin.cpp index 69c31ec59f..3ac8f82849 100644 --- a/ksp_plugin/plugin.cpp +++ b/ksp_plugin/plugin.cpp @@ -681,6 +681,22 @@ RelativeDegreesOfFreedom Plugin::CelestialFromParent( return result; } +void Plugin::SetPredictionAdaptiveStepParameters( + GUID const& vessel_guid, + Ephemeris::AdaptiveStepParameters const& + prediction_adaptive_step_parameters) const { + // If there is a target vessel, it is integrated with the same parameters as + // the given (current) vessel. This makes it possible to plot things like the + // closest approaches. + if (renderer_->HasTargetVessel()) { + renderer_->GetTargetVessel().set_prediction_adaptive_step_parameters( + prediction_adaptive_step_parameters); + } + FindOrDie(vessels_, vessel_guid) + ->set_prediction_adaptive_step_parameters( + prediction_adaptive_step_parameters); +} + void Plugin::UpdatePrediction(GUID const& vessel_guid) const { CHECK(!initializing_); FindOrDie(vessels_, vessel_guid)->UpdatePrediction(InfiniteFuture); diff --git a/ksp_plugin/plugin.hpp b/ksp_plugin/plugin.hpp index 30deca7035..417f160dc3 100644 --- a/ksp_plugin/plugin.hpp +++ b/ksp_plugin/plugin.hpp @@ -278,6 +278,11 @@ class Plugin { virtual RelativeDegreesOfFreedom CelestialFromParent( Index celestial_index) const; + virtual void SetPredictionAdaptiveStepParameters( + GUID const& vessel_guid, + Ephemeris::AdaptiveStepParameters const& + prediction_adaptive_step_parameters) const; + // Updates the prediction for the vessel with guid |vessel_guid|. void UpdatePrediction(GUID const& vessel_guid) const; From fef915bf2c0814ed3832f06c9165d80bd58cd98a Mon Sep 17 00:00:00 2001 From: pleroy Date: Thu, 21 Sep 2017 20:57:47 +0200 Subject: [PATCH 7/8] After egg's review. --- ksp_plugin/plugin.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ksp_plugin/plugin.cpp b/ksp_plugin/plugin.cpp index 3ac8f82849..815cb66a7c 100644 --- a/ksp_plugin/plugin.cpp +++ b/ksp_plugin/plugin.cpp @@ -686,8 +686,8 @@ void Plugin::SetPredictionAdaptiveStepParameters( Ephemeris::AdaptiveStepParameters const& prediction_adaptive_step_parameters) const { // If there is a target vessel, it is integrated with the same parameters as - // the given (current) vessel. This makes it possible to plot things like the - // closest approaches. + // the given (current) vessel. This makes it possible to the prediction of + // the current vessel. if (renderer_->HasTargetVessel()) { renderer_->GetTargetVessel().set_prediction_adaptive_step_parameters( prediction_adaptive_step_parameters); From fe1321033bcd9cd20184746bf86ed8fa3dbb8403 Mon Sep 17 00:00:00 2001 From: pleroy Date: Thu, 21 Sep 2017 20:58:31 +0200 Subject: [PATCH 8/8] Typo. --- ksp_plugin/plugin.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ksp_plugin/plugin.cpp b/ksp_plugin/plugin.cpp index 815cb66a7c..f3722ac340 100644 --- a/ksp_plugin/plugin.cpp +++ b/ksp_plugin/plugin.cpp @@ -686,8 +686,8 @@ void Plugin::SetPredictionAdaptiveStepParameters( Ephemeris::AdaptiveStepParameters const& prediction_adaptive_step_parameters) const { // If there is a target vessel, it is integrated with the same parameters as - // the given (current) vessel. This makes it possible to the prediction of - // the current vessel. + // the given (current) vessel. This makes it possible to plot the prediction + // of the current vessel. if (renderer_->HasTargetVessel()) { renderer_->GetTargetVessel().set_prediction_adaptive_step_parameters( prediction_adaptive_step_parameters);