diff --git a/src/build_vehicle_gui.cpp b/src/build_vehicle_gui.cpp index 6c5e5223e3..9c869426e5 100644 --- a/src/build_vehicle_gui.cpp +++ b/src/build_vehicle_gui.cpp @@ -1422,7 +1422,7 @@ struct BuildVehicleWindow : Window { list.emplace_back(eid, e->info.variant_id, e->display_flags, 0); if (rvi->railveh_type != RAILVEH_WAGON) num_engines++; - if (e->info.variant_id != eid && e->info.variant_id != INVALID_ENGINE) variants.push_back(e->info.variant_id); + if (e->info.variant_id != INVALID_ENGINE) variants.push_back(e->info.variant_id); if (eid == this->sel_engine) sel_id = eid; } @@ -1654,13 +1654,14 @@ struct BuildVehicleWindow : Window { Command::Post(GetCmdBuildVehMsg(this->vehicle_type), CcBuildPrimaryVehicle, this->window_number, sel_eng, true, cargo, INVALID_CLIENT_ID); } - /* Update last used variant and refresh if necessary. */ + /* Update last used variant in hierarchy and refresh if necessary. */ bool refresh = false; - int recursion = 10; /* In case of infinite loop */ - for (Engine *e = Engine::Get(sel_eng); recursion > 0; e = Engine::Get(e->info.variant_id), --recursion) { + EngineID parent = sel_eng; + while (parent != INVALID_ENGINE) { + Engine *e = Engine::Get(parent); refresh |= (e->display_last_variant != sel_eng); e->display_last_variant = sel_eng; - if (e->info.variant_id == INVALID_ENGINE) break; + parent = e->info.variant_id; } if (refresh) { InvalidateWindowData(WC_REPLACE_VEHICLE, this->vehicle_type, 0); // Update the autoreplace window diff --git a/src/engine.cpp b/src/engine.cpp index 3aa242e375..50ae4606b3 100644 --- a/src/engine.cpp +++ b/src/engine.cpp @@ -491,7 +491,7 @@ bool Engine::IsVariantHidden(CompanyID c) const * the last display variant rather than the actual parent variant. */ const Engine *re = this; const Engine *ve = re->GetDisplayVariant(); - while (!(ve->IsHidden(c)) && re->info.variant_id != INVALID_ENGINE && re->info.variant_id != re->index) { + while (!(ve->IsHidden(c)) && re->info.variant_id != INVALID_ENGINE) { re = Engine::Get(re->info.variant_id); ve = re->GetDisplayVariant(); } @@ -607,7 +607,7 @@ void CalcEngineReliability(Engine *e, bool new_month) { /* Get source engine for reliability age. This is normally our engine unless variant reliability syncing is requested. */ Engine *re = e; - while (re->info.variant_id != INVALID_ENGINE && re->info.variant_id != re->index && (re->info.extra_flags & ExtraEngineFlags::SyncReliability) != ExtraEngineFlags::None) { + while (re->info.variant_id != INVALID_ENGINE && (re->info.extra_flags & ExtraEngineFlags::SyncReliability) != ExtraEngineFlags::None) { re = Engine::Get(re->info.variant_id); } @@ -706,7 +706,7 @@ void StartupOneEngine(Engine *e, TimerGameCalendar::Date aging_date, uint32_t se /* Get parent variant index for syncing reliability via random seed. */ const Engine *re = e; - while (re->info.variant_id != INVALID_ENGINE && re->info.variant_id != re->index && (re->info.extra_flags & ExtraEngineFlags::SyncReliability) != ExtraEngineFlags::None) { + while (re->info.variant_id != INVALID_ENGINE && (re->info.extra_flags & ExtraEngineFlags::SyncReliability) != ExtraEngineFlags::None) { re = Engine::Get(re->info.variant_id); } diff --git a/src/newgrf.cpp b/src/newgrf.cpp index a89addfcc4..228832dc2d 100644 --- a/src/newgrf.cpp +++ b/src/newgrf.cpp @@ -9125,12 +9125,9 @@ static void FinaliseEngineArray() } } - /* Do final mapping on variant engine ID and set appropriate flags on variant engine */ + /* Do final mapping on variant engine ID. */ if (e->info.variant_id != INVALID_ENGINE) { e->info.variant_id = GetNewEngineID(e->grf_prop.grffile, e->type, e->info.variant_id); - if (e->info.variant_id != INVALID_ENGINE) { - Engine::Get(e->info.variant_id)->display_flags |= EngineDisplayFlags::HasVariants | EngineDisplayFlags::IsFolded; - } } if (!HasBit(e->info.climates, _settings_game.game_creation.landscape)) continue; @@ -9162,6 +9159,26 @@ static void FinaliseEngineArray() } } } + + /* Check engine variants don't point back on themselves (either directly or via a loop) then set appropriate flags + * on variant engine. This is performed separately as all variant engines need to have been resolved. */ + for (Engine *e : Engine::Iterate()) { + EngineID parent = e->info.variant_id; + while (parent != INVALID_ENGINE) { + parent = Engine::Get(parent)->info.variant_id; + if (parent != e->index) continue; + + /* Engine looped back on itself, so clear the variant. */ + e->info.variant_id = INVALID_ENGINE; + + GrfMsg(1, "FinaliseEngineArray: Variant of engine {:x} in '{}' loops back on itself", _engine_mngr[e->index].internal_id, e->GetGRF()->filename); + break; + } + + if (e->info.variant_id != INVALID_ENGINE) { + Engine::Get(e->info.variant_id)->display_flags |= EngineDisplayFlags::HasVariants | EngineDisplayFlags::IsFolded; + } + } } /** Check for invalid cargoes */