Fix virtual train consist change callbacks being run before cargotype set

When instantiating virtual train from non-buildable template train

See: #402

Also fixes instantiating virtual train from train not refitting leading
vehicle.

src/build_vehicle_gui.cpp

@@ -1452,7 +1452,7 @@ struct BuildVehicleWindow : BuildVehicleWindowBase {
 				SavedRandomSeeds saved_seeds;
 				SaveRandomSeeds(&saved_seeds);
 				StringID err;
-				Train *t = CmdBuildVirtualRailVehicle(this->sel_engine, err, 0);
+				Train *t = BuildVirtualRailVehicle(this->sel_engine, err, 0, false);
 				if (t != nullptr) {
 					const CommandCost ret = CmdRefitVehicle(0, DC_QUERY_COST, t->index, cargo | (1 << 16), nullptr);
 					this->te.cost          = ret.GetCost();
@@ -2234,7 +2234,7 @@ struct BuildVehicleWindowTrainAdvanced final : BuildVehicleWindowBase {
 				SavedRandomSeeds saved_seeds;
 				SaveRandomSeeds(&saved_seeds);
 				StringID err;
-				Train *t = CmdBuildVirtualRailVehicle(state.sel_engine, err, 0);
+				Train *t = BuildVirtualRailVehicle(state.sel_engine, err, 0, false);
 				if (t != nullptr) {
 					const CommandCost ret = CmdRefitVehicle(0, DC_QUERY_COST, t->index, cargo | (1 << 16), nullptr);
 					state.te.cost          = ret.GetCost();

src/tbtr_template_vehicle.h

@@ -162,8 +162,12 @@ public:
 	inline bool IsFrontEngine() const { return HasBit(this->subtype, GVSF_FRONT); }
 	inline bool HasArticulatedPart() const { return this->Next() != nullptr && this->Next()->IsArticulatedPart(); }
 
+	inline bool IsEngine() const { return HasBit(this->subtype, GVSF_ENGINE); }
+	inline bool IsWagon() const { return HasBit(this->subtype, GVSF_WAGON); }
+
 	inline bool IsArticulatedPart() const { return HasBit(this->subtype, GVSF_ARTICULATED_PART); }
 	inline bool IsMultiheaded() const { return HasBit(this->subtype, GVSF_MULTIHEADED); }
+	inline bool IsRearDualheaded() const { return this->IsMultiheaded() && !this->IsEngine(); }
 
 	inline bool IsFreeWagonChain() const { return HasBit(this->subtype, GVSF_FREE_WAGON); }
 

src/train.h

@@ -507,7 +507,7 @@ struct TrainDecelerationStats {
 CommandCost CmdMoveRailVehicle(TileIndex, DoCommandFlag , uint32, uint32, const char *);
 CommandCost CmdMoveVirtualRailVehicle(TileIndex, DoCommandFlag, uint32, uint32, const char*);
 
-Train* CmdBuildVirtualRailVehicle(EngineID, StringID &error, uint32 user);
+Train* BuildVirtualRailVehicle(EngineID, StringID &error, uint32 user, bool no_consist_change);
 
 int GetTileMarginInFrontOfTrain(const Train *v, int x_pos, int y_pos);
 

src/train_cmd.cpp

@@ -6808,7 +6808,7 @@ int GetDisplayImageWidth(Train *t, Point *offset)
 	return t->gcache.cached_veh_length * reference_width / VEHICLE_LENGTH;
 }
 
-Train* CmdBuildVirtualRailWagon(const Engine *e, uint32 user)
+Train* CmdBuildVirtualRailWagon(const Engine *e, uint32 user, bool no_consist_change)
 {
 	const RailVehicleInfo *rvi = &e->u.rail;
 
@@ -6855,7 +6855,7 @@ Train* CmdBuildVirtualRailWagon(const Engine *e, uint32 user)
 
 	_new_vehicle_id = v->index;
 
-	v->UpdateViewport(true, false);
+	if (no_consist_change) return v;
 
 	v->First()->ConsistChanged(CCF_ARRANGE);
 
@@ -6866,7 +6866,7 @@ Train* CmdBuildVirtualRailWagon(const Engine *e, uint32 user)
 	return v;
 }
 
-Train* CmdBuildVirtualRailVehicle(EngineID eid, StringID &error, uint32 user)
+Train* BuildVirtualRailVehicle(EngineID eid, StringID &error, uint32 user, bool no_consist_change)
 {
 	const Engine *e = Engine::GetIfValid(eid);
 	if (e == nullptr || e->type != VEH_TRAIN) {
@@ -6885,7 +6885,7 @@ Train* CmdBuildVirtualRailVehicle(EngineID eid, StringID &error, uint32 user)
 	RegisterGameEvents(GEF_VIRT_TRAIN);
 
 	if (rvi->railveh_type == RAILVEH_WAGON) {
-		return CmdBuildVirtualRailWagon(e, user);
+		return CmdBuildVirtualRailWagon(e, user, no_consist_change);
 	}
 
 	Train *v = new Train();
@@ -6936,6 +6936,8 @@ Train* CmdBuildVirtualRailVehicle(EngineID eid, StringID &error, uint32 user)
 		train_part->SetVirtual();
 	}
 
+	if (no_consist_change) return v;
+
 	v->ConsistChanged(CCF_ARRANGE);
 
 	CheckConsistencyOfArticulatedVehicle(v);
@@ -6972,7 +6974,7 @@ CommandCost CmdBuildVirtualRailVehicle(TileIndex tile, DoCommandFlag flags, uint
 
 	if (should_execute) {
 		StringID err = INVALID_STRING_ID;
-		Train* train = CmdBuildVirtualRailVehicle(eid, err, p2);
+		Train* train = BuildVirtualRailVehicle(eid, err, p2, false);
 
 		if (train == nullptr) {
 			return_cmd_error(err);

src/vehicle_cmd.cpp

@@ -993,6 +993,47 @@ CommandCost CmdVirtualTrainFromTemplateVehicle(TileIndex tile, DoCommandFlag fla
 
 CommandCost CmdDeleteVirtualTrain(TileIndex tile, DoCommandFlag flags, uint32 p1, uint32 p2, const char *text);
 
+template <typename T>
+void UpdateNewVirtualTrainFromSource(Train *v, const T *src)
+{
+	struct helper {
+		static bool IsTrainPartReversed(const Train *src) { return HasBit(src->flags, VRF_REVERSE_DIRECTION); }
+		static bool IsTrainPartReversed(const TemplateVehicle *src) { return HasBit(src->ctrl_flags, TVCF_REVERSED); }
+		static const Train *GetTrainMultiheadOtherPart(const Train *src) { return src->other_multiheaded_part; }
+		static const TemplateVehicle *GetTrainMultiheadOtherPart(const TemplateVehicle *src) { return src; }
+	};
+
+	SB(v->flags, VRF_REVERSE_DIRECTION, 1, helper::IsTrainPartReversed(src) ? 1 : 0);
+
+	if (v->IsMultiheaded()) {
+		const T *other = helper::GetTrainMultiheadOtherPart(src);
+		/* For template vehicles, just use the front part, fix any discrepancy later */
+		v->other_multiheaded_part->cargo_type = other->cargo_type;
+		v->other_multiheaded_part->cargo_subtype = other->cargo_subtype;
+	}
+
+	while (true) {
+		v->cargo_type = src->cargo_type;
+		v->cargo_subtype = src->cargo_subtype;
+
+		if (v->HasArticulatedPart()) {
+			v = v->Next();
+		} else {
+			break;
+		}
+
+		if (src->HasArticulatedPart()) {
+			src = src->Next();
+		} else {
+			break;
+		}
+	}
+
+	v->First()->ConsistChanged(CCF_ARRANGE);
+	CheckConsistencyOfArticulatedVehicle(v);
+	InvalidateVehicleTickCaches();
+}
+
 Train* VirtualTrainFromTemplateVehicle(const TemplateVehicle* tv, StringID &err, uint32 user)
 {
 	CommandCost c;
@@ -1001,18 +1042,22 @@ Train* VirtualTrainFromTemplateVehicle(const TemplateVehicle* tv, StringID &err,
 
 	assert(tv->owner == _current_company);
 
-	head = CmdBuildVirtualRailVehicle(tv->engine_type, err, user);
+	head = BuildVirtualRailVehicle(tv->engine_type, err, user, true);
 	if (!head) return nullptr;
 
+	UpdateNewVirtualTrainFromSource(head, tv);
+
 	tail = head;
 	tv = tv->GetNextUnit();
 	while (tv) {
-		tmp = CmdBuildVirtualRailVehicle(tv->engine_type, err, user);
+		tmp = BuildVirtualRailVehicle(tv->engine_type, err, user, true);
 		if (!tmp) {
 			CmdDeleteVirtualTrain(INVALID_TILE, DC_EXEC, head->index, 0, nullptr);
 			return nullptr;
 		}
 
+		UpdateNewVirtualTrainFromSource(tmp, tv);
+
 		CmdMoveRailVehicle(INVALID_TILE, DC_EXEC, (1 << 21) | tmp->index, tail->index, 0);
 		tail = tmp;
 
@@ -1022,7 +1067,6 @@ Train* VirtualTrainFromTemplateVehicle(const TemplateVehicle* tv, StringID &err,
 	for (tv = tv_head, tmp = head; tv != nullptr && tmp != nullptr; tv = tv->Next(), tmp = tmp->Next()) {
 		tmp->cargo_type = tv->cargo_type;
 		tmp->cargo_subtype = tv->cargo_subtype;
-		SB(tmp->flags, VRF_REVERSE_DIRECTION, 1, HasBit(tv->ctrl_flags, TVCF_REVERSED) ? 1 : 0);
 	}
 
 	_new_vehicle_id = head->index;
@@ -1057,21 +1101,22 @@ CommandCost CmdVirtualTrainFromTrain(TileIndex tile, DoCommandFlag flags, uint32
 		Train *tmp, *head, *tail;
 		StringID err = INVALID_STRING_ID;
 
-		head = CmdBuildVirtualRailVehicle(train->engine_type, err, p2);
+		head = BuildVirtualRailVehicle(train->engine_type, err, p2, true);
 		if (!head) return_cmd_error(err);
 
+		UpdateNewVirtualTrainFromSource(head, train);
+
 		tail = head;
 		train = train->GetNextUnit();
 		while (train) {
-			tmp = CmdBuildVirtualRailVehicle(train->engine_type, err, p2);
+			tmp = BuildVirtualRailVehicle(train->engine_type, err, p2, true);
 			if (!tmp) {
 				CmdDeleteVirtualTrain(tile, flags, head->index, 0, nullptr);
 				return_cmd_error(err);
 			}
 
-			tmp->cargo_type = train->cargo_type;
+			UpdateNewVirtualTrainFromSource(tmp, train);
-			tmp->cargo_subtype = train->cargo_subtype;
+
-			SB(tmp->flags, VRF_REVERSE_DIRECTION, 1, HasBit(train->flags, VRF_REVERSE_DIRECTION) ? 1 : 0);
 			CmdMoveRailVehicle(0, DC_EXEC, (1 << 21) | tmp->index, tail->index, 0);
 			tail = tmp;