FreeOrion

When I worked on https://github.com/freeorion/freeorion/issues/1897 I got some question about code to discuss:

1. Should GeneralizedLocation in universe/Pathfinder.cpp accept std::shared_ptr by value? It doesn't looks like it takes ownership on this parameter.
2. GeneralizedLocation outputs simple error to log if object is very wrong but actually it foreshadows segfaults. When I fixed segfault I also fix this error. Should GeneralizedLocation throw exception instead of logging error?
3. m_existing maps aren't specialized by type but appropriate main objects' maps are. Is it intended?
4. m_existing maps don't require to own objects because those objects are owned by appropriate main objects' maps. What if use std::weak_ptr?
5. I suppose in possible to change boost::remove_const<T>::type to std::remove_const<T>::type as the game uses c++11.

o01eg wrote: ↑Thu Sep 06, 2018 9:58 amShould GeneralizedLocation in universe/Pathfinder.cpp accept std::shared_ptr by value? It doesn't looks like it takes ownership on this parameter.

Could pass by const reference.

m_existing maps aren't specialized by type but appropriate main objects' maps are. Is it intended?

Probably. The main typed object maps are used to look up objects of a particular type and return a pointer of that type. The existing object lists aren't used like that so don't need to store more specialized UniverseObject pointers. That might be a circular rationale, though if they don't need to be specialized, then should they be?

m_existing maps don't require to own objects because those objects are owned by appropriate main objects' maps. What if use std::weak_ptr?

Seems like unnecessary overhead to convert the pointers. Aren't the sets maintained simultaneously anyway?

change boost::remove_const<T>::type to std::remove_const<T>::type as the game uses c++11.

Sounds reasonable.