(svn r13816) -Fix [FS#2150]: check for vehicle length changes outside a depot (callback 0x11) and give a warning about that

12 files changed, 182 insertions, 24 deletions

diff --git a/readme.txt b/readme.txt
index 4395ad72c..851d7886e 100644
--- a/readme.txt
+++ b/readme.txt
@@ -225,6 +225,7 @@ Information logged:
 * Running a modified OTTD build
 * Changing patch settings affecting NewGRF behaviour (non-networksafe patches)
 * Changing landscape (by cheat)
+* Triggering NewGRF bugs
 
 No personal information is stored.
 
diff --git a/src/autoreplace_cmd.cpp b/src/autoreplace_cmd.cpp
index c2a3eaaea..5d728c0c5 100644
--- a/src/autoreplace_cmd.cpp
+++ b/src/autoreplace_cmd.cpp
@@ -57,7 +57,7 @@ static void MoveVehicleCargo(Vehicle *dest, Vehicle *source)
 	 * the complete train, which is without the weight of cargo we just
 	 * moved back into some (of the) new wagon(s).
 	 */
-	if (dest->type == VEH_TRAIN) TrainConsistChanged(dest->First());
+	if (dest->type == VEH_TRAIN) TrainConsistChanged(dest->First(), true);
 }
 
 static bool VerifyAutoreplaceRefitForOrders(const Vehicle *v, const EngineID engine_type)
diff --git a/src/gamelog.cpp b/src/gamelog.cpp
index f612f8951..029f43b70 100644
--- a/src/gamelog.cpp
+++ b/src/gamelog.cpp
@@ -39,6 +39,7 @@ enum GamelogChangeType {
 	GLCT_GRFCOMPAT,   ///< Loading compatible GRF
 	GLCT_GRFPARAM,    ///< GRF parameter changed
 	GLCT_GRFMOVE,     ///< GRF order changed
+	GLCT_GRFBUG,      ///< GRF bug triggered
 	GLCT_END,         ///< So we know how many GLCTs are there
 	GLCT_NONE = 0xFF, ///< In savegames, end of list
 };
@@ -79,6 +80,11 @@ struct LoggedChange {
 			int32 oldval;    ///< old value
 			int32 newval;    ///< new value
 		} patch;
+		struct {
+			uint64 data;     ///< additional data
+			uint32 grfid;    ///< ID of problematic GRF
+			byte bug;        ///< type of bug, @see enum GRFBugs
+		} grfbug;
 	};
 };
 
@@ -198,7 +204,8 @@ static const char *la_text[] = {
 	"game loaded",
 	"GRF config changed",
 	"cheat was used",
-	"patch settings changed"
+	"patch settings changed",
+	"GRF bug triggered",
 };
 
 assert_compile(lengthof(la_text) == GLAT_END);
@@ -300,6 +307,15 @@ void GamelogPrint(GamelogPrintProc *proc)
 						BSWAP32(lc->grfmove.grfid), abs(lc->grfmove.offset), lc->grfmove.offset >= 0 ? "down" : "up" );
 					PrintGrfFilename(buf, lc->grfmove.grfid);
 					break;
+
+				case GLCT_GRFBUG:
+					switch (lc->grfbug.bug) {
+						default: NOT_REACHED();
+						case GBUG_VEH_LENGTH:
+							AddDebugText(buf, "Rail vehicle changes length outside a depot: GRF ID %08X, internal ID 0x%X", BSWAP32(lc->grfbug.grfid), (uint)lc->grfbug.data);
+							PrintGrfFilename(buf, lc->grfbug.grfid);
+							break;
+					}
 			}
 
 			proc(buf);
@@ -469,6 +485,51 @@ void GamelogTestMode()
 }
 
 
+/** Logs triggered GRF bug.
+ * @param grfid ID of problematic GRF
+ * @param bug type of bug, @see enum GRFBugs
+ * @param data additional data
+ */
+static void GamelogGRFBug(uint32 grfid, byte bug, uint64 data)
+{
+	assert(_gamelog_action_type == GLAT_GRFBUG);
+
+	LoggedChange *lc = GamelogChange(GLCT_GRFBUG);
+	if (lc == NULL) return;
+
+	lc->grfbug.data  = data;
+	lc->grfbug.grfid = grfid;
+	lc->grfbug.bug   = bug;
+}
+
+/** Logs GRF bug - rail vehicle has different length after reversing.
+ * Ensures this is logged only once for each GRF and engine type
+ * This check takes some time, but it is called pretty seldom, so it
+ * doesn't matter that much (ideally it shouldn't be called at all).
+ * @param engine engine to log
+ * @return true iff a unique record was done
+ */
+bool GamelogGRFBugReverse(uint32 grfid, uint16 internal_id)
+{
+	const LoggedAction *laend = &_gamelog_action[_gamelog_actions];
+	for (const LoggedAction *la = _gamelog_action; la != laend; la++) {
+		const LoggedChange *lcend = &la->change[la->changes];
+		for (const LoggedChange *lc = la->change; lc != lcend; lc++) {
+			if (lc->ct == GLCT_GRFBUG && lc->grfbug.grfid == grfid &&
+					lc->grfbug.bug == GBUG_VEH_LENGTH && lc->grfbug.data == internal_id) {
+				return false;
+			}
+		}
+	}
+
+	GamelogStartAction(GLAT_GRFBUG);
+	GamelogGRFBug(grfid, GBUG_VEH_LENGTH, internal_id);
+	GamelogStopAction();
+
+	return true;
+}
+
+
 /** Decides if GRF should be logged
  * @param g grf to determine
  * @return true iff GRF is not static and is loaded
@@ -724,8 +785,15 @@ static const SaveLoad _glog_grfparam_desc[] = {
 };
 
 static const SaveLoad _glog_grfmove_desc[] = {
-	SLE_VAR(LoggedChange, grfmove.grfid,    SLE_UINT32),
-	SLE_VAR(LoggedChange, grfmove.offset,   SLE_INT32),
+	SLE_VAR(LoggedChange, grfmove.grfid,     SLE_UINT32),
+	SLE_VAR(LoggedChange, grfmove.offset,    SLE_INT32),
+	SLE_END()
+};
+
+static const SaveLoad _glog_grfbug_desc[] = {
+	SLE_VAR(LoggedChange, grfbug.data,       SLE_UINT64),
+	SLE_VAR(LoggedChange, grfbug.grfid,      SLE_UINT32),
+	SLE_VAR(LoggedChange, grfbug.bug,        SLE_UINT8),
 	SLE_END()
 };
 
@@ -738,7 +806,8 @@ static const SaveLoad *_glog_desc[] = {
 	_glog_grfrem_desc,
 	_glog_grfcompat_desc,
 	_glog_grfparam_desc,
-	_glog_grfmove_desc
+	_glog_grfmove_desc,
+	_glog_grfbug_desc,
 };
 
 assert_compile(lengthof(_glog_desc) == GLCT_END);
diff --git a/src/gamelog.h b/src/gamelog.h
index acf97c7ea..d2146d7a3 100644
--- a/src/gamelog.h
+++ b/src/gamelog.h
@@ -13,6 +13,7 @@ enum GamelogActionType {
 	GLAT_GRF,          ///< GRF changed
 	GLAT_CHEAT,        ///< Cheat was used
 	GLAT_PATCH,        ///< Patches setting changed
+	GLAT_GRFBUG,       ///< GRF bug was triggered
 	GLAT_END,          ///< So we know how many GLATs are there
 	GLAT_NONE  = 0xFF, ///< No logging active; in savegames, end of list
 };
@@ -43,4 +44,6 @@ void GamelogTestRevision();
 void GamelogTestMode();
 void GamelogTestGRF();
 
+bool GamelogGRFBugReverse(uint32 grfid, uint16 internal_id);
+
 #endif /* GAMELOG_H */
diff --git a/src/lang/english.txt b/src/lang/english.txt
index 27c7b0998..c3b57570b 100644
--- a/src/lang/english.txt
+++ b/src/lang/english.txt
@@ -3217,6 +3217,10 @@ STR_NEWGRF_NOT_FOUND_WARNING                                    :{WHITE}Missing
 STR_NEWGRF_UNPAUSE_WARNING_TITLE                                :{YELLOW}Missing GRF file(s)
 STR_NEWGRF_UNPAUSE_WARNING                                      :{WHITE}Unpausing can crash OpenTTD. Do not file bug reports for subsequent crashes.{}Do you really want to unpause?
 
+STR_NEWGRF_BROKEN                                               :{WHITE}Behaviour of NewGRF '{0:RAW_STRING}' is likely to cause desyncs and/or crashes.
+STR_NEWGRF_BROKEN_VEHICLE_LENGTH                                :{WHITE}It changes vehicle length for '{1:ENGINE}' when not inside a depot.
+STR_BROKEN_VEHICLE_LENGTH                                       :{WHITE}Train '{VEHICLE}' belonging to '{COMPANY}' has invalid length. It is probably caused by problems with NewGRFs. Game may desync or crash.
+
 STR_LOADGAME_REMOVED_TRAMS                                      :{WHITE}Game was saved in version without tram support. All trams have been removed.
 
 STR_CURRENCY_WINDOW                                             :{WHITE}Custom currency
diff --git a/src/newgrf_config.h b/src/newgrf_config.h
index 45138d471..e7d5b11cb 100644
--- a/src/newgrf_config.h
+++ b/src/newgrf_config.h
@@ -28,8 +28,13 @@ enum GRFStatus {
 	GCS_ACTIVATED     ///< GRF file has been activated
 };
 
+/** Encountered GRF bugs */
+enum GRFBugs {
+	GBUG_VEH_LENGTH,  ///< Length of rail vehicle changes when not inside a depot
+};
+
 /** Status of post-gameload GRF compatibility check */
-enum GRFListCompatibility{
+enum GRFListCompatibility {
 	GLC_ALL_GOOD,   ///< All GRF needed by game are present
 	GLC_COMPATIBLE, ///< Compatible (eg. the same ID, but different chacksum) GRF found in at least one case
 	GLC_NOT_FOUND   ///< At least one GRF couldn't be found (higher priority than GLC_COMPATIBLE)
@@ -60,6 +65,7 @@ struct GRFConfig : public GRFIdentifier {
 
 	uint8 flags;        ///< NOSAVE: GCF_Flags, bitset
 	GRFStatus status;   ///< NOSAVE: GRFStatus, enum
+	uint32 grf_bugs;    ///< NOSAVE: bugs in this GRF in this run, @see enum GRFBugs
 	uint32 param[0x80]; ///< GRF parameters
 	uint8 num_params;   ///< Number of used parameters
 
diff --git a/src/openttd.cpp b/src/openttd.cpp
index ae06cd2e8..0fdb22b55 100644
--- a/src/openttd.cpp
+++ b/src/openttd.cpp
@@ -1013,7 +1013,7 @@ void StateGameLoop()
 					length = 0;
 					for (Vehicle *u = v; u != NULL; u = u->Next()) wagons[length++] = u->u.rail;
 
-					TrainConsistChanged(v);
+					TrainConsistChanged(v, true);
 
 					length = 0;
 					for (Vehicle *u = v; u != NULL; u = u->Next()) {
@@ -1266,6 +1266,8 @@ static bool InitializeWindowsAndCaches()
 	 * Reset each town's noise_reached value to '0' before. */
 	UpdateAirportsNoise();
 
+	CheckTrainsLengths();
+
 	return true;
 }
 
@@ -1759,7 +1761,7 @@ bool AfterLoadGame()
 		}
 
 		FOR_ALL_VEHICLES(v) {
-			if (v->type == VEH_TRAIN && (IsFrontEngine(v) || IsFreeWagon(v))) TrainConsistChanged(v);
+			if (v->type == VEH_TRAIN && (IsFrontEngine(v) || IsFreeWagon(v))) TrainConsistChanged(v, true);
 		}
 
 	}
@@ -2466,4 +2468,5 @@ void ReloadNewGRFData()
 	for (PlayerID i = PLAYER_FIRST; i < MAX_PLAYERS; i++) InvalidateWindowData(WC_PLAYER_COLOR, i, _loaded_newgrf_features.has_2CC);
 	/* redraw the whole screen */
 	MarkWholeScreenDirty();
+	CheckTrainsLengths();
 }
diff --git a/src/settings.cpp b/src/settings.cpp
index 9e7a561c5..32eb48797 100644
--- a/src/settings.cpp
+++ b/src/settings.cpp
@@ -1240,7 +1240,7 @@ static int32 UpdateConsists(int32 p1)
 	Vehicle *v;
 	FOR_ALL_VEHICLES(v) {
 		/* Update the consist of all trains so the maximum speed is set correctly. */
-		if (v->type == VEH_TRAIN && (IsFrontEngine(v) || IsFreeWagon(v))) TrainConsistChanged(v);
+		if (v->type == VEH_TRAIN && (IsFrontEngine(v) || IsFreeWagon(v))) TrainConsistChanged(v, true);
 	}
 	return 0;
 }
diff --git a/src/train.h b/src/train.h
index f2dc7988f..7f7cb6166 100644
--- a/src/train.h
+++ b/src/train.h
@@ -271,6 +271,7 @@ byte FreightWagonMult(CargoID cargo);
 int CheckTrainInDepot(const Vehicle *v, bool needs_to_be_stopped);
 int CheckTrainStoppedInDepot(const Vehicle *v);
 void UpdateTrainAcceleration(Vehicle* v);
+void CheckTrainsLengths();
 
 /**
  * This class 'wraps' Vehicle; you do not actually instantiate this class.
diff --git a/src/train_cmd.cpp b/src/train_cmd.cpp
index fd7c17127..f2be43e03 100644
--- a/src/train_cmd.cpp
+++ b/src/train_cmd.cpp
@@ -51,6 +51,8 @@
 #include "order_func.h"
 #include "newgrf_station.h"
 #include "effectvehicle_func.h"
+#include "gamelog.h"
+#include "network/network.h"
 
 #include "table/strings.h"
 #include "table/train_cmd.h"
@@ -181,13 +183,76 @@ static void TrainCargoChanged(Vehicle *v)
 }
 
 
+/** Logs a bug in GRF and shows a warning message if this
+ * is for the first time this happened.
+ * @param u first vehicle of chain
+ */
+static void RailVehicleLengthChanged(const Vehicle *u)
+{
+	/* show a warning once for each engine in whole game and once for each GRF after each game load */
+	const Engine *engine = GetEngine(u->engine_type);
+	uint32 grfid = engine->grffile->grfid;
+	GRFConfig *grfconfig = GetGRFConfig(grfid);
+	if (GamelogGRFBugReverse(grfid, engine->internal_id) || !HasBit(grfconfig->grf_bugs, GBUG_VEH_LENGTH)) {
+		SetBit(grfconfig->grf_bugs, GBUG_VEH_LENGTH);
+		SetDParamStr(0, grfconfig->name);
+		SetDParam(1, u->engine_type);
+		ShowErrorMessage(STR_NEWGRF_BROKEN_VEHICLE_LENGTH, STR_NEWGRF_BROKEN, 0, 0);
+
+		/* debug output */
+		char buffer[512];
+
+		SetDParamStr(0, grfconfig->name);
+		GetString(buffer, STR_NEWGRF_BROKEN, lastof(buffer));
+		DEBUG(grf, 0, "%s", buffer + 3);
+
+		SetDParam(1, u->engine_type);
+		GetString(buffer, STR_NEWGRF_BROKEN_VEHICLE_LENGTH, lastof(buffer));
+		DEBUG(grf, 0, "%s", buffer + 3);
+
+#ifdef ENABLE_NETWORK
+		if (!_networking) _pause_game = -1;
+#else
+		_pause_game = -1;
+#endif
+	}
+}
+
+/** Checks if lengths of all rail vehicles are valid. If not, shows an error message. */
+void CheckTrainsLengths()
+{
+	const Vehicle *v;
+
+	FOR_ALL_VEHICLES(v) {
+		if (v->type == VEH_TRAIN && v->First() == v && !(v->vehstatus & VS_CRASHED)) {
+			for (const Vehicle *u = v, *w = v->Next(); w != NULL; u = w, w = w->Next()) {
+				if (u->u.rail.track != TRACK_BIT_DEPOT) {
+					if ((w->u.rail.track != TRACK_BIT_DEPOT &&
+							max(abs(u->x_pos - w->x_pos), abs(u->y_pos - w->y_pos)) != u->u.rail.cached_veh_length) ||
+							(w->u.rail.track == TRACK_BIT_DEPOT && TicksToLeaveDepot(u) <= 0)) {
+						SetDParam(0, v->index);
+						SetDParam(1, v->owner);
+						ShowErrorMessage(INVALID_STRING_ID, STR_BROKEN_VEHICLE_LENGTH, 0, 0);
+#ifdef ENABLE_NETWORK
+						if (!_networking) _pause_game = -1;
+#else
+						_pause_game = -1;
+#endif
+					}
+				}
+			}
+		}
+	}
+}
+
 /**
  * Recalculates the cached stuff of a train. Should be called each time a vehicle is added
  * to/removed from the chain, and when the game is loaded.
  * Note: this needs to be called too for 'wagon chains' (in the depot, without an engine)
  * @param v First vehicle of the chain.
+ * @param same_length should length of vehicles stay the same?
  */
-void TrainConsistChanged(Vehicle *v)
+void TrainConsistChanged(Vehicle *v, bool same_length)
 {
 	uint16 max_speed = UINT16_MAX;
 
@@ -295,8 +360,14 @@ void TrainConsistChanged(Vehicle *v)
 			veh_len = GetVehicleCallback(CBID_VEHICLE_LENGTH, 0, 0, u->engine_type, u);
 		}
 		if (veh_len == CALLBACK_FAILED) veh_len = rvi_u->shorten_factor;
-		veh_len = Clamp(veh_len, 0, u->Next() == NULL ? 7 : 5); // the clamp on vehicles not the last in chain is stricter, as too short wagons can break the 'follow next vehicle' code
-		u->u.rail.cached_veh_length = 8 - veh_len;
+		veh_len = 8 - Clamp(veh_len, 0, u->Next() == NULL ? 7 : 5); // the clamp on vehicles not the last in chain is stricter, as too short wagons can break the 'follow next vehicle' code
+
+		/* verify length hasn't changed */
+		if (same_length && veh_len != u->u.rail.cached_veh_length) RailVehicleLengthChanged(u);
+
+		/* update vehicle length? */
+		if (!same_length) u->u.rail.cached_veh_length = veh_len;
+
 		v->u.rail.cached_total_length += u->u.rail.cached_veh_length;
 	}
 
@@ -618,7 +689,7 @@ static CommandCost CmdBuildRailWagon(EngineID engine, TileIndex tile, uint32 fla
 			_new_vehicle_id = v->index;
 
 			VehiclePositionChanged(v);
-			TrainConsistChanged(v->First());
+			TrainConsistChanged(v->First(), false);
 			UpdateTrainGroupID(v->First());
 
 			InvalidateWindow(WC_VEHICLE_DEPOT, v->tile);
@@ -794,7 +865,7 @@ CommandCost CmdBuildRailVehicle(TileIndex tile, uint32 flags, uint32 p1, uint32
 				AddArticulatedParts(vl, VEH_TRAIN);
 			}
 
-			TrainConsistChanged(v);
+			TrainConsistChanged(v, false);
 			UpdateTrainGroupID(v);
 
 			if (!HasBit(p2, 1) && !(flags & DC_AUTOREPLACE)) { // check if the cars should be added to the new vehicle
@@ -1247,7 +1318,7 @@ CommandCost CmdMoveRailVehicle(TileIndex tile, uint32 flags, uint32 p1, uint32 p
 
 		if (src_head != NULL) {
 			NormaliseTrainConsist(src_head);
-			TrainConsistChanged(src_head);
+			TrainConsistChanged(src_head, false);
 			UpdateTrainGroupID(src_head);
 			if (IsFrontEngine(src_head)) {
 				/* Update the refit button and window */
@@ -1260,7 +1331,7 @@ CommandCost CmdMoveRailVehicle(TileIndex tile, uint32 flags, uint32 p1, uint32 p
 
 		if (dst_head != NULL) {
 			NormaliseTrainConsist(dst_head);
-			TrainConsistChanged(dst_head);
+			TrainConsistChanged(dst_head, false);
 			UpdateTrainGroupID(dst_head);
 			if (IsFrontEngine(dst_head)) {
 				/* Update the refit button and window */
@@ -1432,7 +1503,7 @@ CommandCost CmdSellRailWagon(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 				/* 5. If the train still exists, update its acceleration, window, etc. */
 				if (first != NULL) {
 					NormaliseTrainConsist(first);
-					TrainConsistChanged(first);
+					TrainConsistChanged(first, false);
 					UpdateTrainGroupID(first);
 					if (IsFrontEngine(first)) InvalidateWindow(WC_VEHICLE_REFIT, first->index);
 				}
@@ -1497,7 +1568,7 @@ CommandCost CmdSellRailWagon(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 			/* 3. If it is still a valid train after selling, update its acceleration and cached values */
 			if (flags & DC_EXEC && first != NULL) {
 				NormaliseTrainConsist(first);
-				TrainConsistChanged(first);
+				TrainConsistChanged(first, false);
 				UpdateTrainGroupID(first);
 				InvalidateWindow(WC_VEHICLE_REFIT, first->index);
 			}
@@ -1832,7 +1903,7 @@ static void ReverseTrainDirection(Vehicle *v)
 	ClrBit(v->u.rail.flags, VRF_REVERSING);
 
 	/* recalculate cached data */
-	TrainConsistChanged(v);
+	TrainConsistChanged(v, true);
 
 	/* update all images */
 	for (Vehicle *u = v; u != NULL; u = u->Next()) u->cur_image = u->GetImage(u->direction);
@@ -2009,7 +2080,7 @@ CommandCost CmdRefitRailVehicle(TileIndex tile, uint32 flags, uint32 p1, uint32
 	_returned_refit_capacity = num;
 
 	/* Update the train's cached variables */
-	if (flags & DC_EXEC) TrainConsistChanged(GetVehicle(p1)->First());
+	if (flags & DC_EXEC) TrainConsistChanged(GetVehicle(p1)->First(), false);
 
 	return cost;
 }
@@ -3147,7 +3218,7 @@ static void DeleteLastWagon(Vehicle *v)
 		InvalidateWindow(WC_COMPANY, v->owner);
 	} else {
 		/* Recalculate cached train properties */
-		TrainConsistChanged(first);
+		TrainConsistChanged(first, false);
 		/* Update the depot window if the first vehicle is in depot -
 		 * if v == first, then it is updated in PreDestructor() */
 		if (first->u.rail.track == TRACK_BIT_DEPOT) {
diff --git a/src/vehicle.cpp b/src/vehicle.cpp
index 868039cbb..46866fa0e 100644
--- a/src/vehicle.cpp
+++ b/src/vehicle.cpp
@@ -279,7 +279,7 @@ void AfterLoadVehicles(bool clear_te_id)
 
 		if (v->type == VEH_TRAIN && (IsFrontEngine(v) || IsFreeWagon(v))) {
 			if (IsFrontEngine(v)) v->u.rail.last_speed = v->cur_speed; // update displayed train speed
-			TrainConsistChanged(v);
+			TrainConsistChanged(v, false);
 		} else if (v->type == VEH_ROAD && IsRoadVehFront(v)) {
 			RoadVehUpdateCache(v);
 		}
@@ -1377,7 +1377,7 @@ void VehicleEnterDepot(Vehicle *v)
 			UpdateSignalsOnSegment(v->tile, INVALID_DIAGDIR, v->owner);
 			v->load_unload_time_rem = 0;
 			ClrBit(v->u.rail.flags, VRF_TOGGLE_REVERSE);
-			TrainConsistChanged(v);
+			TrainConsistChanged(v, true);
 			break;
 
 		case VEH_ROAD:
diff --git a/src/vehicle_func.h b/src/vehicle_func.h
index c232b9d68..61c941548 100644
--- a/src/vehicle_func.h
+++ b/src/vehicle_func.h
@@ -64,7 +64,7 @@ void MarkSingleVehicleDirty(const Vehicle *v);
 
 UnitID GetFreeUnitNumber(VehicleType type);
 
-void TrainConsistChanged(Vehicle *v);
+void TrainConsistChanged(Vehicle *v, bool same_length);
 void TrainPowerChanged(Vehicle *v);
 Money GetTrainRunningCost(const Vehicle *v);