Do not cache actor parent in the actor

LibCarla/source/carla/client/ActorList.cpp

@@ -23,7 +23,7 @@ namespace client {
   SharedPtr<Actor> ActorList::Find(const ActorId actor_id) const {
     for (auto &actor : _actors) {
       if (actor_id == actor.GetId()) {
-        return actor.Get(_episode, shared_from_this());
+        return actor.Get(_episode);
       }
     }
     return nullptr;

LibCarla/source/carla/client/ActorList.h

@@ -21,7 +21,7 @@ namespace client {
     template <typename It>
     auto MakeIterator(It it) const {
       return boost::make_transform_iterator(it, [this](auto &v) {
-        return v.Get(_episode, shared_from_this());
+        return v.Get(_episode);
       });
     }
 
@@ -34,11 +34,11 @@ namespace client {
     SharedPtr<ActorList> Filter(const std::string &wildcard_pattern) const;
 
     SharedPtr<Actor> operator[](size_t pos) const {
-      return _actors[pos].Get(_episode, shared_from_this());
+      return _actors[pos].Get(_episode);
     }
 
     SharedPtr<Actor> at(size_t pos) const {
-      return _actors.at(pos).Get(_episode, shared_from_this());
+      return _actors.at(pos).Get(_episode);
     }
 
     auto begin() const {

LibCarla/source/carla/client/World.cpp

@@ -45,6 +45,14 @@ namespace client {
     _episode.Lock()->SetWeatherParameters(weather);
   }
 
+  SharedPtr<Actor> World::GetActor(ActorId id) const {
+    auto simulator = _episode.Lock();
+    auto description = simulator->GetActorById(id);
+    return description.has_value() ?
+        simulator->MakeActor(std::move(*description)) :
+        nullptr;
+  }
+
   SharedPtr<ActorList> World::GetActors() const {
     return SharedPtr<ActorList>{new ActorList{
                                   _episode,

LibCarla/source/carla/client/World.h

@@ -64,6 +64,9 @@ namespace client {
     /// Change the weather in the simulation.
     void SetWeather(const rpc::WeatherParameters &weather);
 
+    /// Find actor by id, return nullptr if not found.
+    SharedPtr<Actor> GetActor(ActorId id) const;
+
     /// Return a list with all the actors currently present in the world.
     SharedPtr<ActorList> GetActors() const;
 

LibCarla/source/carla/client/detail/ActorFactory.cpp

@@ -70,9 +70,8 @@ namespace detail {
   SharedPtr<Actor> ActorFactory::MakeActor(
       EpisodeProxy episode,
       rpc::Actor description,
-      SharedPtr<Actor> parent,
       GarbageCollectionPolicy gc) {
-    auto init = ActorInitializer{description, episode, parent};
+    auto init = ActorInitializer{description, episode};
     if (description.description.id == "sensor.other.lane_invasion") { /// @todo
       return MakeActorImpl<LaneInvasionSensor>(std::move(init), gc);
     } else if (description.description.id == "sensor.other.gnss") { /// @todo

LibCarla/source/carla/client/detail/ActorFactory.h

@@ -24,13 +24,14 @@ namespace detail {
     /// Create an Actor based on the provided @a actor_description. @a episode
     /// must point to the episode in which the actor is living.
     ///
+    /// Do not call this class directly, use Simulator::MakeActor.
+    ///
     /// If @a garbage_collection_policy is GarbageCollectionPolicy::Enabled, the
     /// shared pointer returned is provided with a custom deleter that calls
     /// Destroy() on the actor.
     static SharedPtr<Actor> MakeActor(
         EpisodeProxy episode,
         rpc::Actor actor_description,
-        SharedPtr<Actor> parent,
         GarbageCollectionPolicy garbage_collection_policy);
   };
 

LibCarla/source/carla/client/detail/ActorState.cpp

@@ -15,11 +15,9 @@ namespace detail {
 
   ActorState::ActorState(
       rpc::Actor description,
-      EpisodeProxy episode,
-      SharedPtr<Actor> parent)
+      EpisodeProxy episode)
     : _description(std::move(description)),
       _episode(std::move(episode)),
-      _parent(std::move(parent)),
       _display_id([](const auto &desc) {
         using namespace std::string_literals;
         return
@@ -30,6 +28,11 @@ namespace detail {
       _attributes(_description.description.attributes.begin(), _description.description.attributes.end())
   {}
 
+  SharedPtr<Actor> ActorState::GetParent() const {
+    auto parent_id = GetParentId();
+    return parent_id != 0u ? GetWorld().GetActor(parent_id) : nullptr;
+  }
+
 } // namespace detail
 } // namespace client
 } // namespace carla

LibCarla/source/carla/client/detail/ActorState.h

@@ -22,7 +22,7 @@ namespace detail {
   class ActorState : private MovableNonCopyable {
   public:
 
-    auto GetId() const {
+    ActorId GetId() const {
       return _description.id;
     }
 
@@ -34,13 +34,15 @@ namespace detail {
       return _display_id;
     }
 
+    ActorId GetParentId() const {
+      return _description.parent_id;
+    }
+
     const std::vector<uint8_t> &GetSemanticTags() const {
       return _description.semantic_tags;
     }
 
-    SharedPtr<Actor> GetParent() const {
-      return _parent;
-    }
+    SharedPtr<Actor> GetParent() const;
 
     World GetWorld() const {
       return World{_episode};
@@ -73,17 +75,12 @@ namespace detail {
 
     friend class Simulator;
 
-    ActorState(
-        rpc::Actor description,
-        EpisodeProxy episode,
-        SharedPtr<Actor> parent);
+    explicit ActorState(rpc::Actor description, EpisodeProxy episode);
 
     rpc::Actor _description;
 
     EpisodeProxy _episode;
 
-    SharedPtr<Actor> _parent;
-
     std::string _display_id;
 
     std::vector<ActorAttributeValue> _attributes;

LibCarla/source/carla/client/detail/ActorVariant.cpp

@@ -13,18 +13,10 @@ namespace carla {
 namespace client {
 namespace detail {
 
-  void ActorVariant::MakeActor(EpisodeProxy episode, SharedPtr<const client::ActorList> actor_list) const {
-    auto const parent_id = GetParentId();
-    SharedPtr<client::Actor> parent = nullptr;
-    if ((actor_list != nullptr) && (parent_id != 0)) {
-      // In case we have an actor list as context, we are able to actually
-      // create the parent actor.
-      parent = actor_list->Find(parent_id);
-    }
+  void ActorVariant::MakeActor(EpisodeProxy episode) const {
     _value = detail::ActorFactory::MakeActor(
         episode,
         boost::get<rpc::Actor>(std::move(_value)),
-        parent,
         GarbageCollectionPolicy::Disabled);
   }
 

LibCarla/source/carla/client/detail/ActorVariant.h

@@ -38,9 +38,9 @@ namespace detail {
       return *this;
     }
 
-    SharedPtr<client::Actor> Get(EpisodeProxy episode, SharedPtr<const client::ActorList> actor_list = nullptr) const {
+    SharedPtr<client::Actor> Get(EpisodeProxy episode) const {
       if (_value.which() == 0u) {
-        MakeActor(episode, actor_list);
+        MakeActor(episode);
       }
       DEBUG_ASSERT(_value.which() == 1u);
       return boost::get<SharedPtr<client::Actor>>(_value);
@@ -81,7 +81,7 @@ namespace detail {
       }
     };
 
-    void MakeActor(EpisodeProxy episode, SharedPtr<const client::ActorList> actor_list) const;
+    void MakeActor(EpisodeProxy episode) const;
 
     mutable boost::variant<rpc::Actor, SharedPtr<client::Actor>> _value;
   };

LibCarla/source/carla/client/detail/CachedActorList.h

@@ -42,6 +42,10 @@ namespace detail {
     template <typename RangeT>
     std::vector<ActorId> GetMissingIds(const RangeT &range) const;
 
+    /// Retrieve the actor matching @a id, or empty optional if actor is not
+    /// cached.
+    boost::optional<rpc::Actor> GetActorById(ActorId id) const;
+
     /// Retrieve the actors matching the ids in @a range.
     template <typename RangeT>
     std::vector<rpc::Actor> GetActorsById(const RangeT &range) const;
@@ -89,6 +93,15 @@ namespace detail {
     return result;
   }
 
+  inline boost::optional<rpc::Actor> CachedActorList::GetActorById(ActorId id) const {
+    std::lock_guard<std::mutex> lock(_mutex);
+    auto it = _actors.find(id);
+    if (it != _actors.end()) {
+      return it->second;
+    }
+    return boost::none;
+  }
+
   template <typename RangeT>
   inline std::vector<rpc::Actor> CachedActorList::GetActorsById(const RangeT &range) const {
     std::vector<rpc::Actor> result;

LibCarla/source/carla/client/detail/Episode.cpp

@@ -73,6 +73,18 @@ namespace detail {
     });
   }
 
+  boost::optional<rpc::Actor> Episode::GetActorById(ActorId id) {
+    auto actor = _actors.GetActorById(id);
+    if (!actor.has_value()) {
+      auto actor_list = _client.GetActorsById({id});
+      if (!actor_list.empty()) {
+        actor = std::move(actor_list.front());
+        _actors.Insert(*actor);
+      }
+    }
+    return actor;
+  }
+
   std::vector<rpc::Actor> Episode::GetActorsById(const std::vector<ActorId> &actor_ids) {
     return GetActorsById_Impl(_client, _actors, actor_ids);
   }

LibCarla/source/carla/client/detail/Episode.h

@@ -51,6 +51,8 @@ namespace detail {
       _actors.Insert(std::move(actor));
     }
 
+    boost::optional<rpc::Actor> GetActorById(ActorId id);
+
     std::vector<rpc::Actor> GetActorsById(const std::vector<ActorId> &actor_ids);
 
     std::vector<rpc::Actor> GetActors();

LibCarla/source/carla/client/detail/Simulator.cpp

@@ -13,7 +13,6 @@
 #include "carla/client/Map.h"
 #include "carla/client/Sensor.h"
 #include "carla/client/TimeoutException.h"
-#include "carla/client/detail/ActorFactory.h"
 #include "carla/sensor/Deserializer.h"
 
 #include <exception>
@@ -115,11 +114,7 @@ namespace detail {
   }
 
   SharedPtr<Actor> Simulator::GetSpectator() {
-    return ActorFactory::MakeActor(
-        GetCurrentEpisode(),
-        _client.GetSpectator(),
-        nullptr,
-        GarbageCollectionPolicy::Disabled);
+    return MakeActor(_client.GetSpectator());
   }
 
   // ===========================================================================
@@ -147,8 +142,7 @@ namespace detail {
     DEBUG_ASSERT(_episode != nullptr);
     _episode->RegisterActor(actor);
     const auto gca = (gc == GarbageCollectionPolicy::Inherit ? _gc_policy : gc);
-    auto parent_ptr = parent != nullptr ? parent->shared_from_this() : SharedPtr<Actor>();
-    auto result = ActorFactory::MakeActor(GetCurrentEpisode(), actor, parent_ptr, gca);
+    auto result = ActorFactory::MakeActor(GetCurrentEpisode(), actor, gca);
     log_debug(
         result->GetDisplayId(),
         "created",

LibCarla/source/carla/client/detail/Simulator.h

@@ -14,6 +14,7 @@
 #include "carla/client/TrafficLight.h"
 #include "carla/client/Vehicle.h"
 #include "carla/client/Walker.h"
+#include "carla/client/detail/ActorFactory.h"
 #include "carla/client/detail/Client.h"
 #include "carla/client/detail/Episode.h"
 #include "carla/client/detail/EpisodeProxy.h"
@@ -171,6 +172,11 @@ namespace detail {
     // =========================================================================
     /// @{
 
+    boost::optional<rpc::Actor> GetActorById(ActorId id) const {
+      DEBUG_ASSERT(_episode != nullptr);
+      return _episode->GetActorById(id);
+    }
+
     std::vector<rpc::Actor> GetActorsById(const std::vector<ActorId> &actor_ids) const {
       DEBUG_ASSERT(_episode != nullptr);
       return _episode->GetActorsById(actor_ids);
@@ -181,9 +187,24 @@ namespace detail {
       return _episode->GetActors();
     }
 
+    /// Creates an actor instance out of a description of an existing actor.
+    /// Note that this does not spawn an actor.
+    ///
     /// If @a gc is GarbageCollectionPolicy::Enabled, the shared pointer
     /// returned is provided with a custom deleter that calls Destroy() on the
-    /// actor. If @gc is GarbageCollectionPolicy::Enabled, the default garbage
+    /// actor. This method does not support GarbageCollectionPolicy::Inherit.
+    SharedPtr<Actor> MakeActor(
+        rpc::Actor actor_description,
+        GarbageCollectionPolicy gc = GarbageCollectionPolicy::Disabled) {
+      RELEASE_ASSERT(gc != GarbageCollectionPolicy::Inherit);
+      return ActorFactory::MakeActor(GetCurrentEpisode(), std::move(actor_description), gc);
+    }
+
+    /// Spawns an actor into the simulation.
+    ///
+    /// If @a gc is GarbageCollectionPolicy::Enabled, the shared pointer
+    /// returned is provided with a custom deleter that calls Destroy() on the
+    /// actor. If @gc is GarbageCollectionPolicy::Inherit, the default garbage
     /// collection policy is used.
     SharedPtr<Actor> SpawnActor(
         const ActorBlueprint &blueprint,