(svn r15645) -Fix (r12924)[FS#2612]: Add an EngineOverrideManager to give the term 'compatible newgrf' again some sense and to not crash because of trivial changes.

src/engine.cpp

@@ -28,6 +28,8 @@
 
 DEFINE_OLD_POOL_GENERIC(Engine, Engine)
 
+EngineOverrideManager _engine_mngr;
+
 /** Year that engine aging stops. Engines will not reduce in reliability
  * and no more engines will be introduced */
 Year _year_engine_aging_stops;
@@ -48,6 +50,8 @@ const uint8 _engine_offsets[4] = {
 	lengthof(_orig_rail_vehicle_info) + lengthof(_orig_road_vehicle_info) + lengthof(_orig_ship_vehicle_info),
 };
 
+const uint EngineOverrideManager::NUM_DEFAULT_ENGINES = _engine_counts[VEH_TRAIN] + _engine_counts[VEH_ROAD] + _engine_counts[VEH_SHIP] + _engine_counts[VEH_AIRCRAFT];
+
 Engine::Engine() :
 	name(NULL),
 	overrides_count(0),
@@ -280,6 +284,44 @@ uint Engine::GetDisplayMaxTractiveEffort() const
 	}
 }
 
+/**
+ * Initializes the EngineOverrideManager with the default engines.
+ */
+void EngineOverrideManager::ResetToDefaultMapping()
+{
+	this->Clear();
+	for (VehicleType type = VEH_TRAIN; type <= VEH_AIRCRAFT; type++) {
+		for (uint internal_id = 0; internal_id < _engine_counts[type]; internal_id++) {
+			EngineIDMapping *eid = this->Append();
+			eid->type            = type;
+			eid->grfid           = INVALID_GRFID;
+			eid->internal_id     = internal_id;
+			eid->substitute_id   = internal_id;
+		}
+	}
+}
+
+/**
+ * Looks up an EngineID in the EngineOverrideManager
+ * @param type Vehicle type
+ * @param grf_local_id The local id in the newgrf
+ * @param grfid The GrfID that defines the scope of grf_local_id.
+ *              If a newgrf overrides the engines of another newgrf, the "scope grfid" is the ID of the overridden newgrf.
+ *              If dynnamic_engines is disabled, all newgrf share the same ID scope identified by INVALID_GRFID.
+ * @return The engine ID if present, or INVALID_ENGINE if not.
+ */
+EngineID EngineOverrideManager::GetID(VehicleType type, uint16 grf_local_id, uint32 grfid)
+{
+	const EngineIDMapping *end = this->End();
+	EngineID index = 0;
+	for (const EngineIDMapping *eid = this->Begin(); eid != end; eid++, index++) {
+		if (eid->type == type && eid->grfid == grfid && eid->internal_id == grf_local_id) {
+			return index;
+		}
+	}
+	return INVALID_ENGINE;
+}
+
 /** Sets cached values in Company::num_vehicles and Group::num_vehicles
  */
 void SetCachedEngineCounts()
@@ -323,10 +365,13 @@ void SetupEngines()
 	_Engine_pool.CleanPool();
 	_Engine_pool.AddBlockToPool();
 
-	for (uint i = 0; i < lengthof(_orig_rail_vehicle_info); i++) new Engine(VEH_TRAIN, i);
-	for (uint i = 0; i < lengthof(_orig_road_vehicle_info); i++) new Engine(VEH_ROAD, i);
-	for (uint i = 0; i < lengthof(_orig_ship_vehicle_info); i++) new Engine(VEH_SHIP, i);
-	for (uint i = 0; i < lengthof(_orig_aircraft_vehicle_info); i++) new Engine(VEH_AIRCRAFT, i);
+	assert(_engine_mngr.Length() >= _engine_mngr.NUM_DEFAULT_ENGINES);
+	const EngineIDMapping *end = _engine_mngr.End();
+	uint index = 0;
+	for (const EngineIDMapping *eid = _engine_mngr.Begin(); eid != end; eid++, index++) {
+		const Engine *e = new Engine(eid->type, eid->internal_id);
+		assert(e->index == index);
+	}
 }
 
 

src/engine_base.h

@@ -8,6 +8,7 @@
 #include "engine_type.h"
 #include "economy_type.h"
 #include "oldpool.h"
+#include "core/smallvec_type.hpp"
 
 DECLARE_OLD_POOL(Engine, Engine, 6, 10000)
 
@@ -60,6 +61,26 @@ struct Engine : PoolItem<Engine, EngineID, &_Engine_pool> {
 	uint GetDisplayMaxTractiveEffort() const;
 };
 
+struct EngineIDMapping {
+	uint32 grfid;          ///< The GRF ID of the file the entity belongs to
+	uint16 internal_id;    ///< The internal ID within the GRF file
+	VehicleTypeByte type;  ///< The engine type
+	uint8  substitute_id;  ///< The (original) entity ID to use if this GRF is not available (currently not used)
+};
+
+/**
+ * Stores the mapping of EngineID to the internal id of newgrfs.
+ * Note: This is not part of Engine, as the data in the EngineOverrideManager and the engine pool get resetted in different cases.
+ */
+struct EngineOverrideManager : SmallVector<EngineIDMapping, 256> {
+	static const uint NUM_DEFAULT_ENGINES; ///< Number of default entries
+
+	void ResetToDefaultMapping();
+	EngineID GetID(VehicleType type, uint16 grf_local_id, uint32 grfid);
+};
+
+extern EngineOverrideManager _engine_mngr;
+
 static inline bool IsEngineIndex(uint index)
 {
 	return index < GetEnginePoolSize();

src/newgrf.cpp

@@ -328,39 +328,57 @@ static Engine *GetNewEngine(const GRFFile *file, VehicleType type, uint16 intern
 {
 	/* Hack for add-on GRFs that need to modify another GRF's engines. This lets
 	 * them use the same engine slots. */
-	const GRFFile *grf_match = NULL;
+	uint32 scope_grfid = INVALID_GRFID; // If not using dynamic_engines, all newgrfs share their ID range
 	if (_settings_game.vehicle.dynamic_engines) {
+		/* If dynamic_engies is enabled, there can be multiple independent ID ranges. */
+		scope_grfid = file->grfid;
 		uint32 override = _grf_id_overrides[file->grfid];
 		if (override != 0) {
-			grf_match = GetFileByGRFID(override);
+			scope_grfid = override;
+			const GRFFile *grf_match = GetFileByGRFID(override);
 			if (grf_match == NULL) {
 				grfmsg(5, "Tried mapping from GRFID %x to %x but target is not loaded", BSWAP32(file->grfid), BSWAP32(override));
 			} else {
 				grfmsg(5, "Mapping from GRFID %x to %x", BSWAP32(file->grfid), BSWAP32(override));
 			}
 		}
+
+		/* Check if the engine is registered in the override manager */
+		EngineID engine = _engine_mngr.GetID(type, internal_id, scope_grfid);
+		if (engine != INVALID_ENGINE) return GetEngine(engine);
 	}
 
-	/* Check if this vehicle is already defined... */
-	Engine *e = NULL;
-	FOR_ALL_ENGINES(e) {
-		if (_settings_game.vehicle.dynamic_engines && e->grffile != NULL && e->grffile != file && e->grffile != grf_match) continue;
-		if (e->type != type) continue;
-		if (e->internal_id != internal_id) continue;
+	/* Check if there is an unreserved slot */
+	EngineID engine = _engine_mngr.GetID(type, internal_id, INVALID_GRFID);
+	if (engine != INVALID_ENGINE) {
+		Engine *e = GetEngine(engine);
 
 		if (e->grffile == NULL) {
 			e->grffile = file;
 			grfmsg(5, "Replaced engine at index %d for GRFID %x, type %d, index %d", e->index, BSWAP32(file->grfid), type, internal_id);
 		}
+
+		/* Reserve the engine slot */
+		EngineIDMapping *eid = _engine_mngr.Get(engine);
+		eid->grfid           = scope_grfid; // Note: this is INVALID_GRFID if dynamic_engines is disabled, so no reservation
+
 		return e;
 	}
 
 	uint engine_pool_size = GetEnginePoolSize();
 
 	/* ... it's not, so create a new one based off an existing engine */
-	e = new Engine(type, internal_id);
+	Engine *e = new Engine(type, internal_id);
 	e->grffile = file;
 
+	/* Reserve the engine slot */
+	assert(_engine_mngr.Length() == e->index);
+	EngineIDMapping *eid = _engine_mngr.Append();
+	eid->type            = type;
+	eid->grfid           = scope_grfid; // Note: this is INVALID_GRFID if dynamic_engines is disabled, so no reservation
+	eid->internal_id     = internal_id;
+	eid->substitute_id   = min(internal_id, _engine_counts[type]); // substitute_id == _engine_counts[subtype] means "no substitute"
+
 	if (engine_pool_size != GetEnginePoolSize()) {
 		/* Resize temporary engine data ... */
 		_gted = ReallocT(_gted, GetEnginePoolSize());
@@ -377,24 +395,14 @@ static Engine *GetNewEngine(const GRFFile *file, VehicleType type, uint16 intern
 
 EngineID GetNewEngineID(const GRFFile *file, VehicleType type, uint16 internal_id)
 {
-	extern uint32 GetNewGRFOverride(uint32 grfid);
-
-	const GRFFile *grf_match = NULL;
+	uint32 scope_grfid = INVALID_GRFID; // If not using dynamic_engines, all newgrfs share their ID range
 	if (_settings_game.vehicle.dynamic_engines) {
+		scope_grfid = file->grfid;
 		uint32 override = _grf_id_overrides[file->grfid];
-		if (override != 0) grf_match = GetFileByGRFID(override);
-	}
-
-	const Engine *e = NULL;
-	FOR_ALL_ENGINES(e) {
-		if (_settings_game.vehicle.dynamic_engines && e->grffile != file && (grf_match == NULL || e->grffile != grf_match)) continue;
-		if (e->type != type) continue;
-		if (e->internal_id != internal_id) continue;
-
-		return e->index;
+		if (override != 0) scope_grfid = override;
 	}
 
-	return INVALID_ENGINE;
+	return _engine_mngr.GetID(type, internal_id, scope_grfid);
 }
 
 /** Map the colour modifiers of TTDPatch to those that Open is using.

src/newgrf.h

@@ -50,6 +50,8 @@ enum GrfSpecFeature {
 	GSF_END,
 };
 
+static const uint32 INVALID_GRFID = 0xFFFFFFFF;
+
 struct GRFLabel {
 	byte label;
 	uint32 nfo_line;

src/openttd.cpp

@@ -286,6 +286,8 @@ static void ParseResolution(Dimension *res, const char *s)
 static void InitializeDynamicVariables()
 {
 	/* Dynamic stuff needs to be initialized somewhere... */
+	_engine_mngr.ResetToDefaultMapping();
+	_house_mngr.ResetMapping();
 	_industry_mngr.ResetMapping();
 	_industile_mngr.ResetMapping();
 	_Company_pool.AddBlockToPool();
@@ -763,6 +765,7 @@ static void MakeNewGame(bool from_heightmap)
 	_game_mode = GM_NORMAL;
 
 	ResetGRFConfig(true);
+	_engine_mngr.ResetToDefaultMapping();
 	_house_mngr.ResetMapping();
 	_industile_mngr.ResetMapping();
 	_industry_mngr.ResetMapping();

src/saveload/engine_sl.cpp

@@ -111,7 +111,39 @@ static void Load_ENGS()
 	}
 }
 
+/** Save and load the mapping between the engine id in the pool, and the grf file it came from. */
+static const SaveLoad _engine_id_mapping_desc[] = {
+	SLE_VAR(EngineIDMapping, grfid,         SLE_UINT32),
+	SLE_VAR(EngineIDMapping, internal_id,   SLE_UINT16),
+	SLE_VAR(EngineIDMapping, type,          SLE_UINT8),
+	SLE_VAR(EngineIDMapping, substitute_id, SLE_UINT8),
+	SLE_END()
+};
+
+static void Save_EIDS()
+{
+	const EngineIDMapping *end = _engine_mngr.End();
+	uint index = 0;
+	for (EngineIDMapping *eid = _engine_mngr.Begin(); eid != end; eid++, index++) {
+		SlSetArrayIndex(index);
+		SlObject(eid, _engine_id_mapping_desc);
+	}
+}
+
+static void Load_EIDS()
+{
+	int index;
+
+	_engine_mngr.Clear();
+
+	while ((index = SlIterateArray()) != -1) {
+		EngineIDMapping *eid = _engine_mngr.Append();
+		SlObject(eid, _engine_id_mapping_desc);
+	}
+}
+
 extern const ChunkHandler _engine_chunk_handlers[] = {
+	{ 'EIDS', Save_EIDS,     Load_EIDS,     CH_ARRAY          },
 	{ 'ENGN', Save_ENGN,     Load_ENGN,     CH_ARRAY          },
 	{ 'ENGS', NULL,          Load_ENGS,     CH_RIFF | CH_LAST },
 };

src/saveload/saveload.cpp

@@ -34,6 +34,7 @@
 #include "../fileio_func.h"
 #include "../gamelog.h"
 #include "../string_func.h"
+#include "../engine_base.h"
 
 #include "table/strings.h"
 
@@ -1675,6 +1676,7 @@ SaveOrLoadResult SaveOrLoad(const char *filename, int mode, Subdirectory sb)
 
 	/* Load a TTDLX or TTDPatch game */
 	if (mode == SL_OLD_LOAD) {
+		_engine_mngr.ResetToDefaultMapping();
 		InitializeGame(256, 256, true); // set a mapsize of 256x256 for TTDPatch games or it might get confused
 		GamelogReset();
 		if (!LoadOldSaveGame(filename)) return SL_REINIT;
@@ -1789,6 +1791,8 @@ SaveOrLoadResult SaveOrLoad(const char *filename, int mode, Subdirectory sb)
 				SlError(STR_GAME_SAVELOAD_ERROR_BROKEN_INTERNAL_ERROR, err_str);
 			}
 
+			_engine_mngr.ResetToDefaultMapping();
+
 			/* Old maps were hardcoded to 256x256 and thus did not contain
 			 * any mapsize information. Pre-initialize to 256x256 to not to
 			 * confuse old games */