From 51847b5ebadb27aa2fa37a491daca0efdda19118 Mon Sep 17 00:00:00 2001
From: Filippo Gentile <filippogentile@disroot.org>
Date: Wed, 18 Sep 2024 19:09:28 +0200
Subject: [PATCH] TrainVehicleListItem: the missing piece

- Now closing world works as expected.
- Launching a second instance of server exits for TCP address but
doesn't crash for Trains

And finally...
- Deleting a rail Vehicle, removes it's entries in every Train it's part
of!

Previous attempts to fix crash failed like:

commit 105c51caef909b6b60f577aa5b2a371cacfd2fcc

TrainVehicleListItem: remove assert

It's wrong because on World close, detroying() is not called.
---
 server/src/train/train.cpp                |  1 +
 server/src/train/trainvehiclelistitem.cpp | 13 +++++++------
 server/src/train/trainvehiclelistitem.hpp |  2 +-
 3 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/server/src/train/train.cpp b/server/src/train/train.cpp
index f8b1aa336..b96568b1e 100644
--- a/server/src/train/train.cpp
+++ b/server/src/train/train.cpp
@@ -244,6 +244,7 @@ void Train::destroying()
   for(const auto& item : *vehicles)
   {
     item->vehicle->trains.removeInternal(self);
+    item->destroy();
   }
   m_world.trains->removeObject(self);
   IdObject::destroying();
diff --git a/server/src/train/trainvehiclelistitem.cpp b/server/src/train/trainvehiclelistitem.cpp
index 56a3b23e7..f8ee7a104 100644
--- a/server/src/train/trainvehiclelistitem.cpp
+++ b/server/src/train/trainvehiclelistitem.cpp
@@ -38,7 +38,7 @@ TrainVehicleListItem::TrainVehicleListItem(const std::shared_ptr<RailVehicle> &v
           return false; //Vehicle already in train
 
         if(vehicle)
-          disconnectVehicle(*vehicle);
+          disconnectVehicle();
 
         if(value)
           connectVehicle(*value);
@@ -68,9 +68,7 @@ std::string TrainVehicleListItem::getObjectId() const
 void TrainVehicleListItem::destroying()
 {
   //NOTE: we cannot normally set vehicle to nullptr (rejected by OnSet callback)
-  //So we mirror cleanup operations and manually reset value at end
-  if(vehicle)
-    disconnectVehicle(*vehicle.value());
+  disconnectVehicle();
   vehicle.setValueInternal(nullptr);
 
   Object::destroying();
@@ -104,11 +102,14 @@ void TrainVehicleListItem::connectVehicle(RailVehicle &object)
     [this]([[maybe_unused]] Object& obj)
     {
       assert(vehicle.value().get() == &obj);
-      vehicle = nullptr;
+      auto self = shared_ptr<TrainVehicleListItem>();
+
+      // Remove ourselves from Train vehicles
+      m_parent.removeObject(self);
     });
 }
 
-void TrainVehicleListItem::disconnectVehicle(RailVehicle &/*object*/)
+void TrainVehicleListItem::disconnectVehicle()
 {
   //Disconnect from previous vehicle
   m_vehiclePropertyChanged.disconnect();
diff --git a/server/src/train/trainvehiclelistitem.hpp b/server/src/train/trainvehiclelistitem.hpp
index 23ff1c9e5..157af3498 100644
--- a/server/src/train/trainvehiclelistitem.hpp
+++ b/server/src/train/trainvehiclelistitem.hpp
@@ -47,7 +47,7 @@ class TrainVehicleListItem final : public Object
     void loaded() final;
 
     void connectVehicle(RailVehicle &object);
-    void disconnectVehicle(RailVehicle &);
+    void disconnectVehicle();
 
   public:
     ObjectProperty<RailVehicle> vehicle;