From 10e76b27884911de0527b083ad3aa45d7f4391df Mon Sep 17 00:00:00 2001 From: Jonathan G Rennison Date: Fri, 6 Jan 2023 20:21:27 +0000 Subject: [PATCH 01/23] Fix #10032: Capacities of articulated vehicles in build window See also: #9954 --- src/articulated_vehicles.cpp | 35 ---------------- src/autoreplace_cmd.cpp | 2 +- src/autoreplace_gui.cpp | 3 +- src/build_vehicle_gui.cpp | 61 ++++++++++++++++++---------- src/economy.cpp | 2 +- src/engine_func.h | 1 - src/script/api/script_vehicle.cpp | 4 +- src/train_cmd.h | 2 +- src/train_gui.cpp | 2 +- src/vehicle_cmd.cpp | 66 ++++++++++++++++++++----------- src/vehicle_cmd.h | 24 +++++++++-- src/vehicle_gui.cpp | 4 +- src/vehicle_gui.h | 3 ++ 13 files changed, 115 insertions(+), 94 deletions(-) diff --git a/src/articulated_vehicles.cpp b/src/articulated_vehicles.cpp index bdca9a5070..2fdb07a453 100644 --- a/src/articulated_vehicles.cpp +++ b/src/articulated_vehicles.cpp @@ -160,41 +160,6 @@ CargoArray GetCapacityOfArticulatedParts(EngineID engine) return capacity; } -/** - * Get the default cargoes and refits of an articulated vehicle. - * The refits are linked to a cargo rather than an articulated part to prevent a long list of parts. - * @param engine Model to investigate. - * @param[out] cargoes Total amount of units that can be transported, summed by cargo. - * @param[out] refits Whether a (possibly partial) refit for each cargo is possible. - * @param cargo_type Selected refitted cargo type - * @param cargo_capacity Capacity of selected refitted cargo type - */ -void GetArticulatedVehicleCargoesAndRefits(EngineID engine, CargoArray *cargoes, CargoTypes *refits, CargoID cargo_type, uint cargo_capacity) -{ - cargoes->Clear(); - *refits = 0; - - const Engine *e = Engine::Get(engine); - - if (cargo_type < NUM_CARGO && cargo_capacity > 0) { - (*cargoes)[cargo_type] += cargo_capacity; - if (IsEngineRefittable(engine)) SetBit(*refits, cargo_type); - } - - if (!e->IsGroundVehicle() || !HasBit(e->info.callback_mask, CBM_VEHICLE_ARTIC_ENGINE)) return; - - for (uint i = 1; i < MAX_ARTICULATED_PARTS; i++) { - EngineID artic_engine = GetNextArticulatedPart(i, engine); - if (artic_engine == INVALID_ENGINE) break; - - cargo_capacity = GetVehicleDefaultCapacity(artic_engine, &cargo_type); - if (cargo_type < NUM_CARGO && cargo_capacity > 0) { - (*cargoes)[cargo_type] += cargo_capacity; - if (IsEngineRefittable(artic_engine)) SetBit(*refits, cargo_type); - } - } -} - /** * Checks whether any of the articulated parts is refittable * @param engine the first part diff --git a/src/autoreplace_cmd.cpp b/src/autoreplace_cmd.cpp index 654003e39e..4068e743e0 100644 --- a/src/autoreplace_cmd.cpp +++ b/src/autoreplace_cmd.cpp @@ -346,7 +346,7 @@ static CommandCost BuildReplacementVehicle(Vehicle *old_veh, Vehicle **new_vehic /* Build the new vehicle */ VehicleID new_veh_id; - std::tie(cost, new_veh_id, std::ignore, std::ignore) = Command::Do(DC_EXEC | DC_AUTOREPLACE, old_veh->tile, e, true, CT_INVALID, INVALID_CLIENT_ID); + std::tie(cost, new_veh_id, std::ignore, std::ignore, std::ignore) = Command::Do(DC_EXEC | DC_AUTOREPLACE, old_veh->tile, e, true, CT_INVALID, INVALID_CLIENT_ID); if (cost.Failed()) return cost; Vehicle *new_veh = Vehicle::Get(new_veh_id); diff --git a/src/autoreplace_gui.cpp b/src/autoreplace_gui.cpp index 2c8186f294..d53496ca8d 100644 --- a/src/autoreplace_gui.cpp +++ b/src/autoreplace_gui.cpp @@ -523,8 +523,7 @@ public: const Engine *e = Engine::Get(this->sel_engine[side]); TestedEngineDetails ted; ted.cost = 0; - ted.cargo = e->GetDefaultCargoType(); - ted.capacity = e->GetDisplayDefaultCapacity(&ted.mail_capacity); + ted.FillDefaultCapacities(e); const Rect r = this->GetWidget(side == 0 ? WID_RV_LEFT_DETAILS : WID_RV_RIGHT_DETAILS)->GetCurrentRect() .Shrink(WidgetDimensions::scaled.frametext, WidgetDimensions::scaled.framerect); diff --git a/src/build_vehicle_gui.cpp b/src/build_vehicle_gui.cpp index 8b4802aea5..eb4860cd94 100644 --- a/src/build_vehicle_gui.cpp +++ b/src/build_vehicle_gui.cpp @@ -553,18 +553,29 @@ static GUIEngineList::FilterFunction * const _filter_funcs[] = { &CargoAndEngineFilter, }; -static int DrawCargoCapacityInfo(int left, int right, int y, EngineID engine, TestedEngineDetails &te) +static uint GetCargoWeight(const CargoArray &cap, VehicleType vtype) { - CargoArray cap; - CargoTypes refits; - GetArticulatedVehicleCargoesAndRefits(engine, &cap, &refits, te.cargo, te.capacity); - + uint weight = 0; for (CargoID c = 0; c < NUM_CARGO; c++) { - if (cap[c] == 0) continue; + if (cap[c] != 0) { + if (vtype == VEH_TRAIN) { + weight += CargoSpec::Get(c)->WeightOfNUnitsInTrain(cap[c]); + } else { + weight += CargoSpec::Get(c)->WeightOfNUnits(cap[c]); + } + } + } + return weight; +} + +static int DrawCargoCapacityInfo(int left, int right, int y, TestedEngineDetails &te, bool refittable) +{ + for (CargoID c = 0; c < NUM_CARGO; c++) { + if (te.all_capacities[c] == 0) continue; SetDParam(0, c); - SetDParam(1, cap[c]); - SetDParam(2, HasBit(refits, c) ? STR_PURCHASE_INFO_REFITTABLE : STR_EMPTY); + SetDParam(1, te.all_capacities[c]); + SetDParam(2, refittable ? STR_PURCHASE_INFO_REFITTABLE : STR_EMPTY); DrawString(left, right, y, STR_PURCHASE_INFO_CAPACITY); y += FONT_HEIGHT_NORMAL; } @@ -591,8 +602,7 @@ static int DrawRailWagonPurchaseInfo(int left, int right, int y, EngineID engine /* Wagon weight - (including cargo) */ uint weight = e->GetDisplayWeight(); SetDParam(0, weight); - uint cargo_weight = (e->CanCarryCargo() ? CargoSpec::Get(te.cargo)->WeightOfNUnitsInTrain(te.capacity) : 0); - SetDParam(1, cargo_weight + weight); + SetDParam(1, GetCargoWeight(te.all_capacities, VEH_TRAIN) + weight); DrawString(left, right, y, STR_PURCHASE_INFO_WEIGHT_CWEIGHT); y += FONT_HEIGHT_NORMAL; @@ -685,8 +695,7 @@ static int DrawRoadVehPurchaseInfo(int left, int right, int y, EngineID engine_n /* Road vehicle weight - (including cargo) */ int16 weight = e->GetDisplayWeight(); SetDParam(0, weight); - uint cargo_weight = (e->CanCarryCargo() ? CargoSpec::Get(te.cargo)->WeightOfNUnits(te.capacity) : 0); - SetDParam(1, cargo_weight + weight); + SetDParam(1, GetCargoWeight(te.all_capacities, VEH_ROAD) + weight); DrawString(left, right, y, STR_PURCHASE_INFO_WEIGHT_CWEIGHT); y += FONT_HEIGHT_NORMAL; @@ -868,6 +877,21 @@ static uint ShowAdditionalText(int left, int right, int y, EngineID engine) return result; } +void TestedEngineDetails::FillDefaultCapacities(const Engine *e) +{ + this->cargo = e->GetDefaultCargoType(); + if (e->type == VEH_TRAIN || e->type == VEH_ROAD) { + this->all_capacities = GetCapacityOfArticulatedParts(e->index); + this->capacity = this->all_capacities[this->cargo]; + this->mail_capacity = 0; + } else { + this->capacity = e->GetDisplayDefaultCapacity(&this->mail_capacity); + this->all_capacities[this->cargo] = this->capacity; + this->all_capacities[CT_MAIL] = this->mail_capacity; + } + if (this->all_capacities.GetCount() == 0) this->cargo = CT_INVALID; +} + /** * Draw the purchase info details of a vehicle at a given location. * @param left,right,y location where to draw the info @@ -909,7 +933,7 @@ int DrawVehiclePurchaseInfo(int left, int right, int y, EngineID engine_number, if (articulated_cargo) { /* Cargo type + capacity, or N/A */ - int new_y = DrawCargoCapacityInfo(left, right, y, engine_number, te); + int new_y = DrawCargoCapacityInfo(left, right, y, te, refittable); if (new_y == y) { SetDParam(0, CT_INVALID); @@ -1261,28 +1285,23 @@ struct BuildVehicleWindow : Window { if (this->sel_engine == INVALID_ENGINE) return; const Engine *e = Engine::Get(this->sel_engine); - if (!e->CanCarryCargo()) { - this->te.cost = 0; - this->te.cargo = CT_INVALID; - return; - } if (!this->listview_mode) { /* Query for cost and refitted capacity */ - auto [ret, veh_id, refit_capacity, refit_mail] = Command::Do(DC_QUERY_COST, this->window_number, this->sel_engine, true, cargo, INVALID_CLIENT_ID); + auto [ret, veh_id, refit_capacity, refit_mail, cargo_capacities] = Command::Do(DC_QUERY_COST, this->window_number, this->sel_engine, true, cargo, INVALID_CLIENT_ID); if (ret.Succeeded()) { this->te.cost = ret.GetCost() - e->GetCost(); this->te.capacity = refit_capacity; this->te.mail_capacity = refit_mail; this->te.cargo = (cargo == CT_INVALID) ? e->GetDefaultCargoType() : cargo; + this->te.all_capacities = cargo_capacities; return; } } /* Purchase test was not possible or failed, fill in the defaults instead. */ this->te.cost = 0; - this->te.capacity = e->GetDisplayDefaultCapacity(&this->te.mail_capacity); - this->te.cargo = e->GetDefaultCargoType(); + this->te.FillDefaultCapacities(e); } void OnInit() override diff --git a/src/economy.cpp b/src/economy.cpp index 5e8318fd0d..27ab5120f2 100644 --- a/src/economy.cpp +++ b/src/economy.cpp @@ -1494,7 +1494,7 @@ static void HandleStationRefit(Vehicle *v, CargoArray &consist_capleft, Station if (st->goods[cid].cargo.HasCargoFor(next_station)) { /* Try to find out if auto-refitting would succeed. In case the refit is allowed, * the returned refit capacity will be greater than zero. */ - auto [cc, refit_capacity, mail_capacity] = Command::Do(DC_QUERY_COST, v_start->index, cid, 0xFF, true, false, 1); // Auto-refit and only this vehicle including artic parts. + auto [cc, refit_capacity, mail_capacity, cargo_capacities] = Command::Do(DC_QUERY_COST, v_start->index, cid, 0xFF, true, false, 1); // Auto-refit and only this vehicle including artic parts. /* Try to balance different loadable cargoes between parts of the consist, so that * all of them can be loaded. Avoid a situation where all vehicles suddenly switch * to the first loadable cargo for which there is only one packet. If the capacities diff --git a/src/engine_func.h b/src/engine_func.h index 0d69743bfe..25ec83e404 100644 --- a/src/engine_func.h +++ b/src/engine_func.h @@ -24,7 +24,6 @@ extern const uint8 _engine_offsets[4]; bool IsEngineBuildable(EngineID engine, VehicleType type, CompanyID company); bool IsEngineRefittable(EngineID engine); -void GetArticulatedVehicleCargoesAndRefits(EngineID engine, CargoArray *cargoes, CargoTypes *refits, CargoID cargo_type, uint cargo_capacity); void SetYearEngineAgingStops(); void CalcEngineReliability(Engine *e, bool new_month); void StartupOneEngine(Engine *e, Date aging_date, uint32 seed); diff --git a/src/script/api/script_vehicle.cpp b/src/script/api/script_vehicle.cpp index 82db2dfce8..3ff05d9ba6 100644 --- a/src/script/api/script_vehicle.cpp +++ b/src/script/api/script_vehicle.cpp @@ -94,7 +94,7 @@ if (!ScriptEngine::IsBuildable(engine_id)) return -1; if (!ScriptCargo::IsValidCargo(cargo)) return -1; - auto [res, veh_id, refit_capacity, refit_mail] = ::Command::Do(DC_QUERY_COST, depot, engine_id, true, cargo, INVALID_CLIENT_ID); + auto [res, veh_id, refit_capacity, refit_mail, cargo_capacities] = ::Command::Do(DC_QUERY_COST, depot, engine_id, true, cargo, INVALID_CLIENT_ID); return res.Succeeded() ? refit_capacity : -1; } @@ -143,7 +143,7 @@ if (!IsValidVehicle(vehicle_id)) return -1; if (!ScriptCargo::IsValidCargo(cargo)) return -1; - auto [res, refit_capacity, refit_mail] = ::Command::Do(DC_QUERY_COST, vehicle_id, cargo, 0, false, false, 0); + auto [res, refit_capacity, refit_mail, cargo_capacities] = ::Command::Do(DC_QUERY_COST, vehicle_id, cargo, 0, false, false, 0); return res.Succeeded() ? refit_capacity : -1; } diff --git a/src/train_cmd.h b/src/train_cmd.h index 97bd43a74b..cb9e683682 100644 --- a/src/train_cmd.h +++ b/src/train_cmd.h @@ -25,6 +25,6 @@ DEF_CMD_TRAIT(CMD_MOVE_RAIL_VEHICLE, CmdMoveRailVehicle, 0, CMDT_VEH DEF_CMD_TRAIT(CMD_FORCE_TRAIN_PROCEED, CmdForceTrainProceed, 0, CMDT_VEHICLE_MANAGEMENT) DEF_CMD_TRAIT(CMD_REVERSE_TRAIN_DIRECTION, CmdReverseTrainDirection, 0, CMDT_VEHICLE_MANAGEMENT) -void CcBuildWagon(Commands cmd, const CommandCost &result, VehicleID new_veh_id, uint, uint16, TileIndex tile, EngineID, bool, CargoID, ClientID); +void CcBuildWagon(Commands cmd, const CommandCost &result, VehicleID new_veh_id, uint, uint16, CargoArray, TileIndex tile, EngineID, bool, CargoID, ClientID); #endif /* TRAIN_CMD_H */ diff --git a/src/train_gui.cpp b/src/train_gui.cpp index 53130e134b..369da11c36 100644 --- a/src/train_gui.cpp +++ b/src/train_gui.cpp @@ -27,7 +27,7 @@ * @param new_veh_id ID of the ne vehicle. * @param tile The tile the command was executed on. */ -void CcBuildWagon(Commands cmd, const CommandCost &result, VehicleID new_veh_id, uint, uint16, TileIndex tile, EngineID, bool, CargoID, ClientID) +void CcBuildWagon(Commands cmd, const CommandCost &result, VehicleID new_veh_id, uint, uint16, CargoArray, TileIndex tile, EngineID, bool, CargoID, ClientID) { if (result.Failed()) return; diff --git a/src/vehicle_cmd.cpp b/src/vehicle_cmd.cpp index ed7cf56689..fa1c6e54c7 100644 --- a/src/vehicle_cmd.cpp +++ b/src/vehicle_cmd.cpp @@ -84,25 +84,25 @@ const StringID _send_to_depot_msg_table[] = { * @param client_id User * @return the cost of this operation + the new vehicle ID + the refitted capacity + the refitted mail capacity (aircraft) or an error */ -std::tuple CmdBuildVehicle(DoCommandFlag flags, TileIndex tile, EngineID eid, bool use_free_vehicles, CargoID cargo, ClientID client_id) +std::tuple CmdBuildVehicle(DoCommandFlag flags, TileIndex tile, EngineID eid, bool use_free_vehicles, CargoID cargo, ClientID client_id) { /* Elementary check for valid location. */ - if (!IsDepotTile(tile) || !IsTileOwner(tile, _current_company)) return { CMD_ERROR, INVALID_VEHICLE, 0, 0 }; + if (!IsDepotTile(tile) || !IsTileOwner(tile, _current_company)) return { CMD_ERROR, INVALID_VEHICLE, 0, 0, {} }; VehicleType type = GetDepotVehicleType(tile); /* Validate the engine type. */ - if (!IsEngineBuildable(eid, type, _current_company)) return { CommandCost(STR_ERROR_RAIL_VEHICLE_NOT_AVAILABLE + type), INVALID_VEHICLE, 0, 0 }; + if (!IsEngineBuildable(eid, type, _current_company)) return { CommandCost(STR_ERROR_RAIL_VEHICLE_NOT_AVAILABLE + type), INVALID_VEHICLE, 0, 0, {} }; /* Validate the cargo type. */ - if (cargo >= NUM_CARGO && cargo != CT_INVALID) return { CMD_ERROR, INVALID_VEHICLE, 0, 0 }; + if (cargo >= NUM_CARGO && cargo != CT_INVALID) return { CMD_ERROR, INVALID_VEHICLE, 0, 0, {} }; const Engine *e = Engine::Get(eid); CommandCost value(EXPENSES_NEW_VEHICLES, e->GetCost()); /* Engines without valid cargo should not be available */ CargoID default_cargo = e->GetDefaultCargoType(); - if (default_cargo == CT_INVALID) return { CMD_ERROR, INVALID_VEHICLE, 0, 0 }; + if (default_cargo == CT_INVALID) return { CMD_ERROR, INVALID_VEHICLE, 0, 0, {} }; bool refitting = cargo != CT_INVALID && cargo != default_cargo; @@ -115,13 +115,13 @@ std::tuple CmdBuildVehicle(DoCommandFlag f case VEH_AIRCRAFT: num_vehicles = e->u.air.subtype & AIR_CTOL ? 2 : 3; break; default: NOT_REACHED(); // Safe due to IsDepotTile() } - if (!Vehicle::CanAllocateItem(num_vehicles)) return { CommandCost(STR_ERROR_TOO_MANY_VEHICLES_IN_GAME), INVALID_VEHICLE, 0, 0 }; + if (!Vehicle::CanAllocateItem(num_vehicles)) return { CommandCost(STR_ERROR_TOO_MANY_VEHICLES_IN_GAME), INVALID_VEHICLE, 0, 0, {} }; /* Check whether we can allocate a unit number. Autoreplace does not allocate * an unit number as it will (always) reuse the one of the replaced vehicle * and (train) wagons don't have an unit number in any scenario. */ UnitID unit_num = (flags & DC_QUERY_COST || flags & DC_AUTOREPLACE || (type == VEH_TRAIN && e->u.rail.railveh_type == RAILVEH_WAGON)) ? 0 : GetFreeUnitNumber(type); - if (unit_num == UINT16_MAX) return { CommandCost(STR_ERROR_TOO_MANY_VEHICLES_IN_GAME), INVALID_VEHICLE, 0, 0 }; + if (unit_num == UINT16_MAX) return { CommandCost(STR_ERROR_TOO_MANY_VEHICLES_IN_GAME), INVALID_VEHICLE, 0, 0, {} }; /* If we are refitting we need to temporarily purchase the vehicle to be able to * test it. */ @@ -145,6 +145,7 @@ std::tuple CmdBuildVehicle(DoCommandFlag f VehicleID veh_id = INVALID_VEHICLE; uint refitted_capacity = 0; uint16 refitted_mail_capacity = 0; + CargoArray cargo_capacities; if (value.Succeeded()) { if (subflags & DC_EXEC) { v->unitnumber = unit_num; @@ -155,11 +156,20 @@ std::tuple CmdBuildVehicle(DoCommandFlag f if (refitting) { /* Refit only one vehicle. If we purchased an engine, it may have gained free wagons. */ CommandCost cc; - std::tie(cc, refitted_capacity, refitted_mail_capacity) = CmdRefitVehicle(flags, v->index, cargo, 0, false, false, 1); + std::tie(cc, refitted_capacity, refitted_mail_capacity, cargo_capacities) = CmdRefitVehicle(flags, v->index, cargo, 0, false, false, 1); value.AddCost(cc); } else { /* Fill in non-refitted capacities */ - refitted_capacity = e->GetDisplayDefaultCapacity(&refitted_mail_capacity); + if (e->type == VEH_TRAIN || e->type == VEH_ROAD) { + cargo_capacities = GetCapacityOfArticulatedParts(eid); + refitted_capacity = cargo_capacities[default_cargo]; + refitted_mail_capacity = 0; + } else { + refitted_capacity = e->GetDisplayDefaultCapacity(&refitted_mail_capacity); + cargo_capacities.Clear(); + cargo_capacities[default_cargo] = refitted_capacity; + cargo_capacities[CT_MAIL] = refitted_mail_capacity; + } } if (flags & DC_EXEC) { @@ -191,7 +201,7 @@ std::tuple CmdBuildVehicle(DoCommandFlag f /* Only restore if we actually did some refitting */ if (flags != subflags) RestoreRandomSeeds(saved_seeds); - return { value, veh_id, refitted_capacity, refitted_mail_capacity }; + return { value, veh_id, refitted_capacity, refitted_mail_capacity, cargo_capacities }; } /** @@ -339,12 +349,13 @@ struct RefitResult { * @param auto_refit Refitting is done as automatic refitting outside a depot. * @return Refit cost + refittet capacity + mail capacity (aircraft). */ -static std::tuple RefitVehicle(Vehicle *v, bool only_this, uint8 num_vehicles, CargoID new_cid, byte new_subtype, DoCommandFlag flags, bool auto_refit) +static std::tuple RefitVehicle(Vehicle *v, bool only_this, uint8 num_vehicles, CargoID new_cid, byte new_subtype, DoCommandFlag flags, bool auto_refit) { CommandCost cost(v->GetExpenseType(false)); uint total_capacity = 0; uint total_mail_capacity = 0; num_vehicles = num_vehicles == 0 ? UINT8_MAX : num_vehicles; + CargoArray cargo_capacities; VehicleSet vehicles_to_refit; if (!only_this) { @@ -369,7 +380,11 @@ static std::tuple RefitVehicle(Vehicle *v, bool only_ /* If the vehicle is not refittable, or does not allow automatic refitting, * count its capacity nevertheless if the cargo matches */ bool refittable = HasBit(e->info.refit_mask, new_cid) && (!auto_refit || HasBit(e->info.misc_flags, EF_AUTO_REFIT)); - if (!refittable && v->cargo_type != new_cid) continue; + if (!refittable && v->cargo_type != new_cid) { + uint amount = e->DetermineCapacity(v, nullptr); + if (amount > 0) cargo_capacities[v->cargo_type] += amount; + continue; + } /* Determine best fitting subtype if requested */ if (actual_subtype == 0xFF) { @@ -390,6 +405,9 @@ static std::tuple RefitVehicle(Vehicle *v, bool only_ /* mail_capacity will always be zero if the vehicle is not an aircraft. */ total_mail_capacity += mail_capacity; + cargo_capacities[new_cid] += amount; + cargo_capacities[CT_MAIL] += mail_capacity; + if (!refittable) continue; /* Restore the original cargo type */ @@ -445,7 +463,7 @@ static std::tuple RefitVehicle(Vehicle *v, bool only_ } refit_result.clear(); - return { cost, total_capacity, total_mail_capacity }; + return { cost, total_capacity, total_mail_capacity, cargo_capacities }; } /** @@ -460,42 +478,42 @@ static std::tuple RefitVehicle(Vehicle *v, bool only_ * Only used if "refit only this vehicle" is false. * @return the cost of this operation or an error */ -std::tuple CmdRefitVehicle(DoCommandFlag flags, VehicleID veh_id, CargoID new_cid, byte new_subtype, bool auto_refit, bool only_this, uint8 num_vehicles) +std::tuple CmdRefitVehicle(DoCommandFlag flags, VehicleID veh_id, CargoID new_cid, byte new_subtype, bool auto_refit, bool only_this, uint8 num_vehicles) { Vehicle *v = Vehicle::GetIfValid(veh_id); - if (v == nullptr) return { CMD_ERROR, 0, 0 }; + if (v == nullptr) return { CMD_ERROR, 0, 0, {} }; /* Don't allow disasters and sparks and such to be refitted. * We cannot check for IsPrimaryVehicle as autoreplace also refits in free wagon chains. */ - if (!IsCompanyBuildableVehicleType(v->type)) return { CMD_ERROR, 0, 0 }; + if (!IsCompanyBuildableVehicleType(v->type)) return { CMD_ERROR, 0, 0, {} }; Vehicle *front = v->First(); CommandCost ret = CheckOwnership(front->owner); - if (ret.Failed()) return { ret, 0, 0 }; + if (ret.Failed()) return { ret, 0, 0, {} }; bool free_wagon = v->type == VEH_TRAIN && Train::From(front)->IsFreeWagon(); // used by autoreplace/renew /* Don't allow shadows and such to be refitted. */ - if (v != front && (v->type == VEH_SHIP || v->type == VEH_AIRCRAFT)) return { CMD_ERROR, 0, 0 }; + if (v != front && (v->type == VEH_SHIP || v->type == VEH_AIRCRAFT)) return { CMD_ERROR, 0, 0, {} }; /* Allow auto-refitting only during loading and normal refitting only in a depot. */ if ((flags & DC_QUERY_COST) == 0 && // used by the refit GUI, including the order refit GUI. !free_wagon && // used by autoreplace/renew (!auto_refit || !front->current_order.IsType(OT_LOADING)) && // refit inside stations !front->IsStoppedInDepot()) { // refit inside depots - return { CommandCost(STR_ERROR_TRAIN_MUST_BE_STOPPED_INSIDE_DEPOT + front->type), 0, 0}; + return { CommandCost(STR_ERROR_TRAIN_MUST_BE_STOPPED_INSIDE_DEPOT + front->type), 0, 0, {} }; } - if (front->vehstatus & VS_CRASHED) return { CommandCost(STR_ERROR_VEHICLE_IS_DESTROYED), 0, 0}; + if (front->vehstatus & VS_CRASHED) return { CommandCost(STR_ERROR_VEHICLE_IS_DESTROYED), 0, 0, {} }; /* Check cargo */ - if (new_cid >= NUM_CARGO) return { CMD_ERROR, 0, 0 }; + if (new_cid >= NUM_CARGO) return { CMD_ERROR, 0, 0, {} }; /* For ships and aircraft there is always only one. */ only_this |= front->type == VEH_SHIP || front->type == VEH_AIRCRAFT; - auto [cost, refit_capacity, mail_capacity] = RefitVehicle(v, only_this, num_vehicles, new_cid, new_subtype, flags, auto_refit); + auto [cost, refit_capacity, mail_capacity, cargo_capacities] = RefitVehicle(v, only_this, num_vehicles, new_cid, new_subtype, flags, auto_refit); if (flags & DC_EXEC) { /* Update the cached variables */ @@ -532,7 +550,7 @@ std::tuple CmdRefitVehicle(DoCommandFlag flags, Vehic v->InvalidateNewGRFCacheOfChain(); } - return { cost, refit_capacity, mail_capacity }; + return { cost, refit_capacity, mail_capacity, cargo_capacities }; } /** @@ -851,7 +869,7 @@ std::tuple CmdCloneVehicle(DoCommandFlag flags, TileInde if ((flags & DC_EXEC) && !v->IsPrimaryVehicle()) build_flags |= DC_AUTOREPLACE; CommandCost cost; - std::tie(cost, new_veh_id, std::ignore, std::ignore) = Command::Do(build_flags, tile, v->engine_type, false, CT_INVALID, INVALID_CLIENT_ID); + std::tie(cost, new_veh_id, std::ignore, std::ignore, std::ignore) = Command::Do(build_flags, tile, v->engine_type, false, CT_INVALID, INVALID_CLIENT_ID); if (cost.Failed()) { /* Can't build a part, then sell the stuff we already made; clear up the mess */ diff --git a/src/vehicle_cmd.h b/src/vehicle_cmd.h index 2f203b8652..0b9318d6b5 100644 --- a/src/vehicle_cmd.h +++ b/src/vehicle_cmd.h @@ -14,10 +14,11 @@ #include "engine_type.h" #include "vehicle_type.h" #include "vehiclelist.h" +#include "cargo_type.h" -std::tuple CmdBuildVehicle(DoCommandFlag flags, TileIndex tile, EngineID eid, bool use_free_vehicles, CargoID cargo, ClientID client_id); +std::tuple CmdBuildVehicle(DoCommandFlag flags, TileIndex tile, EngineID eid, bool use_free_vehicles, CargoID cargo, ClientID client_id); CommandCost CmdSellVehicle(DoCommandFlag flags, VehicleID v_id, bool sell_chain, bool backup_order, ClientID client_id); -std::tuple CmdRefitVehicle(DoCommandFlag flags, VehicleID veh_id, CargoID new_cid, byte new_subtype, bool auto_refit, bool only_this, uint8 num_vehicles); +std::tuple CmdRefitVehicle(DoCommandFlag flags, VehicleID veh_id, CargoID new_cid, byte new_subtype, bool auto_refit, bool only_this, uint8 num_vehicles); CommandCost CmdSendVehicleToDepot(DoCommandFlag flags, VehicleID veh_id, DepotCommand depot_cmd, const VehicleListIdentifier &vli); CommandCost CmdChangeServiceInt(DoCommandFlag flags, VehicleID veh_id, uint16 serv_int, bool is_custom, bool is_percent); CommandCost CmdRenameVehicle(DoCommandFlag flags, VehicleID veh_id, const std::string &text); @@ -39,7 +40,7 @@ DEF_CMD_TRAIT(CMD_MASS_START_STOP, CmdMassStartStopVehicle, 0, DEF_CMD_TRAIT(CMD_DEPOT_SELL_ALL_VEHICLES, CmdDepotSellAllVehicles, 0, CMDT_VEHICLE_CONSTRUCTION) DEF_CMD_TRAIT(CMD_DEPOT_MASS_AUTOREPLACE, CmdDepotMassAutoReplace, 0, CMDT_VEHICLE_CONSTRUCTION) -void CcBuildPrimaryVehicle(Commands cmd, const CommandCost &result, VehicleID new_veh_id, uint, uint16); +void CcBuildPrimaryVehicle(Commands cmd, const CommandCost &result, VehicleID new_veh_id, uint, uint16, CargoArray); void CcStartStopVehicle(Commands cmd, const CommandCost &result, VehicleID veh_id, bool); template @@ -53,4 +54,21 @@ inline EndianBufferReader &operator >>(EndianBufferReader &buffer, VehicleListId return buffer >> vli.type >> vli.vtype >> vli.company >> vli.index; } +template +inline EndianBufferWriter &operator <<(EndianBufferWriter &buffer, const CargoArray &cargo_array) +{ + for (CargoID c = 0; c < NUM_CARGO; c++) { + buffer << cargo_array[c]; + } + return buffer; +} + +inline EndianBufferReader &operator >>(EndianBufferReader &buffer, CargoArray &cargo_array) +{ + for (CargoID c = 0; c < NUM_CARGO; c++) { + buffer >> cargo_array[c]; + } + return buffer; +} + #endif /* VEHICLE_CMD_H */ diff --git a/src/vehicle_gui.cpp b/src/vehicle_gui.cpp index c7273a885a..42633aff51 100644 --- a/src/vehicle_gui.cpp +++ b/src/vehicle_gui.cpp @@ -892,7 +892,7 @@ struct RefitWindow : public Window { StringID GetCapacityString(RefitOption *option) const { assert(_current_company == _local_company); - auto [cost, refit_capacity, mail_capacity] = Command::Do(DC_QUERY_COST, this->selected_vehicle, option->cargo, option->subtype, this->auto_refit, false, this->num_vehicles); + auto [cost, refit_capacity, mail_capacity, cargo_capacities] = Command::Do(DC_QUERY_COST, this->selected_vehicle, option->cargo, option->subtype, this->auto_refit, false, this->num_vehicles); if (cost.Failed()) return INVALID_STRING_ID; @@ -3356,7 +3356,7 @@ void StopGlobalFollowVehicle(const Vehicle *v) * @param result indicates completion (or not) of the operation * @param new_veh_id ID of the new vehicle. */ -void CcBuildPrimaryVehicle(Commands cmd, const CommandCost &result, VehicleID new_veh_id, uint, uint16) +void CcBuildPrimaryVehicle(Commands cmd, const CommandCost &result, VehicleID new_veh_id, uint, uint16, CargoArray) { if (result.Failed()) return; diff --git a/src/vehicle_gui.h b/src/vehicle_gui.h index af8a354e6f..1c66e9671a 100644 --- a/src/vehicle_gui.h +++ b/src/vehicle_gui.h @@ -43,6 +43,9 @@ struct TestedEngineDetails { CargoID cargo; ///< Cargo type uint capacity; ///< Cargo capacity uint16 mail_capacity; ///< Mail capacity if available + CargoArray all_capacities; ///< Capacities for all cargoes + + void FillDefaultCapacities(const Engine *e); }; int DrawVehiclePurchaseInfo(int left, int right, int y, EngineID engine_number, TestedEngineDetails &te); From 5ddfd38de6ae30f86c38d381864e3dcd497315ac Mon Sep 17 00:00:00 2001 From: translators Date: Sat, 14 Jan 2023 18:43:25 +0000 Subject: [PATCH 02/23] Update: Translations from eints norwegian (bokmal): 1 change by buzzCraft --- src/lang/norwegian_bokmal.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lang/norwegian_bokmal.txt b/src/lang/norwegian_bokmal.txt index 9aa94e6f80..29945b6834 100644 --- a/src/lang/norwegian_bokmal.txt +++ b/src/lang/norwegian_bokmal.txt @@ -2064,6 +2064,7 @@ STR_INTRO_TOOLTIP_HIGHSCORE :{BLACK}Vise tav STR_INTRO_TOOLTIP_CONFIG_SETTINGS_TREE :{BLACK}Vis innstillinger STR_INTRO_TOOLTIP_NEWGRF_SETTINGS :{BLACK}Vis NewGRF-instillinger STR_INTRO_TOOLTIP_ONLINE_CONTENT :{BLACK}Se etter nytt og oppdatert innhold for nedlasting +STR_INTRO_TOOLTIP_AI_SETTINGS :{BLACK}Vis KI innstillinger STR_INTRO_TOOLTIP_QUIT :{BLACK}Avslutt 'OpenTTD' STR_INTRO_BASESET :{BLACK}Det valgte innebygde grafikksettet mangler {NUM} sprite{P "" r}. Se etter oppdateringer for settet. From 9c70c38c5e9b4a299a08ed2790ff45ebbdf2d144 Mon Sep 17 00:00:00 2001 From: Rubidium Date: Sat, 7 Jan 2023 10:16:52 +0100 Subject: [PATCH 03/23] Fix: check for the existence of shadow and rotor vehicles for aircraft Instead of just assuming that it exists in the savegame that got loaded. --- src/saveload/vehicle_sl.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/saveload/vehicle_sl.cpp b/src/saveload/vehicle_sl.cpp index 5254f94d21..86e3d83ada 100644 --- a/src/saveload/vehicle_sl.cpp +++ b/src/saveload/vehicle_sl.cpp @@ -454,13 +454,17 @@ void AfterLoadVehicles(bool part_of_load) if (Aircraft::From(v)->IsNormalAircraft()) { v->GetImage(v->direction, EIT_ON_MAP, &v->sprite_cache.sprite_seq); - /* The plane's shadow will have the same image as the plane, but no colour */ + /* The aircraft's shadow will have the same image as the aircraft, but no colour */ Vehicle *shadow = v->Next(); + if (shadow == nullptr) SlErrorCorrupt("Missing shadow for aircraft"); + shadow->sprite_cache.sprite_seq.CopyWithoutPalette(v->sprite_cache.sprite_seq); /* In the case of a helicopter we will update the rotor sprites */ if (v->subtype == AIR_HELICOPTER) { Vehicle *rotor = shadow->Next(); + if (rotor == nullptr) SlErrorCorrupt("Missing rotor for helicopter"); + GetRotorImage(Aircraft::From(v), EIT_ON_MAP, &rotor->sprite_cache.sprite_seq); } From bcfe0fb076bcbacfb7cdc13d88d23997b4713097 Mon Sep 17 00:00:00 2001 From: Rubidium Date: Fri, 6 Jan 2023 23:24:38 +0100 Subject: [PATCH 04/23] Codechange: introduce GetMainWindow() to properly account for nullptr checks Some nullptr checks have been removed as they were not triggered with nullptr with the null video driver and in dedicated server mode. --- src/console_cmds.cpp | 2 +- src/error_gui.cpp | 2 +- src/genworld.cpp | 4 +--- src/intro_gui.cpp | 2 +- src/linkgraph/linkgraph_gui.cpp | 2 +- src/main_gui.cpp | 2 +- src/misc_gui.cpp | 2 +- src/network/network_gui.cpp | 2 +- src/saveload/misc_sl.cpp | 12 +++++------- src/screenshot.cpp | 4 ++-- src/smallmap_gui.cpp | 8 ++++---- src/toolbar_gui.cpp | 8 ++++---- src/vehicle_gui.cpp | 12 ++++++------ src/viewport_gui.cpp | 6 +++--- src/window.cpp | 18 ++++++++++++++---- src/window_func.h | 1 + 16 files changed, 47 insertions(+), 40 deletions(-) diff --git a/src/console_cmds.cpp b/src/console_cmds.cpp index 33b47d9600..e652e64111 100644 --- a/src/console_cmds.cpp +++ b/src/console_cmds.cpp @@ -295,7 +295,7 @@ DEF_CONSOLE_CMD(ConZoomToLevel) } else if (level > _settings_client.gui.zoom_max) { IConsolePrint(CC_ERROR, "Current client settings do not allow zooming out beyond level {}.", _settings_client.gui.zoom_max); } else { - Window *w = FindWindowById(WC_MAIN_WINDOW, 0); + Window *w = GetMainWindow(); Viewport *vp = w->viewport; while (vp->zoom > level) DoZoomInOutWindow(ZOOM_IN, w); while (vp->zoom < level) DoZoomInOutWindow(ZOOM_OUT, w); diff --git a/src/error_gui.cpp b/src/error_gui.cpp index 77a96bfc7d..7e04db7c56 100644 --- a/src/error_gui.cpp +++ b/src/error_gui.cpp @@ -235,7 +235,7 @@ public: int scr_bot = GetMainViewBottom() - 20; Point pt = RemapCoords(this->position.x, this->position.y, GetSlopePixelZOutsideMap(this->position.x, this->position.y)); - const Viewport *vp = FindWindowById(WC_MAIN_WINDOW, 0)->viewport; + const Viewport *vp = GetMainWindow()->viewport; if (this->face == INVALID_COMPANY) { /* move x pos to opposite corner */ pt.x = UnScaleByZoom(pt.x - vp->virtual_left, vp->zoom) + vp->left; diff --git a/src/genworld.cpp b/src/genworld.cpp index 8ff7da064a..711a9868ba 100644 --- a/src/genworld.cpp +++ b/src/genworld.cpp @@ -321,9 +321,7 @@ void GenerateWorld(GenWorldMode mode, uint size_x, uint size_y, bool reset_setti ShowGenerateWorldProgress(); /* Centre the view on the map */ - if (FindWindowById(WC_MAIN_WINDOW, 0) != nullptr) { - ScrollMainWindowToTile(TileXY(MapSizeX() / 2, MapSizeY() / 2), true); - } + ScrollMainWindowToTile(TileXY(MapSizeX() / 2, MapSizeY() / 2), true); _GenerateWorld(); } diff --git a/src/intro_gui.cpp b/src/intro_gui.cpp index 7e86c1e53c..1d84732bf7 100644 --- a/src/intro_gui.cpp +++ b/src/intro_gui.cpp @@ -222,7 +222,7 @@ struct SelectGameWindow : public Window { } IntroGameViewportCommand &vc = intro_viewport_commands[this->cur_viewport_command_index]; - Window *mw = FindWindowByClass(WC_MAIN_WINDOW); + Window *mw = GetMainWindow(); Viewport *vp = mw->viewport; /* Early exit if the current command hasn't elapsed and isn't animated. */ diff --git a/src/linkgraph/linkgraph_gui.cpp b/src/linkgraph/linkgraph_gui.cpp index 8752894b04..ee7cdc05dd 100644 --- a/src/linkgraph/linkgraph_gui.cpp +++ b/src/linkgraph/linkgraph_gui.cpp @@ -553,7 +553,7 @@ LinkGraphLegendWindow::LinkGraphLegendWindow(WindowDesc *desc, int window_number { this->InitNested(window_number); this->InvalidateData(0); - this->SetOverlay(FindWindowById(WC_MAIN_WINDOW, 0)->viewport->overlay); + this->SetOverlay(GetMainWindow()->viewport->overlay); } /** diff --git a/src/main_gui.cpp b/src/main_gui.cpp index 33d3d5d311..683d4f9d97 100644 --- a/src/main_gui.cpp +++ b/src/main_gui.cpp @@ -157,7 +157,7 @@ void FixTitleGameZoom(int zoom_adjust) { if (_game_mode != GM_MENU) return; - Viewport *vp = FindWindowByClass(WC_MAIN_WINDOW)->viewport; + Viewport *vp = GetMainWindow()->viewport; /* Adjust the zoom in/out. * Can't simply add, since operator+ is not defined on the ZoomLevel type. */ diff --git a/src/misc_gui.cpp b/src/misc_gui.cpp index 2641f7227e..352cbf7c2a 100644 --- a/src/misc_gui.cpp +++ b/src/misc_gui.cpp @@ -1264,7 +1264,7 @@ static WindowDesc _query_desc( */ void ShowQuery(StringID caption, StringID message, Window *parent, QueryCallbackProc *callback) { - if (parent == nullptr) parent = FindWindowById(WC_MAIN_WINDOW, 0); + if (parent == nullptr) parent = GetMainWindow(); for (Window *w : Window::Iterate()) { if (w->window_class != WC_CONFIRM_POPUP_QUERY) continue; diff --git a/src/network/network_gui.cpp b/src/network/network_gui.cpp index cc9adaa01a..6591cde3bd 100644 --- a/src/network/network_gui.cpp +++ b/src/network/network_gui.cpp @@ -2517,6 +2517,6 @@ void ShowNetworkAskRelay(const std::string &server_connection_string, const std: { CloseWindowByClass(WC_NETWORK_ASK_RELAY); - Window *parent = FindWindowById(WC_MAIN_WINDOW, 0); + Window *parent = GetMainWindow(); new NetworkAskRelayWindow(&_network_ask_relay_desc, parent, server_connection_string, relay_connection_string, token); } diff --git a/src/saveload/misc_sl.cpp b/src/saveload/misc_sl.cpp index f50b03fe41..e537bee019 100644 --- a/src/saveload/misc_sl.cpp +++ b/src/saveload/misc_sl.cpp @@ -34,18 +34,16 @@ ZoomLevel _saved_scrollpos_zoom; void SaveViewportBeforeSaveGame() { - const Window *w = FindWindowById(WC_MAIN_WINDOW, 0); + const Window *w = GetMainWindow(); - if (w != nullptr) { - _saved_scrollpos_x = w->viewport->scrollpos_x; - _saved_scrollpos_y = w->viewport->scrollpos_y; - _saved_scrollpos_zoom = w->viewport->zoom; - } + _saved_scrollpos_x = w->viewport->scrollpos_x; + _saved_scrollpos_y = w->viewport->scrollpos_y; + _saved_scrollpos_zoom = w->viewport->zoom; } void ResetViewportAfterLoadGame() { - Window *w = FindWindowById(WC_MAIN_WINDOW, 0); + Window *w = GetMainWindow(); w->viewport->scrollpos_x = _saved_scrollpos_x; w->viewport->scrollpos_y = _saved_scrollpos_y; diff --git a/src/screenshot.cpp b/src/screenshot.cpp index 91a16d835b..1a8d0d3c4e 100644 --- a/src/screenshot.cpp +++ b/src/screenshot.cpp @@ -732,7 +732,7 @@ void SetupScreenshotViewport(ScreenshotType t, Viewport *vp, uint32 width, uint3 case SC_CRASHLOG: { assert(width == 0 && height == 0); - Window *w = FindWindowById(WC_MAIN_WINDOW, 0); + Window *w = GetMainWindow(); vp->virtual_left = w->viewport->virtual_left; vp->virtual_top = w->viewport->virtual_top; vp->virtual_width = w->viewport->virtual_width; @@ -776,7 +776,7 @@ void SetupScreenshotViewport(ScreenshotType t, Viewport *vp, uint32 width, uint3 default: { vp->zoom = (t == SC_ZOOMEDIN) ? _settings_client.gui.zoom_min : ZOOM_LVL_VIEWPORT; - Window *w = FindWindowById(WC_MAIN_WINDOW, 0); + Window *w = GetMainWindow(); vp->virtual_left = w->viewport->virtual_left; vp->virtual_top = w->viewport->virtual_top; diff --git a/src/smallmap_gui.cpp b/src/smallmap_gui.cpp index 67aec0e430..dd2e0749cd 100644 --- a/src/smallmap_gui.cpp +++ b/src/smallmap_gui.cpp @@ -941,7 +941,7 @@ void SmallMapWindow::DrawTowns(const DrawPixelInfo *dpi) const void SmallMapWindow::DrawMapIndicators() const { /* Find main viewport. */ - const Viewport *vp = FindWindowById(WC_MAIN_WINDOW, 0)->viewport; + const Viewport *vp = GetMainWindow()->viewport; Point upper_left_smallmap_coord = InverseRemapCoords2(vp->virtual_left, vp->virtual_top); Point lower_right_smallmap_coord = InverseRemapCoords2(vp->virtual_left + vp->virtual_width - 1, vp->virtual_top + vp->virtual_height - 1); @@ -1433,7 +1433,7 @@ int SmallMapWindow::GetPositionOnLegend(Point pt) if (click_count > 0) this->mouse_capture_widget = widget; const NWidgetBase *wid = this->GetWidget(WID_SM_MAP); - Window *w = FindWindowById(WC_MAIN_WINDOW, 0); + Window *w = GetMainWindow(); int sub; pt = this->PixelToTile(pt.x - wid->pos_x, pt.y - wid->pos_y, &sub); ScrollWindowTo(this->scroll_x + pt.x * TILE_SIZE, this->scroll_y + pt.y * TILE_SIZE, -1, w); @@ -1665,7 +1665,7 @@ void SmallMapWindow::SetNewScroll(int sx, int sy, int sub) */ void SmallMapWindow::SmallMapCenterOnCurrentPos() { - const Viewport *vp = FindWindowById(WC_MAIN_WINDOW, 0)->viewport; + const Viewport *vp = GetMainWindow()->viewport; Point viewport_center = InverseRemapCoords2(vp->virtual_left + vp->virtual_width / 2, vp->virtual_top + vp->virtual_height / 2); int sub; @@ -1882,7 +1882,7 @@ void ShowSmallMap() */ bool ScrollMainWindowTo(int x, int y, int z, bool instant) { - bool res = ScrollWindowTo(x, y, z, FindWindowById(WC_MAIN_WINDOW, 0), instant); + bool res = ScrollWindowTo(x, y, z, GetMainWindow(), instant); /* If a user scrolls to a tile (via what way what so ever) and already is on * that tile (e.g.: pressed twice), move the smallmap to that location, diff --git a/src/toolbar_gui.cpp b/src/toolbar_gui.cpp index c0967054b7..9d1aad649a 100644 --- a/src/toolbar_gui.cpp +++ b/src/toolbar_gui.cpp @@ -884,7 +884,7 @@ static CallBackFunction MenuClickShowAir(int index) static CallBackFunction ToolbarZoomInClick(Window *w) { - if (DoZoomInOutWindow(ZOOM_IN, FindWindowById(WC_MAIN_WINDOW, 0))) { + if (DoZoomInOutWindow(ZOOM_IN, GetMainWindow())) { w->HandleButtonClick((_game_mode == GM_EDITOR) ? (byte)WID_TE_ZOOM_IN : (byte)WID_TN_ZOOM_IN); if (_settings_client.sound.click_beep) SndPlayFx(SND_15_BEEP); } @@ -895,7 +895,7 @@ static CallBackFunction ToolbarZoomInClick(Window *w) static CallBackFunction ToolbarZoomOutClick(Window *w) { - if (DoZoomInOutWindow(ZOOM_OUT, FindWindowById(WC_MAIN_WINDOW, 0))) { + if (DoZoomInOutWindow(ZOOM_OUT, GetMainWindow())) { w->HandleButtonClick((_game_mode == GM_EDITOR) ? (byte)WID_TE_ZOOM_OUT : (byte)WID_TN_ZOOM_OUT); if (_settings_client.sound.click_beep) SndPlayFx(SND_15_BEEP); } @@ -2146,7 +2146,7 @@ struct MainToolbarWindow : Window { void OnInvalidateData(int data = 0, bool gui_scope = true) override { if (!gui_scope) return; - if (FindWindowById(WC_MAIN_WINDOW, 0) != nullptr) HandleZoomMessage(this, FindWindowById(WC_MAIN_WINDOW, 0)->viewport, WID_TN_ZOOM_IN, WID_TN_ZOOM_OUT); + HandleZoomMessage(this, GetMainWindow()->viewport, WID_TN_ZOOM_IN, WID_TN_ZOOM_OUT); } static HotkeyList hotkeys; @@ -2520,7 +2520,7 @@ struct ScenarioEditorToolbarWindow : Window { void OnInvalidateData(int data = 0, bool gui_scope = true) override { if (!gui_scope) return; - if (FindWindowById(WC_MAIN_WINDOW, 0) != nullptr) HandleZoomMessage(this, FindWindowById(WC_MAIN_WINDOW, 0)->viewport, WID_TE_ZOOM_IN, WID_TE_ZOOM_OUT); + HandleZoomMessage(this, GetMainWindow()->viewport, WID_TE_ZOOM_IN, WID_TE_ZOOM_OUT); } void OnQueryTextFinished(char *str) override diff --git a/src/vehicle_gui.cpp b/src/vehicle_gui.cpp index 42633aff51..f8c9bce791 100644 --- a/src/vehicle_gui.cpp +++ b/src/vehicle_gui.cpp @@ -3118,7 +3118,7 @@ public: if (_ctrl_pressed) { ShowExtraViewportWindow(TileVirtXY(v->x_pos, v->y_pos)); } else { - const Window *mainwindow = FindWindowById(WC_MAIN_WINDOW, 0); + const Window *mainwindow = GetMainWindow(); if (click_count > 1 && mainwindow->viewport->zoom <= ZOOM_LVL_OUT_4X) { /* main window 'follows' vehicle */ mainwindow->viewport->follow_vehicle = v->index; @@ -3176,9 +3176,9 @@ public: { /* If the hotkey is not for any widget in the UI (i.e. for honking) */ if (hotkey == WID_VV_HONK_HORN) { - const Window* mainwindow = FindWindowById(WC_MAIN_WINDOW, 0); - const Vehicle* v = Vehicle::Get(window_number); - /*Only play the sound if we're following this vehicle */ + const Window *mainwindow = GetMainWindow(); + const Vehicle *v = Vehicle::Get(window_number); + /* Only play the sound if we're following this vehicle */ if (mainwindow->viewport->follow_vehicle == v->index) { v->PlayLeaveStationSound(true); } @@ -3342,8 +3342,8 @@ bool VehicleClicked(const GUIVehicleGroup &vehgroup) void StopGlobalFollowVehicle(const Vehicle *v) { - Window *w = FindWindowById(WC_MAIN_WINDOW, 0); - if (w != nullptr && w->viewport->follow_vehicle == v->index) { + Window *w = GetMainWindow(); + if (w->viewport->follow_vehicle == v->index) { ScrollMainWindowTo(v->x_pos, v->y_pos, v->z_pos, true); // lock the main view on the vehicle's last position w->viewport->follow_vehicle = INVALID_VEHICLE; } diff --git a/src/viewport_gui.cpp b/src/viewport_gui.cpp index 6f74237d33..df670cc152 100644 --- a/src/viewport_gui.cpp +++ b/src/viewport_gui.cpp @@ -63,7 +63,7 @@ public: Point pt; if (tile == INVALID_TILE) { /* No tile? Use center of main viewport. */ - const Window *w = FindWindowById(WC_MAIN_WINDOW, 0); + const Window *w = GetMainWindow(); /* center on same place as main window (zoom is maximum, no adjustment needed) */ pt.x = w->viewport->scrollpos_x + w->viewport->virtual_width / 2; @@ -95,7 +95,7 @@ public: case WID_EV_ZOOM_OUT: DoZoomInOutWindow(ZOOM_OUT, this); break; case WID_EV_MAIN_TO_VIEW: { // location button (move main view to same spot as this view) 'Paste Location' - Window *w = FindWindowById(WC_MAIN_WINDOW, 0); + Window *w = GetMainWindow(); int x = this->viewport->scrollpos_x; // Where is the main looking at int y = this->viewport->scrollpos_y; @@ -107,7 +107,7 @@ public: } case WID_EV_VIEW_TO_MAIN: { // inverse location button (move this view to same spot as main view) 'Copy Location' - const Window *w = FindWindowById(WC_MAIN_WINDOW, 0); + const Window *w = GetMainWindow(); int x = w->viewport->scrollpos_x; int y = w->viewport->scrollpos_y; diff --git a/src/window.cpp b/src/window.cpp index 81952d27db..736b2c443f 100644 --- a/src/window.cpp +++ b/src/window.cpp @@ -1178,6 +1178,18 @@ Window *FindWindowByClass(WindowClass cls) return nullptr; } +/** + * Get the main window, i.e. FindWindowById(WC_MAIN_WINDOW, 0). + * If the main window is not available, this function will trigger an assert. + * @return Pointer to the main window. + */ +Window *GetMainWindow() +{ + Window *w = FindWindowById(WC_MAIN_WINDOW, 0); + assert(w != nullptr); + return w; +} + /** * Close a window by its class and window number (if it is open). * @param cls Window class @@ -2431,7 +2443,7 @@ static EventState HandleViewportScroll() return ES_NOT_HANDLED; } - if (_last_scroll_window == FindWindowById(WC_MAIN_WINDOW, 0) && _last_scroll_window->viewport->follow_vehicle != INVALID_VEHICLE) { + if (_last_scroll_window == GetMainWindow() && _last_scroll_window->viewport->follow_vehicle != INVALID_VEHICLE) { /* If the main window is following a vehicle, then first let go of it! */ const Vehicle *veh = Vehicle::Get(_last_scroll_window->viewport->follow_vehicle); ScrollMainWindowTo(veh->x_pos, veh->y_pos, veh->z_pos, true); // This also resets follow_vehicle @@ -2776,9 +2788,7 @@ const std::chrono::milliseconds TIME_BETWEEN_DOUBLE_CLICK(500); ///< Time betwee static void ScrollMainViewport(int x, int y) { if (_game_mode != GM_MENU) { - Window *w = FindWindowById(WC_MAIN_WINDOW, 0); - assert(w); - + Window *w = GetMainWindow(); w->viewport->dest_scrollpos_x += ScaleByZoom(x, w->viewport->zoom); w->viewport->dest_scrollpos_y += ScaleByZoom(y, w->viewport->zoom); } diff --git a/src/window_func.h b/src/window_func.h index d68b7ba1c1..0f4d154e54 100644 --- a/src/window_func.h +++ b/src/window_func.h @@ -16,6 +16,7 @@ Window *FindWindowById(WindowClass cls, WindowNumber number); Window *FindWindowByClass(WindowClass cls); +Window *GetMainWindow(); void ChangeWindowOwner(Owner old_owner, Owner new_owner); void ResizeWindow(Window *w, int x, int y, bool clamp_to_screen = true); From 90f1768006f29b979d7f80cff46e36277c5e3394 Mon Sep 17 00:00:00 2001 From: Rubidium Date: Fri, 6 Jan 2023 23:43:50 +0100 Subject: [PATCH 05/23] Codechange: add non-nullptr asserts in cases where it should never be nullptr Though where similar calls are checked for nullptr as in those instances of the use of that function it can actually return nullptr. In other words, write down the assumption that the function never returns nullptr in an assert. --- src/build_vehicle_gui.cpp | 1 + src/console_cmds.cpp | 1 + src/game/game_text.cpp | 1 + src/network/network_server.cpp | 2 ++ src/newgrf_engine.cpp | 1 + src/newgrf_gui.cpp | 3 +++ src/order_cmd.cpp | 1 + src/order_gui.cpp | 3 +++ src/script/api/script_gamesettings.cpp | 2 ++ src/script/api/script_order.cpp | 5 +++++ src/script/api/script_window.cpp | 1 + src/timetable_cmd.cpp | 2 ++ src/vehicle_cmd.cpp | 1 + 13 files changed, 24 insertions(+) diff --git a/src/build_vehicle_gui.cpp b/src/build_vehicle_gui.cpp index eb4860cd94..91b94d652f 100644 --- a/src/build_vehicle_gui.cpp +++ b/src/build_vehicle_gui.cpp @@ -866,6 +866,7 @@ static uint ShowAdditionalText(int left, int right, int y, EngineID engine) uint16 callback = GetVehicleCallback(CBID_VEHICLE_ADDITIONAL_TEXT, 0, 0, engine, nullptr); if (callback == CALLBACK_FAILED || callback == 0x400) return y; const GRFFile *grffile = Engine::Get(engine)->GetGRF(); + assert(grffile != nullptr); if (callback > 0x400) { ErrorUnknownCallbackResult(grffile->grfid, CBID_VEHICLE_ADDITIONAL_TEXT, callback); return y; diff --git a/src/console_cmds.cpp b/src/console_cmds.cpp index e652e64111..65343aba5d 100644 --- a/src/console_cmds.cpp +++ b/src/console_cmds.cpp @@ -925,6 +925,7 @@ DEF_CONSOLE_CMD(ConResetCompany) return false; } const NetworkClientInfo *ci = NetworkClientInfo::GetByClientID(CLIENT_ID_SERVER); + assert(ci != nullptr); if (ci->client_playas == index) { IConsolePrint(CC_ERROR, "Cannot remove company: the server is connected to that company."); return true; diff --git a/src/game/game_text.cpp b/src/game/game_text.cpp index ca102f9120..85d9b03ced 100644 --- a/src/game/game_text.cpp +++ b/src/game/game_text.cpp @@ -225,6 +225,7 @@ public: GameStrings *LoadTranslations() { const GameInfo *info = Game::GetInfo(); + assert(info != nullptr); std::string basename(info->GetMainScript()); auto e = basename.rfind(PATHSEPCHAR); if (e == std::string::npos) return nullptr; diff --git a/src/network/network_server.cpp b/src/network/network_server.cpp index d3cbdf9e3a..e40b973ea5 100644 --- a/src/network/network_server.cpp +++ b/src/network/network_server.cpp @@ -1532,6 +1532,7 @@ static void NetworkAutoCleanCompanies() if (!_network_dedicated) { const NetworkClientInfo *ci = NetworkClientInfo::GetByClientID(CLIENT_ID_SERVER); + assert(ci != nullptr); if (Company::IsValidID(ci->client_playas)) clients_in_company[ci->client_playas] = true; } @@ -1918,6 +1919,7 @@ void NetworkServerDoMove(ClientID client_id, CompanyID company_id) if (client_id == CLIENT_ID_SERVER && _network_dedicated) return; NetworkClientInfo *ci = NetworkClientInfo::GetByClientID(client_id); + assert(ci != nullptr); /* No need to waste network resources if the client is in the company already! */ if (ci->client_playas == company_id) return; diff --git a/src/newgrf_engine.cpp b/src/newgrf_engine.cpp index af72c65ed1..eeccf30e96 100644 --- a/src/newgrf_engine.cpp +++ b/src/newgrf_engine.cpp @@ -518,6 +518,7 @@ static uint32 VehicleGetVariable(Vehicle *v, const VehicleScopeResolver *object, { const Vehicle *w = v->Next(); + assert(w != nullptr); uint16 altitude = ClampToU16(v->z_pos - w->z_pos); // Aircraft height - shadow height byte airporttype = ATP_TTDP_LARGE; diff --git a/src/newgrf_gui.cpp b/src/newgrf_gui.cpp index 223c1e21af..89e1a6e5e8 100644 --- a/src/newgrf_gui.cpp +++ b/src/newgrf_gui.cpp @@ -1464,6 +1464,9 @@ private: this->avails.push_back(c); } else { const GRFConfig *best = FindGRFConfig(c->ident.grfid, HasBit(c->flags, GCF_INVALID) ? FGCM_NEWEST : FGCM_NEWEST_VALID); + /* Never triggers; FindGRFConfig returns either c, or a newer version of c. */ + assert(best != nullptr); + /* * If the best version is 0, then all NewGRF with this GRF ID * have version 0, so for backward compatibility reasons we diff --git a/src/order_cmd.cpp b/src/order_cmd.cpp index ca3207ee73..ab2f9fb647 100644 --- a/src/order_cmd.cpp +++ b/src/order_cmd.cpp @@ -1251,6 +1251,7 @@ CommandCost CmdModifyOrder(DoCommandFlag flags, VehicleID veh, VehicleOrderID se if (sel_ord >= v->GetNumOrders()) return CMD_ERROR; Order *order = v->GetOrder(sel_ord); + assert(order != nullptr); switch (order->GetType()) { case OT_GOTO_STATION: if (mof != MOF_NON_STOP && mof != MOF_STOP_LOCATION && mof != MOF_UNLOAD && mof != MOF_LOAD) return CMD_ERROR; diff --git a/src/order_gui.cpp b/src/order_gui.cpp index 816a1b85b3..134b488960 100644 --- a/src/order_gui.cpp +++ b/src/order_gui.cpp @@ -1223,6 +1223,7 @@ public: this->OrderClick_Nonstop(-1); } else { const Order *o = this->vehicle->GetOrder(this->OrderGetSel()); + assert(o != nullptr); ShowDropDownMenu(this, _order_non_stop_drowdown, o->GetNonStopType(), WID_O_NON_STOP, 0, o->IsType(OT_GOTO_STATION) ? 0 : (o->IsType(OT_GOTO_WAYPOINT) ? 3 : 12)); } @@ -1299,12 +1300,14 @@ public: case WID_O_COND_COMPARATOR: { const Order *o = this->vehicle->GetOrder(this->OrderGetSel()); + assert(o != nullptr); ShowDropDownMenu(this, _order_conditional_condition, o->GetConditionComparator(), WID_O_COND_COMPARATOR, 0, (o->GetConditionVariable() == OCV_REQUIRES_SERVICE) ? 0x3F : 0xC0); break; } case WID_O_COND_VALUE: { const Order *order = this->vehicle->GetOrder(this->OrderGetSel()); + assert(order != nullptr); uint value = order->GetConditionValue(); if (order->GetConditionVariable() == OCV_MAX_SPEED) value = ConvertSpeedToDisplaySpeed(value); SetDParam(0, value); diff --git a/src/script/api/script_gamesettings.cpp b/src/script/api/script_gamesettings.cpp index 2791aba006..425531cce7 100644 --- a/src/script/api/script_gamesettings.cpp +++ b/src/script/api/script_gamesettings.cpp @@ -27,6 +27,7 @@ if (!IsValid(setting)) return -1; const SettingDesc *sd = GetSettingFromName(setting); + assert(sd != nullptr); return sd->AsIntSetting()->Read(&_settings_game); } @@ -35,6 +36,7 @@ if (!IsValid(setting)) return false; const SettingDesc *sd = GetSettingFromName(setting); + assert(sd != nullptr); if ((sd->flags & SF_NO_NETWORK_SYNC) != 0) return false; diff --git a/src/script/api/script_order.cpp b/src/script/api/script_order.cpp index 1886080aa7..44427bb19b 100644 --- a/src/script/api/script_order.cpp +++ b/src/script/api/script_order.cpp @@ -68,6 +68,7 @@ static const Order *ResolveOrder(VehicleID vehicle_id, ScriptOrder::OrderPositio if (order_position == ScriptOrder::ORDER_INVALID) return nullptr; } const Order *order = v->GetFirstOrder(); + assert(order != nullptr); while (order->GetType() == OT_IMPLICIT) order = order->next; while (order_position > 0) { order_position = (ScriptOrder::OrderPosition)(order_position - 1); @@ -92,6 +93,7 @@ static int ScriptOrderPositionToRealOrderPosition(VehicleID vehicle_id, ScriptOr int res = (int)order_position; const Order *order = v->orders->GetFirstOrder(); + assert(order != nullptr); for (; order->GetType() == OT_IMPLICIT; order = order->next) res++; while (order_position > 0) { order_position = (ScriptOrder::OrderPosition)(order_position - 1); @@ -132,6 +134,7 @@ static int ScriptOrderPositionToRealOrderPosition(VehicleID vehicle_id, ScriptOr if (!IsValidVehicleOrder(vehicle_id, order_position)) return false; const Order *order = ::Vehicle::Get(vehicle_id)->GetOrder(ScriptOrderPositionToRealOrderPosition(vehicle_id, order_position)); + assert(order != nullptr); return order->GetType() == OT_CONDITIONAL; } @@ -141,6 +144,7 @@ static int ScriptOrderPositionToRealOrderPosition(VehicleID vehicle_id, ScriptOr if (!IsValidVehicleOrder(vehicle_id, order_position)) return false; const Order *order = ::ResolveOrder(vehicle_id, order_position); + assert(order != nullptr); return order->GetType() == OT_DUMMY; } @@ -172,6 +176,7 @@ static int ScriptOrderPositionToRealOrderPosition(VehicleID vehicle_id, ScriptOr if (order_position == ORDER_CURRENT) { int cur_order_pos = ::Vehicle::Get(vehicle_id)->cur_real_order_index; const Order *order = ::Vehicle::Get(vehicle_id)->GetFirstOrder(); + assert(order != nullptr); int num_implicit_orders = 0; for (int i = 0; i < cur_order_pos; i++) { if (order->GetType() == OT_IMPLICIT) num_implicit_orders++; diff --git a/src/script/api/script_window.cpp b/src/script/api/script_window.cpp index 7969380cb3..a164d1f171 100644 --- a/src/script/api/script_window.cpp +++ b/src/script/api/script_window.cpp @@ -46,6 +46,7 @@ if (colour != TC_INVALID && (::TextColour)colour >= ::TC_END) return; Window *w = FindWindowById((::WindowClass)window, number); + assert(w != nullptr); if (widget == WIDGET_ALL) { if (colour != TC_INVALID) return; diff --git a/src/timetable_cmd.cpp b/src/timetable_cmd.cpp index f800b25ae6..a1f4ea4ebe 100644 --- a/src/timetable_cmd.cpp +++ b/src/timetable_cmd.cpp @@ -30,6 +30,7 @@ static void ChangeTimetable(Vehicle *v, VehicleOrderID order_number, uint16 val, ModifyTimetableFlags mtf, bool timetabled) { Order *order = v->GetOrder(order_number); + assert(order != nullptr); int total_delta = 0; int timetable_delta = 0; @@ -390,6 +391,7 @@ void UpdateVehicleTimetable(Vehicle *v, bool travelling) if (v->cur_real_order_index >= v->GetNumOrders()) return; Order *real_current_order = v->GetOrder(v->cur_real_order_index); + assert(real_current_order != nullptr); VehicleOrderID first_manual_order = 0; for (Order *o = v->GetFirstOrder(); o != nullptr && o->IsType(OT_IMPLICIT); o = o->next) { diff --git a/src/vehicle_cmd.cpp b/src/vehicle_cmd.cpp index fa1c6e54c7..4c432d9eef 100644 --- a/src/vehicle_cmd.cpp +++ b/src/vehicle_cmd.cpp @@ -455,6 +455,7 @@ static std::tuple RefitVehicle(Vehicle *v u->cargo_subtype = result.subtype; if (u->type == VEH_AIRCRAFT) { Vehicle *w = u->Next(); + assert(w != nullptr); w->refit_cap = std::min(w->refit_cap, result.mail_capacity); w->cargo_cap = result.mail_capacity; if (w->cargo.TotalCount() > w->refit_cap) w->cargo.Truncate(w->cargo.TotalCount() - w->refit_cap); From 5370e910d3577a451dd9c09f749179669c3f4ff0 Mon Sep 17 00:00:00 2001 From: Peter Nelson Date: Sat, 14 Jan 2023 18:16:14 +0000 Subject: [PATCH 06/23] Change: Use std::vector for fallback font list. --- src/os/windows/font_win32.cpp | 49 +++++++++-------------------------- 1 file changed, 12 insertions(+), 37 deletions(-) diff --git a/src/os/windows/font_win32.cpp b/src/os/windows/font_win32.cpp index 093d466954..2c02993db1 100644 --- a/src/os/windows/font_win32.cpp +++ b/src/os/windows/font_win32.cpp @@ -233,46 +233,21 @@ err1: } #endif /* WITH_FREETYPE */ -class FontList { -protected: - wchar_t **fonts; - uint items; - uint capacity; - -public: - FontList() : fonts(nullptr), items(0), capacity(0) { }; - - ~FontList() { - if (this->fonts == nullptr) return; - - for (uint i = 0; i < this->items; i++) { - free(this->fonts[i]); - } - - free(this->fonts); - } - - bool Add(const wchar_t *font) { - for (uint i = 0; i < this->items; i++) { - if (wcscmp(this->fonts[i], font) == 0) return false; - } - - if (this->items == this->capacity) { - this->capacity += 10; - this->fonts = ReallocT(this->fonts, this->capacity); - } - - this->fonts[this->items++] = wcsdup(font); - - return true; - } -}; - struct EFCParam { FontCacheSettings *settings; LOCALESIGNATURE locale; MissingGlyphSearcher *callback; - FontList fonts; + std::vector fonts; + + bool Add(const std::wstring_view &font) { + for (const auto &entry : this->fonts) { + if (font.compare(entry) == 0) return false; + } + + this->fonts.emplace_back(font); + + return true; + } }; static int CALLBACK EnumFontCallback(const ENUMLOGFONTEX *logfont, const NEWTEXTMETRICEX *metric, DWORD type, LPARAM lParam) @@ -280,7 +255,7 @@ static int CALLBACK EnumFontCallback(const ENUMLOGFONTEX *logfont, const NEWTEXT EFCParam *info = (EFCParam *)lParam; /* Skip duplicates */ - if (!info->fonts.Add((const wchar_t *)logfont->elfFullName)) return 1; + if (!info->Add(logfont->elfFullName)) return 1; /* Only use TrueType fonts */ if (!(type & TRUETYPE_FONTTYPE)) return 1; /* Don't use SYMBOL fonts */ From fa0c67b10a20d143acb8c50ea6cbd1d2d8046152 Mon Sep 17 00:00:00 2001 From: Peter Nelson Date: Sat, 14 Jan 2023 18:07:05 +0000 Subject: [PATCH 07/23] Change: Remove guess-work from calls to GetGlyphOutline(). This API method is intended to be called twice, so don't attempt to guess the required size. --- src/os/windows/font_win32.cpp | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/src/os/windows/font_win32.cpp b/src/os/windows/font_win32.cpp index 2c02993db1..35c606eb8f 100644 --- a/src/os/windows/font_win32.cpp +++ b/src/os/windows/font_win32.cpp @@ -436,22 +436,13 @@ void Win32FontCache::ClearFontCache() GLYPHMETRICS gm; MAT2 mat = { {0, 1}, {0, 0}, {0, 0}, {0, 1} }; - /* Make a guess for the needed memory size. */ - DWORD size = this->glyph_size.cy * Align(aa ? this->glyph_size.cx : std::max(this->glyph_size.cx / 8l, 1l), 4); // Bitmap data is DWORD-aligned rows. - byte *bmp = AllocaM(byte, size); - size = GetGlyphOutline(this->dc, key, GGO_GLYPH_INDEX | (aa ? GGO_GRAY8_BITMAP : GGO_BITMAP), &gm, size, bmp, &mat); + /* Call GetGlyphOutline with zero size initially to get required memory size. */ + DWORD size = GetGlyphOutline(this->dc, key, GGO_GLYPH_INDEX | (aa ? GGO_GRAY8_BITMAP : GGO_BITMAP), &gm, 0, nullptr, &mat); + if (size == GDI_ERROR) usererror("Unable to render font glyph"); - if (size == GDI_ERROR) { - /* No dice with the guess. First query size of needed glyph memory, then allocate the - * memory and query again. This dance is necessary as some glyphs will only render with - * the exact matching size; e.g. the space glyph has no pixels and must be requested - * with size == 0, anything else fails. Unfortunately, a failed call doesn't return any - * info about the size and thus the triple GetGlyphOutline()-call. */ - size = GetGlyphOutline(this->dc, key, GGO_GLYPH_INDEX | (aa ? GGO_GRAY8_BITMAP : GGO_BITMAP), &gm, 0, nullptr, &mat); - if (size == GDI_ERROR) usererror("Unable to render font glyph"); - bmp = AllocaM(byte, size); - GetGlyphOutline(this->dc, key, GGO_GLYPH_INDEX | (aa ? GGO_GRAY8_BITMAP : GGO_BITMAP), &gm, size, bmp, &mat); - } + /* Call GetGlyphOutline again with size to actually render the glyph. */ + byte *bmp = AllocaM(byte, size); + GetGlyphOutline(this->dc, key, GGO_GLYPH_INDEX | (aa ? GGO_GRAY8_BITMAP : GGO_BITMAP), &gm, size, bmp, &mat); /* Add 1 scaled pixel for the shadow on the medium font. Our sprite must be at least 1x1 pixel. */ uint shadow = (this->fs == FS_NORMAL) ? ScaleGUITrad(1) : 0; From 8149ba338f1855999a7559b159c037d84c8c1a3e Mon Sep 17 00:00:00 2001 From: Peter Nelson Date: Sat, 14 Jan 2023 18:09:00 +0000 Subject: [PATCH 08/23] Change: Check glyph size before trying to render it. This change of order ensures that the "Font glyph is foot large" occurs even if the glyph is too large for an alloca() allocation. --- src/os/windows/font_win32.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/os/windows/font_win32.cpp b/src/os/windows/font_win32.cpp index 35c606eb8f..aa81a65466 100644 --- a/src/os/windows/font_win32.cpp +++ b/src/os/windows/font_win32.cpp @@ -440,10 +440,6 @@ void Win32FontCache::ClearFontCache() DWORD size = GetGlyphOutline(this->dc, key, GGO_GLYPH_INDEX | (aa ? GGO_GRAY8_BITMAP : GGO_BITMAP), &gm, 0, nullptr, &mat); if (size == GDI_ERROR) usererror("Unable to render font glyph"); - /* Call GetGlyphOutline again with size to actually render the glyph. */ - byte *bmp = AllocaM(byte, size); - GetGlyphOutline(this->dc, key, GGO_GLYPH_INDEX | (aa ? GGO_GRAY8_BITMAP : GGO_BITMAP), &gm, size, bmp, &mat); - /* Add 1 scaled pixel for the shadow on the medium font. Our sprite must be at least 1x1 pixel. */ uint shadow = (this->fs == FS_NORMAL) ? ScaleGUITrad(1) : 0; uint width = std::max(1U, (uint)gm.gmBlackBoxX + shadow); @@ -452,6 +448,10 @@ void Win32FontCache::ClearFontCache() /* Limit glyph size to prevent overflows later on. */ if (width > MAX_GLYPH_DIM || height > MAX_GLYPH_DIM) usererror("Font glyph is too large"); + /* Call GetGlyphOutline again with size to actually render the glyph. */ + byte *bmp = AllocaM(byte, size); + GetGlyphOutline(this->dc, key, GGO_GLYPH_INDEX | (aa ? GGO_GRAY8_BITMAP : GGO_BITMAP), &gm, size, bmp, &mat); + /* GDI has rendered the glyph, now we allocate a sprite and copy the image into it. */ SpriteLoader::Sprite sprite; sprite.AllocateData(ZOOM_LVL_NORMAL, width * height); From 5e6dac6fd4feb13830313e31e4a2d350e570a5e3 Mon Sep 17 00:00:00 2001 From: rubidium42 Date: Sat, 31 Dec 2022 18:21:28 +0100 Subject: [PATCH 09/23] Add: enable CodeQL code scanning As a replacement to the now deprecated LGTM(.com) --- .github/codeql/codeql-config.yml | 10 ++++ .github/workflows/codeql.yml | 78 ++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+) create mode 100644 .github/codeql/codeql-config.yml create mode 100644 .github/workflows/codeql.yml diff --git a/.github/codeql/codeql-config.yml b/.github/codeql/codeql-config.yml new file mode 100644 index 0000000000..e5bd27659c --- /dev/null +++ b/.github/codeql/codeql-config.yml @@ -0,0 +1,10 @@ +name: openttd +queries: +- uses: security-and-quality +query-filters: +- exclude: + id: + # Only feasible way is to move away from fopen; fopen_s is optional C11 and not implemented on most platforms. + - cpp/world-writable-file-creation + # Basically OpenTTD's coding style for adding things like ..._INVALID to enumerations + - cpp/irregular-enum-init diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml new file mode 100644 index 0000000000..d30e9b3755 --- /dev/null +++ b/.github/workflows/codeql.yml @@ -0,0 +1,78 @@ +name: CodeQL + +on: + push: + branches: + - master + pull_request: + # The branches below must be a subset of the branches above + branches: + - master + +jobs: + analyze: + name: Analyze + runs-on: ubuntu-latest + permissions: + actions: read + contents: read + security-events: write + + steps: + - name: Checkout + uses: actions/checkout@v3 + + - name: Install dependencies + run: | + echo "::group::Update apt" + sudo apt-get update + echo "::endgroup::" + + echo "::group::Install dependencies" + sudo apt-get install -y --no-install-recommends \ + liballegro4-dev \ + libfontconfig-dev \ + libicu-dev \ + liblzma-dev \ + liblzo2-dev \ + libsdl2-dev \ + zlib1g-dev \ + # EOF + echo "::endgroup::" + env: + DEBIAN_FRONTEND: noninteractive + + - name: Set number of make jobs + run: | + echo "MAKEFLAGS=-j$(nproc)" >> $GITHUB_ENV + + - name: Initialize CodeQL + uses: github/codeql-action/init@v2 + with: + languages: cpp + config-file: ./.github/codeql/codeql-config.yml + + - name: Autobuild + uses: github/codeql-action/autobuild@v2 + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v2 + with: + category: /language:cpp + upload: False + output: sarif-results + + - name: Filter out table & generated code + uses: advanced-security/filter-sarif@v1 + with: + patterns: | + +**/*.* + -**/table/*.* + -**/generated/**/*.* + input: sarif-results/cpp.sarif + output: sarif-results/cpp.sarif + + - name: Upload results + uses: github/codeql-action/upload-sarif@v2 + with: + sarif_file: sarif-results/cpp.sarif From b3b8c3fd2d19694b91f2ebdd835445cd8022095b Mon Sep 17 00:00:00 2001 From: Rubidium Date: Fri, 13 Jan 2023 22:30:18 +0100 Subject: [PATCH 10/23] Codechange: pass the randomizer to use directly to the company face generation --- src/company_cmd.cpp | 2 +- src/company_gui.cpp | 2 +- src/company_manager_face.h | 8 +++----- src/script/api/script_company.cpp | 2 +- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/company_cmd.cpp b/src/company_cmd.cpp index a4e5fc9c3b..58da230836 100644 --- a/src/company_cmd.cpp +++ b/src/company_cmd.cpp @@ -573,7 +573,7 @@ Company *DoStartupNewCompany(bool is_ai, CompanyID company = INVALID_COMPANY) if (_company_manager_face != 0 && !is_ai && !_networking) { c->face = _company_manager_face; } else { - RandomCompanyManagerFaceBits(c->face, (GenderEthnicity)Random(), false, false); + RandomCompanyManagerFaceBits(c->face, (GenderEthnicity)Random(), false, _random); } SetDefaultCompanySettings(c->index); diff --git a/src/company_gui.cpp b/src/company_gui.cpp index 3d8fe8896b..3649e79c21 100644 --- a/src/company_gui.cpp +++ b/src/company_gui.cpp @@ -1694,7 +1694,7 @@ public: /* Randomize face button */ case WID_SCMF_RANDOM_NEW_FACE: - RandomCompanyManagerFaceBits(this->face, this->ge, this->advanced); + RandomCompanyManagerFaceBits(this->face, this->ge, this->advanced, _interactive_random); this->UpdateData(); this->SetDirty(); break; diff --git a/src/company_manager_face.h b/src/company_manager_face.h index fe6365a121..83bfd29e11 100644 --- a/src/company_manager_face.h +++ b/src/company_manager_face.h @@ -199,15 +199,13 @@ static inline void ScaleAllCompanyManagerFaceBits(CompanyManagerFace &cmf) * @param cmf the company manager's face to write the bits to * @param ge the gender and ethnicity of the old company manager's face * @param adv if it for the advanced company manager's face window - * @param interactive is the call from within the user interface? + * @param randomizer the source of random to use for creating the manager face * * @pre scale 'ge' to a valid gender/ethnicity combination */ -static inline void RandomCompanyManagerFaceBits(CompanyManagerFace &cmf, GenderEthnicity ge, bool adv, bool interactive = true) +static inline void RandomCompanyManagerFaceBits(CompanyManagerFace &cmf, GenderEthnicity ge, bool adv, Randomizer &randomizer) { - /* This method is called from a command when not interactive and - * then we must use Random to get the same result on all clients. */ - cmf = interactive ? InteractiveRandom() : Random(); // random all company manager's face bits + cmf = randomizer.Next(); // random all company manager's face bits /* scale ge: 0 == GE_WM, 1 == GE_WF, 2 == GE_BM, 3 == GE_BF (and maybe in future: ...) */ ge = (GenderEthnicity)((uint)ge % GE_END); diff --git a/src/script/api/script_company.cpp b/src/script/api/script_company.cpp index 6a06852da5..c38f85385a 100644 --- a/src/script/api/script_company.cpp +++ b/src/script/api/script_company.cpp @@ -99,7 +99,7 @@ CompanyManagerFace cmf; GenderEthnicity ge = (GenderEthnicity)((gender == GENDER_FEMALE ? (1 << ::GENDER_FEMALE) : 0) | (::InteractiveRandom() & (1 << ETHNICITY_BLACK))); - RandomCompanyManagerFaceBits(cmf, ge, false); + RandomCompanyManagerFaceBits(cmf, ge, false, _interactive_random); return ScriptObject::Command::Do(cmf); } From 33731282330e4e84b34e1cf5acd697d661d897c3 Mon Sep 17 00:00:00 2001 From: Rubidium Date: Fri, 13 Jan 2023 22:31:09 +0100 Subject: [PATCH 11/23] Codechange: pass the randomizer directly to the town name generation --- src/script/api/script_town.cpp | 2 +- src/town_cmd.cpp | 4 ++-- src/town_gui.cpp | 2 +- src/townname.cpp | 5 +++-- src/townname_func.h | 3 ++- src/tree_cmd.cpp | 1 + 6 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/script/api/script_town.cpp b/src/script/api/script_town.cpp index afa309c92b..e3894b24ec 100644 --- a/src/script/api/script_town.cpp +++ b/src/script/api/script_town.cpp @@ -301,7 +301,7 @@ EnforcePreconditionCustomError(false, ::Utf8StringLength(text) < MAX_LENGTH_TOWN_NAME_CHARS, ScriptError::ERR_PRECONDITION_STRING_TOO_LONG); } uint32 townnameparts; - if (!GenerateTownName(&townnameparts)) { + if (!GenerateTownName(_interactive_random, &townnameparts)) { ScriptObject::SetLastError(ScriptError::ERR_NAME_IS_NOT_UNIQUE); return false; } diff --git a/src/town_cmd.cpp b/src/town_cmd.cpp index 280fa4efea..c21e1ebaad 100644 --- a/src/town_cmd.cpp +++ b/src/town_cmd.cpp @@ -2245,7 +2245,7 @@ bool GenerateTowns(TownLayout layout) bool city = (_settings_game.economy.larger_towns != 0 && Chance16(1, _settings_game.economy.larger_towns)); IncreaseGeneratingWorldProgress(GWP_TOWN); /* Get a unique name for the town. */ - if (!GenerateTownName(&townnameparts, &town_names)) continue; + if (!GenerateTownName(_random, &townnameparts, &town_names)) continue; /* try 20 times to create a random-sized town for the first loop. */ if (CreateRandomTown(20, townnameparts, TSZ_RANDOM, city, layout) != nullptr) current_number++; // If creation was successful, raise a flag. } while (--total); @@ -2259,7 +2259,7 @@ bool GenerateTowns(TownLayout layout) /* If current_number is still zero at this point, it means that not a single town has been created. * So give it a last try, but now more aggressive */ - if (GenerateTownName(&townnameparts) && + if (GenerateTownName(_random, &townnameparts) && CreateRandomTown(10000, townnameparts, TSZ_RANDOM, _settings_game.economy.larger_towns != 0, layout) != nullptr) { return true; } diff --git a/src/town_gui.cpp b/src/town_gui.cpp index e44531a5cb..f13fb19ecb 100644 --- a/src/town_gui.cpp +++ b/src/town_gui.cpp @@ -1146,7 +1146,7 @@ public: void RandomTownName() { - this->townnamevalid = GenerateTownName(&this->townnameparts); + this->townnamevalid = GenerateTownName(_interactive_random, &this->townnameparts); if (!this->townnamevalid) { this->townname_editbox.text.DeleteAll(); diff --git a/src/townname.cpp b/src/townname.cpp index bce894bfcf..86954b933c 100644 --- a/src/townname.cpp +++ b/src/townname.cpp @@ -112,11 +112,12 @@ bool VerifyTownName(uint32 r, const TownNameParams *par, TownNames *town_names) /** * Generates valid town name. + * @param randomizer the source of random data for generating the name * @param townnameparts if a name is generated, it's stored there * @param town_names if a name is generated, check its uniqueness with the set * @return true iff a name was generated */ -bool GenerateTownName(uint32 *townnameparts, TownNames *town_names) +bool GenerateTownName(Randomizer &randomizer, uint32 *townnameparts, TownNames *town_names) { TownNameParams par(_settings_game.game_creation.town_name); @@ -130,7 +131,7 @@ bool GenerateTownName(uint32 *townnameparts, TownNames *town_names) * the other towns may take considerable amount of time (10000 is * too much). */ for (int i = 1000; i != 0; i--) { - uint32 r = _generating_world ? Random() : InteractiveRandom(); + uint32 r = randomizer.Next(); if (!VerifyTownName(r, &par, town_names)) continue; *townnameparts = r; diff --git a/src/townname_func.h b/src/townname_func.h index 6438d2b283..a3d29e467d 100644 --- a/src/townname_func.h +++ b/src/townname_func.h @@ -10,12 +10,13 @@ #ifndef TOWNNAME_FUNC_H #define TOWNNAME_FUNC_H +#include "core/random_func.hpp" #include "townname_type.h" char *GenerateTownNameString(char *buf, const char *last, size_t lang, uint32 seed); char *GetTownName(char *buff, const TownNameParams *par, uint32 townnameparts, const char *last); char *GetTownName(char *buff, const Town *t, const char *last); bool VerifyTownName(uint32 r, const TownNameParams *par, TownNames *town_names = nullptr); -bool GenerateTownName(uint32 *townnameparts, TownNames *town_names = nullptr); +bool GenerateTownName(Randomizer &randomizer, uint32 *townnameparts, TownNames *town_names = nullptr); #endif /* TOWNNAME_FUNC_H */ diff --git a/src/tree_cmd.cpp b/src/tree_cmd.cpp index 505d5bb7af..19c077168d 100644 --- a/src/tree_cmd.cpp +++ b/src/tree_cmd.cpp @@ -308,6 +308,7 @@ void PlaceTreesRandomly() */ uint PlaceTreeGroupAroundTile(TileIndex tile, TreeType treetype, uint radius, uint count, bool set_zone) { + assert(_game_mode == GM_EDITOR); // Due to InteractiveRandom being used in this function assert(treetype < TREE_TOYLAND + TREE_COUNT_TOYLAND); const bool allow_desert = treetype == TREE_CACTUS; uint planted = 0; From 6abad681bd2e8e6303db876eb5da3a8f92bc2ba3 Mon Sep 17 00:00:00 2001 From: Rubidium Date: Fri, 13 Jan 2023 23:48:59 +0100 Subject: [PATCH 12/23] Codechange: move choice for randomizer of scripts to a single location --- src/script/api/script_base.cpp | 12 ++---------- src/script/api/script_base.hpp | 3 --- src/script/api/script_company.cpp | 5 +++-- src/script/api/script_industrytype.cpp | 7 ++++--- src/script/api/script_object.cpp | 7 +++++++ src/script/api/script_object.hpp | 6 ++++++ src/script/api/script_town.cpp | 2 +- 7 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/script/api/script_base.cpp b/src/script/api/script_base.cpp index 996d4d488f..52946520a2 100644 --- a/src/script/api/script_base.cpp +++ b/src/script/api/script_base.cpp @@ -10,17 +10,12 @@ #include "../../stdafx.h" #include "script_base.hpp" #include "script_error.hpp" -#include "../../network/network.h" -#include "../../core/random_func.hpp" #include "../../safeguards.h" /* static */ uint32 ScriptBase::Rand() { - /* We pick RandomRange if we are in SP (so when saved, we do the same over and over) - * but we pick InteractiveRandomRange if we are a network_server or network-client. */ - if (_networking) return ::InteractiveRandom(); - return ::Random(); + return ScriptObject::GetRandomizer().Next(); } /* static */ uint32 ScriptBase::RandItem(int unused_param) @@ -30,10 +25,7 @@ /* static */ uint ScriptBase::RandRange(uint max) { - /* We pick RandomRange if we are in SP (so when saved, we do the same over and over) - * but we pick InteractiveRandomRange if we are a network_server or network-client. */ - if (_networking) return ::InteractiveRandomRange(max); - return ::RandomRange(max); + return ScriptObject::GetRandomizer().Next(max); } /* static */ uint32 ScriptBase::RandRangeItem(int unused_param, uint max) diff --git a/src/script/api/script_base.hpp b/src/script/api/script_base.hpp index b8eebdf1b0..4184e79d69 100644 --- a/src/script/api/script_base.hpp +++ b/src/script/api/script_base.hpp @@ -18,9 +18,6 @@ * * @note The random functions are not called Random and RandomRange, because * RANDOM_DEBUG does some tricky stuff, which messes with those names. - * @note In MP we cannot use Random because that will cause desyncs (scripts are - * ran on the server only, not on all clients). This means that - * we use InteractiveRandom in MP. Rand() takes care of this for you. */ class ScriptBase : public ScriptObject { public: diff --git a/src/script/api/script_company.cpp b/src/script/api/script_company.cpp index c38f85385a..33c0c0ea48 100644 --- a/src/script/api/script_company.cpp +++ b/src/script/api/script_company.cpp @@ -97,9 +97,10 @@ EnforcePrecondition(false, gender == GENDER_MALE || gender == GENDER_FEMALE); EnforcePrecondition(false, GetPresidentGender(ScriptCompany::COMPANY_SELF) != gender); + Randomizer &randomizer = ScriptObject::GetRandomizer(); CompanyManagerFace cmf; - GenderEthnicity ge = (GenderEthnicity)((gender == GENDER_FEMALE ? (1 << ::GENDER_FEMALE) : 0) | (::InteractiveRandom() & (1 << ETHNICITY_BLACK))); - RandomCompanyManagerFaceBits(cmf, ge, false, _interactive_random); + GenderEthnicity ge = (GenderEthnicity)((gender == GENDER_FEMALE ? (1 << ::GENDER_FEMALE) : 0) | (randomizer.Next() & (1 << ETHNICITY_BLACK))); + RandomCompanyManagerFaceBits(cmf, ge, false, randomizer); return ScriptObject::Command::Do(cmf); } diff --git a/src/script/api/script_industrytype.cpp b/src/script/api/script_industrytype.cpp index 14b9a3f67a..3101ac7fdc 100644 --- a/src/script/api/script_industrytype.cpp +++ b/src/script/api/script_industrytype.cpp @@ -9,6 +9,7 @@ #include "../../stdafx.h" #include "script_industrytype.hpp" +#include "script_base.hpp" #include "script_map.hpp" #include "script_error.hpp" #include "../../strings_func.h" @@ -121,8 +122,8 @@ EnforcePrecondition(false, CanBuildIndustry(industry_type)); EnforcePrecondition(false, ScriptMap::IsValidTile(tile)); - uint32 seed = ::InteractiveRandom(); - uint32 layout_index = ::InteractiveRandomRange((uint32)::GetIndustrySpec(industry_type)->layouts.size()); + uint32 seed = ScriptBase::Rand(); + uint32 layout_index = ScriptBase::RandRange((uint32)::GetIndustrySpec(industry_type)->layouts.size()); return ScriptObject::Command::Do(tile, industry_type, layout_index, true, seed); } @@ -130,7 +131,7 @@ { EnforcePrecondition(false, CanProspectIndustry(industry_type)); - uint32 seed = ::InteractiveRandom(); + uint32 seed = ScriptBase::Rand(); return ScriptObject::Command::Do(0, industry_type, 0, false, seed); } diff --git a/src/script/api/script_object.cpp b/src/script/api/script_object.cpp index 7d1042156c..2c798df9ab 100644 --- a/src/script/api/script_object.cpp +++ b/src/script/api/script_object.cpp @@ -311,3 +311,10 @@ bool ScriptObject::DoCommandProcessResult(const CommandCost &res, Script_Suspend NOT_REACHED(); } + +Randomizer &ScriptObject::GetRandomizer() +{ + /* We pick _random if we are in SP (so when saved, we do the same over and over) + * but we pick _interactive_random if we are a network_server or network-client. */ + return _networking ? _interactive_random : _random; +} diff --git a/src/script/api/script_object.hpp b/src/script/api/script_object.hpp index 96c78acb7f..8b104a2e13 100644 --- a/src/script/api/script_object.hpp +++ b/src/script/api/script_object.hpp @@ -15,6 +15,7 @@ #include "../../rail_type.h" #include "../../string_func.h" #include "../../command_func.h" +#include "../../core/random_func.hpp" #include "script_types.hpp" #include "../script_suspend.hpp" @@ -73,6 +74,11 @@ public: */ static class ScriptInstance *GetActiveInstance(); + /** + * Get a reference of the randomizer that brings this script random values. + */ + static Randomizer &GetRandomizer(); + protected: template struct ScriptDoCommandHelper; diff --git a/src/script/api/script_town.cpp b/src/script/api/script_town.cpp index e3894b24ec..7138e22e79 100644 --- a/src/script/api/script_town.cpp +++ b/src/script/api/script_town.cpp @@ -301,7 +301,7 @@ EnforcePreconditionCustomError(false, ::Utf8StringLength(text) < MAX_LENGTH_TOWN_NAME_CHARS, ScriptError::ERR_PRECONDITION_STRING_TOO_LONG); } uint32 townnameparts; - if (!GenerateTownName(_interactive_random, &townnameparts)) { + if (!GenerateTownName(ScriptObject::GetRandomizer(), &townnameparts)) { ScriptObject::SetLastError(ScriptError::ERR_NAME_IS_NOT_UNIQUE); return false; } From c5ff61c5f2c30748ea03825685f0d9d68c0952de Mon Sep 17 00:00:00 2001 From: Rubidium Date: Sat, 14 Jan 2023 13:26:17 +0100 Subject: [PATCH 13/23] Add: script specific Randomizer instances --- regression/regression/result.txt | 284 +++++++++++++++---------------- src/genworld.cpp | 1 + src/saveload/afterload.cpp | 2 + src/script/api/script_object.cpp | 17 +- src/script/api/script_object.hpp | 10 +- 5 files changed, 167 insertions(+), 147 deletions(-) diff --git a/regression/regression/result.txt b/regression/regression/result.txt index dc3e294844..7ce9550599 100644 --- a/regression/regression/result.txt +++ b/regression/regression/result.txt @@ -88,24 +88,24 @@ abs( 21): 21 --AIBase-- - Rand(): -54346916 - Rand(): -937374575 - Rand(): 823953997 + Rand(): -2062310602 + Rand(): -1780331126 + Rand(): -397928569 RandRange(0): 0 RandRange(0): 0 RandRange(0): 0 RandRange(1): 0 RandRange(1): 0 RandRange(1): 0 - RandRange(2): 1 - RandRange(2): 1 - RandRange(2): 1 - RandRange(1000000): 966676 - RandRange(1000000): 289525 - RandRange(1000000): 170283 - Chance(1, 2): false + RandRange(2): 0 + RandRange(2): 0 + RandRange(2): 0 + RandRange(1000000): 666804 + RandRange(1000000): 624059 + RandRange(1000000): 697029 Chance(1, 2): true Chance(1, 2): false + Chance(1, 2): true --List-- IsEmpty(): true @@ -420,144 +420,144 @@ 1098 => 46116 1099 => 46158 Randomize ListDump: - 1 => -200078348 - 2 => -29799264 - 1000 => 1630721656 - 1001 => 959306175 - 1002 => 1527421791 - 1003 => 1259692483 - 1004 => -1289244298 - 1005 => -1572996668 - 1006 => -2069479746 - 1007 => -1819131606 - 1008 => -1007163964 - 1009 => -1185394870 - 1010 => -1471365065 - 1011 => 364354366 - 1012 => -1478084253 - 1013 => 405281367 - 1014 => -11170062 - 1015 => 156767750 - 1016 => 1288924796 - 1017 => 1796884876 - 1018 => -1947073702 - 1019 => -1999614238 - 1020 => -231292809 - 1021 => 966621566 - 1022 => -606766557 - 1023 => -1138727825 - 1024 => -749544262 - 1025 => 2004771271 - 1026 => 686734186 - 1027 => 923274744 - 1028 => -1672035149 - 1029 => -1642064950 - 1030 => 1363389551 - 1031 => -559500928 - 1032 => 1656196991 - 1033 => 1655354425 - 1034 => -1027156689 - 1035 => 1952644328 - 1036 => 1217870217 - 1037 => 242274100 - 1038 => 201816080 - 1039 => 2127464758 - 1040 => 446043650 - 1041 => -319728455 - 1042 => 204701002 - 1043 => -571265398 - 1044 => -1422217131 - 1045 => -391208397 - 1046 => -1822628371 - 1047 => -1499755350 - 1048 => -1422137641 - 1049 => 1621693134 - 1051 => -1428728134 - 1052 => -147587573 - 1053 => 681719500 - 1054 => 1172011190 - 1055 => -1834344882 - 1056 => 1157634586 - 1057 => 1902133676 - 1058 => -1967780161 - 1059 => -1618025531 - 1060 => -810220453 - 1061 => 1582854921 - 1062 => -410004643 - 1063 => 1159917159 - 1064 => -1377804984 - 1065 => -738843914 - 1066 => -1578756103 - 1067 => -464090986 - 1068 => 1711504679 - 1069 => 545330655 - 1070 => 379462570 - 1071 => 514511099 - 1072 => -1813251176 - 1073 => 1424958266 - 1074 => -825255131 - 1075 => 539054595 - 1076 => -1764192010 - 1077 => -1243277769 - 1078 => 2017874281 - 1079 => -1972353607 - 1080 => 1879761467 - 1081 => 1638986560 - 1082 => -1832287507 - 1083 => -492411882 - 1084 => 658940812 - 1085 => -1044199400 - 1086 => 1586504918 - 1087 => -125492611 - 1088 => -1562883174 - 1089 => -1013778441 - 1090 => 1560228607 - 1091 => -550265689 - 1092 => 524767105 - 1093 => -713387661 - 1094 => 1425927738 - 1095 => 942653932 - 1096 => 1233220698 - 1097 => 1313602368 - 1098 => -140318584 - 1099 => 1199179892 + 1 => 688298322 + 2 => -1709546982 + 1000 => 1701392078 + 1001 => -1630848421 + 1002 => -886500935 + 1003 => -196324972 + 1004 => -436037402 + 1005 => -520341784 + 1006 => -1485224804 + 1007 => -311036236 + 1008 => -1503442439 + 1009 => -110945695 + 1010 => -82825175 + 1011 => 46859773 + 1012 => -1199223018 + 1013 => -1190555925 + 1014 => 326384434 + 1015 => 1486817960 + 1016 => -1411425597 + 1017 => -508426854 + 1018 => 820019294 + 1019 => 710762995 + 1020 => -760867032 + 1021 => -709611146 + 1022 => 732190215 + 1023 => 236336673 + 1024 => 740596257 + 1025 => 1135321785 + 1026 => 2067474156 + 1027 => -1395683607 + 1028 => -240528699 + 1029 => 928616892 + 1030 => 1712486685 + 1031 => 1994118287 + 1032 => 1333321243 + 1033 => 194124284 + 1034 => 615083294 + 1035 => 628086450 + 1036 => 498957825 + 1037 => 1359697121 + 1038 => 1888433963 + 1039 => 941623020 + 1040 => -1925663292 + 1041 => -771540264 + 1042 => -1058341359 + 1043 => 182127597 + 1044 => 646955927 + 1045 => -1424621714 + 1046 => 623062612 + 1047 => -1986955586 + 1048 => -1268826980 + 1049 => -456776220 + 1051 => -1112555329 + 1052 => -1532134052 + 1053 => 1960404034 + 1054 => 1573325453 + 1055 => -316619303 + 1056 => 699712177 + 1057 => 863274966 + 1058 => 1728276475 + 1059 => -246695889 + 1060 => 1919485436 + 1061 => 111273464 + 1062 => 125435213 + 1063 => 155132602 + 1064 => -171674076 + 1065 => 655046914 + 1066 => 1577399562 + 1067 => 1028818150 + 1068 => 447058239 + 1069 => -1057920269 + 1070 => -1326215323 + 1071 => -198688588 + 1072 => 1523643051 + 1073 => 231373233 + 1074 => 1121759962 + 1075 => 1449439846 + 1076 => -1615270753 + 1077 => -1509293864 + 1078 => 2116903943 + 1079 => 672822173 + 1080 => -969573911 + 1081 => 1589904755 + 1082 => 1148782015 + 1083 => 663503316 + 1084 => 933352745 + 1085 => 577717039 + 1086 => 402172048 + 1087 => 1812250453 + 1088 => 667300501 + 1089 => -1838825777 + 1090 => -856474776 + 1091 => 420696035 + 1092 => 2131427774 + 1093 => -435303548 + 1094 => -160883878 + 1095 => 1969629634 + 1096 => -555794155 + 1097 => -835119691 + 1098 => -1460907909 + 1099 => -1146924084 KeepTop(10): - 1 => -200078348 - 2 => -29799264 - 1000 => 1630721656 - 1001 => 959306175 - 1002 => 1527421791 - 1003 => 1259692483 - 1004 => -1289244298 - 1005 => -1572996668 - 1006 => -2069479746 - 1007 => -1819131606 + 1 => 688298322 + 2 => -1709546982 + 1000 => 1701392078 + 1001 => -1630848421 + 1002 => -886500935 + 1003 => -196324972 + 1004 => -436037402 + 1005 => -520341784 + 1006 => -1485224804 + 1007 => -311036236 KeepBottom(8): - 1000 => 1630721656 - 1001 => 959306175 - 1002 => 1527421791 - 1003 => 1259692483 - 1004 => -1289244298 - 1005 => -1572996668 - 1006 => -2069479746 - 1007 => -1819131606 + 1000 => 1701392078 + 1001 => -1630848421 + 1002 => -886500935 + 1003 => -196324972 + 1004 => -436037402 + 1005 => -520341784 + 1006 => -1485224804 + 1007 => -311036236 RemoveBottom(2): - 1000 => 1630721656 - 1001 => 959306175 - 1002 => 1527421791 - 1003 => 1259692483 - 1004 => -1289244298 - 1005 => -1572996668 + 1000 => 1701392078 + 1001 => -1630848421 + 1002 => -886500935 + 1003 => -196324972 + 1004 => -436037402 + 1005 => -520341784 RemoveTop(2): - 1002 => 1527421791 - 1003 => 1259692483 - 1004 => -1289244298 - 1005 => -1572996668 + 1002 => -886500935 + 1003 => -196324972 + 1004 => -436037402 + 1005 => -520341784 RemoveList({1003, 1004}): - 1002 => 1527421791 - 1005 => -1572996668 + 1002 => -886500935 + 1005 => -520341784 KeepList({1003, 1004, 1005}): - 1005 => -1572996668 + 1005 => -520341784 AddList({1005, 4000, 4001, 4002}): 1005 => 1005 4000 => 8000 diff --git a/src/genworld.cpp b/src/genworld.cpp index 711a9868ba..ecdae28860 100644 --- a/src/genworld.cpp +++ b/src/genworld.cpp @@ -96,6 +96,7 @@ static void _GenerateWorld() _random.SetSeed(_settings_game.game_creation.generation_seed); SetGeneratingWorldProgress(GWP_MAP_INIT, 2); SetObjectToPlace(SPR_CURSOR_ZZZ, PAL_NONE, HT_NONE, WC_MAIN_WINDOW, 0); + ScriptObject::InitializeRandomizers(); BasePersistentStorageArray::SwitchMode(PSM_ENTER_GAMELOOP); diff --git a/src/saveload/afterload.cpp b/src/saveload/afterload.cpp index 9deed82e16..b8f80d1685 100644 --- a/src/saveload/afterload.cpp +++ b/src/saveload/afterload.cpp @@ -251,6 +251,8 @@ static void InitializeWindowsAndCaches() UpdateAllVirtCoords(); ResetViewportAfterLoadGame(); + ScriptObject::InitializeRandomizers(); + for (Company *c : Company::Iterate()) { /* For each company, verify (while loading a scenario) that the inauguration date is the current year and set it * accordingly if it is not the case. No need to set it on companies that are not been used already, diff --git a/src/script/api/script_object.cpp b/src/script/api/script_object.cpp index 2c798df9ab..1d02088376 100644 --- a/src/script/api/script_object.cpp +++ b/src/script/api/script_object.cpp @@ -312,9 +312,18 @@ bool ScriptObject::DoCommandProcessResult(const CommandCost &res, Script_Suspend NOT_REACHED(); } -Randomizer &ScriptObject::GetRandomizer() + +/* static */ Randomizer ScriptObject::random_states[OWNER_END]; + +Randomizer &ScriptObject::GetRandomizer(Owner owner) { - /* We pick _random if we are in SP (so when saved, we do the same over and over) - * but we pick _interactive_random if we are a network_server or network-client. */ - return _networking ? _interactive_random : _random; + return ScriptObject::random_states[owner]; +} + +void ScriptObject::InitializeRandomizers() +{ + Randomizer random = _random; + for (Owner owner = OWNER_BEGIN; owner < OWNER_END; owner++) { + ScriptObject::GetRandomizer(owner).SetSeed(random.Next()); + } } diff --git a/src/script/api/script_object.hpp b/src/script/api/script_object.hpp index 8b104a2e13..7a11e18cfe 100644 --- a/src/script/api/script_object.hpp +++ b/src/script/api/script_object.hpp @@ -76,8 +76,15 @@ public: /** * Get a reference of the randomizer that brings this script random values. + * @param owner The owner/script to get the randomizer for. This defaults to ScriptObject::GetRootCompany() */ - static Randomizer &GetRandomizer(); + static Randomizer &GetRandomizer(Owner owner = ScriptObject::GetRootCompany()); + + /** + * Initialize/reset the script random states. The state of the scripts are + * based on the current _random seed, but _random does not get changed. + */ + static void InitializeRandomizers(); protected: template struct ScriptDoCommandHelper; @@ -279,6 +286,7 @@ private: static std::tuple DoCommandPrep(); static bool DoCommandProcessResult(const CommandCost &res, Script_SuspendCallbackProc *callback, bool estimate_only); static CommandCallbackData *GetDoCommandCallback(); + static Randomizer random_states[OWNER_END]; ///< Random states for each of the scripts (game script uses OWNER_DEITY) }; namespace ScriptObjectInternal { From 921c6591f95d3c46432e7c3c7cc48ac8a629d9c3 Mon Sep 17 00:00:00 2001 From: Rubidium Date: Sat, 14 Jan 2023 13:38:05 +0100 Subject: [PATCH 14/23] Codechange: do not use interactive random anymore for script configuration --- src/ai/ai_scanner.cpp | 8 ++------ src/script/script_config.cpp | 5 +++-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/ai/ai_scanner.cpp b/src/ai/ai_scanner.cpp index ac77a0b8fd..77a2c814a5 100644 --- a/src/ai/ai_scanner.cpp +++ b/src/ai/ai_scanner.cpp @@ -14,6 +14,7 @@ #include "../core/random_func.hpp" #include "../script/squirrel_class.hpp" +#include "../script/api/script_object.hpp" #include "ai_info.hpp" #include "ai_scanner.hpp" @@ -77,12 +78,7 @@ AIInfo *AIScannerInfo::SelectRandomAI() const } /* Find a random AI */ - uint pos; - if (_networking) { - pos = InteractiveRandomRange(num_random_ais); - } else { - pos = RandomRange(num_random_ais); - } + uint pos = ScriptObject::GetRandomizer(OWNER_NONE).Next(num_random_ais); /* Find the Nth item from the array */ ScriptInfoList::const_iterator it = this->info_single_list.begin(); diff --git a/src/script/script_config.cpp b/src/script/script_config.cpp index 07be966052..738a898a79 100644 --- a/src/script/script_config.cpp +++ b/src/script/script_config.cpp @@ -11,6 +11,7 @@ #include "../settings_type.h" #include "../core/random_func.hpp" #include "script_info.hpp" +#include "api/script_object.hpp" #include "../textfile_gui.h" #include "../string_func.h" @@ -35,7 +36,7 @@ void ScriptConfig::Change(const char *name, int version, bool force_exact_match, * for the Script that have the random flag to a random value. */ for (const auto &item : *this->info->GetConfigList()) { if (item.flags & SCRIPTCONFIG_RANDOM) { - this->SetSetting(item.name, InteractiveRandomRange(item.max_value + 1 - item.min_value) + item.min_value); + this->SetSetting(item.name, ScriptObject::GetRandomizer(OWNER_NONE).Next(item.max_value + 1 - item.min_value) + item.min_value); } } @@ -157,7 +158,7 @@ void ScriptConfig::AddRandomDeviation() { for (const auto &item : *this->GetConfigList()) { if (item.random_deviation != 0) { - this->SetSetting(item.name, InteractiveRandomRange(item.random_deviation * 2 + 1) - item.random_deviation + this->GetSetting(item.name)); + this->SetSetting(item.name, ScriptObject::GetRandomizer(OWNER_NONE).Next(item.random_deviation * 2 + 1) - item.random_deviation + this->GetSetting(item.name)); } } } From b4f0450974a91649b6b72b820c8b8079937dfe23 Mon Sep 17 00:00:00 2001 From: PeterN Date: Sat, 14 Jan 2023 22:39:15 +0000 Subject: [PATCH 15/23] Change: Display font status as aa/noaa instead of true/false. (#10352) --- src/console_cmds.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/console_cmds.cpp b/src/console_cmds.cpp index 65343aba5d..22e3225a01 100644 --- a/src/console_cmds.cpp +++ b/src/console_cmds.cpp @@ -2054,7 +2054,7 @@ DEF_CONSOLE_CMD(ConFont) InitFontCache(fs == FS_MONO); fc = FontCache::Get(fs); } - IConsolePrint(CC_DEFAULT, "{}: \"{}\" {} {} [\"{}\" {} {}]", FontSizeToName(fs), fc->GetFontName(), fc->GetFontSize(), GetFontAAState(fs), setting->font, setting->size, setting->aa); + IConsolePrint(CC_DEFAULT, "{}: \"{}\" {} {} [\"{}\" {} {}]", FontSizeToName(fs), fc->GetFontName(), fc->GetFontSize(), GetFontAAState(fs) ? "aa" : "noaa", setting->font, setting->size, setting->aa ? "aa" : "noaa"); } return true; From 6dfd2cad692312cb273751bee70588fea372294b Mon Sep 17 00:00:00 2001 From: Rubidium Date: Thu, 12 Jan 2023 20:23:56 +0100 Subject: [PATCH 16/23] Fix: comparison result is always the same warnings --- src/command_type.h | 2 +- src/console_cmds.cpp | 27 ++++++++++++++------------- src/network/core/game_info.cpp | 12 ++++++------ 3 files changed, 21 insertions(+), 20 deletions(-) diff --git a/src/command_type.h b/src/command_type.h index d2b573b333..8af665d3b9 100644 --- a/src/command_type.h +++ b/src/command_type.h @@ -25,7 +25,7 @@ class CommandCost { ExpensesType expense_type; ///< the type of expence as shown on the finances view Money cost; ///< The cost of this action StringID message; ///< Warning message for when success is unset - bool success; ///< Whether the comment went fine up to this moment + bool success; ///< Whether the command went fine up to this moment const GRFFile *textref_stack_grffile; ///< NewGRF providing the #TextRefStack content. uint textref_stack_size; ///< Number of uint32 values to put on the #TextRefStack for the error message. diff --git a/src/console_cmds.cpp b/src/console_cmds.cpp index 22e3225a01..88cfaa43e8 100644 --- a/src/console_cmds.cpp +++ b/src/console_cmds.cpp @@ -286,7 +286,11 @@ DEF_CONSOLE_CMD(ConZoomToLevel) case 2: { uint32 level; if (GetArgumentInteger(&level, argv[1])) { - if (level < ZOOM_LVL_MIN) { + /* In case ZOOM_LVL_MIN is more than 0, the next if statement needs to be amended. + * A simple check for less than ZOOM_LVL_MIN does not work here because we are + * reading an unsigned integer from the console, so just check for a '-' char. */ + static_assert(ZOOM_LVL_MIN == 0); + if (argv[1][0] == '-') { IConsolePrint(CC_ERROR, "Zoom-in levels below {} are not supported.", ZOOM_LVL_MIN); } else if (level < _settings_client.gui.zoom_min) { IConsolePrint(CC_ERROR, "Current client settings do not allow zooming in below level {}.", _settings_client.gui.zoom_min); @@ -2012,18 +2016,15 @@ DEF_CONSOLE_CMD(ConFont) bool aa = setting->aa; byte arg_index = 2; - - if (argc > arg_index) { - /* We may encounter "aa" or "noaa" but it must be the last argument. */ - if (strcasecmp(argv[arg_index], "aa") == 0 || strcasecmp(argv[arg_index], "noaa") == 0) { - aa = strncasecmp(argv[arg_index++], "no", 2) != 0; - if (argc > arg_index) return false; - } else { - /* For we want a string. */ - uint v; - if (!GetArgumentInteger(&v, argv[arg_index])) { - font = argv[arg_index++]; - } + /* We may encounter "aa" or "noaa" but it must be the last argument. */ + if (strcasecmp(argv[arg_index], "aa") == 0 || strcasecmp(argv[arg_index], "noaa") == 0) { + aa = strncasecmp(argv[arg_index++], "no", 2) != 0; + if (argc > arg_index) return false; + } else { + /* For we want a string. */ + uint v; + if (!GetArgumentInteger(&v, argv[arg_index])) { + font = argv[arg_index++]; } } diff --git a/src/network/core/game_info.cpp b/src/network/core/game_info.cpp index 1576ae4132..7dbf7c4b70 100644 --- a/src/network/core/game_info.cpp +++ b/src/network/core/game_info.cpp @@ -281,14 +281,14 @@ void DeserializeNetworkGameInfo(Packet *p, NetworkGameInfo *info, const GameInfo } case 4: { - GRFConfig **dst = &info->grfconfig; - uint i; + /* Ensure that the maximum number of NewGRFs and the field in the network + * protocol are matched to eachother. If that is not the case anymore a + * check must be added to ensure the received data is still valid. */ + static_assert(std::numeric_limits::max() == NETWORK_MAX_GRF_COUNT); uint num_grfs = p->Recv_uint8(); - /* Broken/bad data. It cannot have that many NewGRFs. */ - if (num_grfs > NETWORK_MAX_GRF_COUNT) return; - - for (i = 0; i < num_grfs; i++) { + GRFConfig **dst = &info->grfconfig; + for (uint i = 0; i < num_grfs; i++) { NamedGRFIdentifier grf; switch (newgrf_serialisation) { case NST_GRFID_MD5: From 20a9e13272fa0652142778059f4b0dbbd093f9f9 Mon Sep 17 00:00:00 2001 From: Rubidium Date: Thu, 12 Jan 2023 21:24:30 +0100 Subject: [PATCH 17/23] Fix: inconsistent definition of copy constructor and assignment --- src/3rdparty/fmt/core.h | 1 - src/3rdparty/squirrel/squirrel/sqclass.h | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/3rdparty/fmt/core.h b/src/3rdparty/fmt/core.h index 0a81e0ccd9..bddad78ec4 100644 --- a/src/3rdparty/fmt/core.h +++ b/src/3rdparty/fmt/core.h @@ -529,7 +529,6 @@ void check_format_string(S); struct error_handler { constexpr error_handler() = default; - constexpr error_handler(const error_handler&) = default; // This function is intentionally not constexpr to give a compile-time error. FMT_NORETURN FMT_API void on_error(const char* message); diff --git a/src/3rdparty/squirrel/squirrel/sqclass.h b/src/3rdparty/squirrel/squirrel/sqclass.h index 4fb6ecbd97..dad4ba08af 100644 --- a/src/3rdparty/squirrel/squirrel/sqclass.h +++ b/src/3rdparty/squirrel/squirrel/sqclass.h @@ -10,6 +10,7 @@ struct SQClassMember { val = o.val; attrs = o.attrs; } + SQClassMember& operator=(SQClassMember &o) = delete; SQObjectPtr val; SQObjectPtr attrs; }; From 96ec9908a051fa22fcfc9a57912e286c9f6b3bb9 Mon Sep 17 00:00:00 2001 From: SamuXarick <43006711+SamuXarick@users.noreply.github.com> Date: Sat, 14 Jan 2023 23:43:41 +0000 Subject: [PATCH 18/23] Codechange: refactor removal of desert around river tiles --- src/landscape.cpp | 38 ++++++-------------------------------- src/water.h | 1 + src/water_cmd.cpp | 12 ++++++++++++ 3 files changed, 19 insertions(+), 32 deletions(-) diff --git a/src/landscape.cpp b/src/landscape.cpp index 66912ee3fc..31cdd7f6df 100644 --- a/src/landscape.cpp +++ b/src/landscape.cpp @@ -1064,11 +1064,7 @@ static bool MakeLake(TileIndex tile, void *user_data) for (DiagDirection d = DIAGDIR_BEGIN; d < DIAGDIR_END; d++) { TileIndex t2 = tile + TileOffsByDiagDir(d); if (IsWaterTile(t2)) { - MakeRiver(tile, Random()); - MarkTileDirtyByTile(tile); - /* Remove desert directly around the river tile. */ - TileIndex t = tile; - CircularTileSearch(&t, RIVER_OFFSET_DESERT_DISTANCE, RiverModifyDesertZone, nullptr); + MakeRiverAndModifyDesertZoneAround(tile); return false; } } @@ -1202,12 +1198,7 @@ static bool RiverMakeWider(TileIndex tile, void *data) /* If the tile upstream isn't flat, don't bother. */ if (GetTileSlope(downstream_tile) != SLOPE_FLAT) return false; - MakeRiver(downstream_tile, Random()); - MarkTileDirtyByTile(downstream_tile); - - /* Remove desert directly around the river tile. */ - TileIndex cur_tile = downstream_tile; - CircularTileSearch(&cur_tile, RIVER_OFFSET_DESERT_DISTANCE, RiverModifyDesertZone, nullptr); + MakeRiverAndModifyDesertZoneAround(downstream_tile); } /* If upstream is dry and flat, try making it a river tile. */ @@ -1215,23 +1206,13 @@ static bool RiverMakeWider(TileIndex tile, void *data) /* If the tile upstream isn't flat, don't bother. */ if (GetTileSlope(upstream_tile) != SLOPE_FLAT) return false; - MakeRiver(upstream_tile, Random()); - MarkTileDirtyByTile(upstream_tile); - - /* Remove desert directly around the river tile. */ - TileIndex cur_tile = upstream_tile; - CircularTileSearch(&cur_tile, RIVER_OFFSET_DESERT_DISTANCE, RiverModifyDesertZone, nullptr); + MakeRiverAndModifyDesertZoneAround(upstream_tile); } } /* If the tile slope matches the desired slope, add a river tile. */ if (cur_slope == desired_slope) { - MakeRiver(tile, Random()); - MarkTileDirtyByTile(tile); - - /* Remove desert directly around the river tile. */ - TileIndex cur_tile = tile; - CircularTileSearch(&cur_tile, RIVER_OFFSET_DESERT_DISTANCE, RiverModifyDesertZone, nullptr); + MakeRiverAndModifyDesertZoneAround(tile); } /* Always return false to keep searching. */ @@ -1310,10 +1291,7 @@ static void River_FoundEndNode(AyStar *aystar, OpenListNode *current) for (PathNode *path = ¤t->path; path != nullptr; path = path->parent, cur_pos++) { TileIndex tile = path->node.tile; if (!IsWaterTile(tile)) { - MakeRiver(tile, Random()); - MarkTileDirtyByTile(tile); - /* Remove desert directly around the river tile. */ - CircularTileSearch(&tile, RIVER_OFFSET_DESERT_DISTANCE, RiverModifyDesertZone, nullptr); + MakeRiverAndModifyDesertZoneAround(tile); } } @@ -1453,11 +1431,7 @@ static std::tuple FlowRiver(TileIndex spring, TileIndex begin, uint /* We only want a lake if the river is long enough. */ DistanceManhattan(spring, lakeCenter) > min_river_length) { end = lakeCenter; - MakeRiver(lakeCenter, Random()); - MarkTileDirtyByTile(lakeCenter); - /* Remove desert directly around the river tile. */ - CircularTileSearch(&lakeCenter, RIVER_OFFSET_DESERT_DISTANCE, RiverModifyDesertZone, nullptr); - lakeCenter = end; + MakeRiverAndModifyDesertZoneAround(lakeCenter); uint range = RandomRange(8) + 3; CircularTileSearch(&lakeCenter, range, MakeLake, &height); /* Call the search a second time so artefacts from going circular in one direction get (mostly) hidden. */ diff --git a/src/water.h b/src/water.h index 27c3339c18..379c13ef72 100644 --- a/src/water.h +++ b/src/water.h @@ -39,6 +39,7 @@ void MakeWaterKeepingClass(TileIndex tile, Owner o); void CheckForDockingTile(TileIndex t); bool RiverModifyDesertZone(TileIndex tile, void *data); +void MakeRiverAndModifyDesertZoneAround(TileIndex tile); static const uint RIVER_OFFSET_DESERT_DISTANCE = 5; ///< Circular tile search radius to create non-desert around a river tile. bool IsWateredTile(TileIndex tile, Direction from); diff --git a/src/water_cmd.cpp b/src/water_cmd.cpp index 68b2cef3e7..0e23b7259c 100644 --- a/src/water_cmd.cpp +++ b/src/water_cmd.cpp @@ -427,6 +427,18 @@ bool RiverModifyDesertZone(TileIndex tile, void *) return false; } +/** + * Make a river tile and remove desert directly around it. + * @param tile The tile to change into river and create non-desert around + */ +void MakeRiverAndModifyDesertZoneAround(TileIndex tile) { + MakeRiver(tile, Random()); + MarkTileDirtyByTile(tile); + + /* Remove desert directly around the river tile. */ + CircularTileSearch(&tile, RIVER_OFFSET_DESERT_DISTANCE, RiverModifyDesertZone, nullptr); +} + /** * Build a piece of canal. * @param flags type of operation From c578917783b1235f4264fb890a88d2372d880916 Mon Sep 17 00:00:00 2001 From: Rubidium Date: Sun, 15 Jan 2023 08:51:00 +0100 Subject: [PATCH 19/23] Fix #10057: FallbackParagraphLayout fails to properly wrap ... during the first word after a new run has been started. --- src/gfx_layout.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/gfx_layout.cpp b/src/gfx_layout.cpp index 40f9adeb94..1a163330d2 100644 --- a/src/gfx_layout.cpp +++ b/src/gfx_layout.cpp @@ -552,8 +552,6 @@ std::unique_ptr FallbackParagraphLayout::NextLine next_run = this->buffer_begin + iter->first; begin = this->buffer; - - last_space = nullptr; } if (IsWhitespace(c)) last_space = this->buffer; @@ -591,7 +589,7 @@ std::unique_ptr FallbackParagraphLayout::NextLine this->buffer++; } - if (l->size() == 0 || last_char - begin != 0) { + if (l->size() == 0 || last_char - begin > 0) { int w = l->GetWidth(); l->emplace_back(iter->second, begin, last_char - begin, w); } From 8f9a60893d03aa6c5ddd58aa8c5ecec91558c212 Mon Sep 17 00:00:00 2001 From: Rubidium Date: Sun, 15 Jan 2023 07:24:18 +0100 Subject: [PATCH 20/23] Fix #10177: company list password padlock showed after switching to single player --- src/network/network.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/network/network.cpp b/src/network/network.cpp index 19cb6ab3a4..c3b4974c36 100644 --- a/src/network/network.cpp +++ b/src/network/network.cpp @@ -609,6 +609,7 @@ void NetworkClose(bool close_admins) delete[] _network_company_states; _network_company_states = nullptr; + _network_company_passworded = 0; InitializeNetworkPools(close_admins); } From 2355882ec14a43f906f2baf39b67be46d0dfad49 Mon Sep 17 00:00:00 2001 From: PeterN Date: Sun, 15 Jan 2023 10:58:03 +0000 Subject: [PATCH 21/23] Codechange: Remove object `enabled` flag and shuffle members. (#10358) `enabled` flag is replaced with IsEnabled() which checks if views is non-zero. ObjectSpec is shuffled to reduce its memory footprint. --- src/newgrf.cpp | 3 +-- src/newgrf_object.cpp | 2 +- src/newgrf_object.h | 13 +++++++++---- src/object_cmd.cpp | 4 ++-- src/table/object_land.h | 2 +- 5 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/newgrf.cpp b/src/newgrf.cpp index bbf29750a0..69d41af8d5 100644 --- a/src/newgrf.cpp +++ b/src/newgrf.cpp @@ -4113,7 +4113,6 @@ static ChangeInfoResult ObjectChangeInfo(uint id, int numinfo, int prop, ByteRea /* Swap classid because we read it in BE. */ uint32 classid = buf->ReadDWord(); (*ospec)->cls_id = ObjectClass::Allocate(BSWAP32(classid)); - (*ospec)->enabled = true; break; } @@ -9283,7 +9282,7 @@ static void FinaliseObjectsArray() ObjectSpec **&objectspec = file->objectspec; if (objectspec != nullptr) { for (int i = 0; i < NUM_OBJECTS_PER_GRF; i++) { - if (objectspec[i] != nullptr && objectspec[i]->grf_prop.grffile != nullptr && objectspec[i]->enabled) { + if (objectspec[i] != nullptr && objectspec[i]->grf_prop.grffile != nullptr && objectspec[i]->IsEnabled()) { _object_mngr.SetEntitySpec(objectspec[i]); } } diff --git a/src/newgrf_object.cpp b/src/newgrf_object.cpp index e69787c0ce..730fa7afd0 100644 --- a/src/newgrf_object.cpp +++ b/src/newgrf_object.cpp @@ -58,7 +58,7 @@ ObjectSpec _object_specs[NUM_OBJECTS]; */ bool ObjectSpec::IsEverAvailable() const { - return this->enabled && HasBit(this->climate, _settings_game.game_creation.landscape) && + return this->IsEnabled() && HasBit(this->climate, _settings_game.game_creation.landscape) && (this->flags & ((_game_mode != GM_EDITOR && !_generating_world) ? OBJECT_FLAG_ONLY_IN_SCENEDIT : OBJECT_FLAG_ONLY_IN_GAME)) == 0; } diff --git a/src/newgrf_object.h b/src/newgrf_object.h index fa48506ac2..a441d3ceec 100644 --- a/src/newgrf_object.h +++ b/src/newgrf_object.h @@ -21,7 +21,7 @@ #include "newgrf_commons.h" /** Various object behaviours. */ -enum ObjectFlags { +enum ObjectFlags : uint16 { OBJECT_FLAG_NONE = 0, ///< Just nothing. OBJECT_FLAG_ONLY_IN_SCENEDIT = 1 << 0, ///< Object can only be constructed in the scenario editor. OBJECT_FLAG_CANNOT_REMOVE = 1 << 1, ///< Object can not be removed. @@ -45,7 +45,7 @@ static const uint8 OBJECT_SIZE_1X1 = 0x11; ///< The value of a NewGRF's size pro void ResetObjects(); /** Class IDs for objects. */ -enum ObjectClassID { +enum ObjectClassID : uint8 { OBJECT_CLASS_BEGIN = 0, ///< The lowest valid value OBJECT_CLASS_MAX = 0xFF, ///< Maximum number of classes. INVALID_OBJECT_CLASS = 0xFF, ///< Class for the less fortunate. @@ -60,6 +60,7 @@ DECLARE_POSTFIX_INCREMENT(ObjectClassID) struct ObjectSpec { /* 2 because of the "normal" and "buy" sprite stacks. */ GRFFilePropsBase<2> grf_prop; ///< Properties related the the grf file + AnimationInfo animation; ///< Information about the animation. ObjectClassID cls_id; ///< The class to which this spec belongs. StringID name; ///< The name for this object. @@ -70,12 +71,16 @@ struct ObjectSpec { Date introduction_date; ///< From when can this object be built. Date end_of_life_date; ///< When can't this object be built anymore. ObjectFlags flags; ///< Flags/settings related to the object. - AnimationInfo animation; ///< Information about the animation. uint16 callback_mask; ///< Bitmask of requested/allowed callbacks. uint8 height; ///< The height of this structure, in heightlevels; max MAX_TILE_HEIGHT. uint8 views; ///< The number of views. uint8 generate_amount; ///< Number of objects which are attempted to be generated per 256^2 map during world generation. - bool enabled; ///< Is this spec enabled? + + /** + * Test if this object is enabled. + * @return True iif this object is enabled. + */ + bool IsEnabled() const { return this->views > 0; } /** * Get the cost for building a structure of this type. diff --git a/src/object_cmd.cpp b/src/object_cmd.cpp index 4c9b95809f..b5fe6f561d 100644 --- a/src/object_cmd.cpp +++ b/src/object_cmd.cpp @@ -437,7 +437,7 @@ static void DrawTile_Object(TileInfo *ti) const ObjectSpec *spec = ObjectSpec::Get(type); /* Fall back for when the object doesn't exist anymore. */ - if (!spec->enabled) type = OBJECT_TRANSMITTER; + if (!spec->IsEnabled()) type = OBJECT_TRANSMITTER; if ((spec->flags & OBJECT_FLAG_HAS_NO_FOUNDATION) == 0) DrawFoundation(ti, GetFoundation_Object(ti->tile, ti->tileh)); @@ -906,7 +906,7 @@ static CommandCost TerraformTile_Object(TileIndex tile, DoCommandFlag flags, int /* If the callback fails, allow autoslope. */ uint16 res = GetObjectCallback(CBID_OBJECT_AUTOSLOPE, 0, 0, spec, Object::GetByTile(tile), tile); if (res == CALLBACK_FAILED || !ConvertBooleanCallback(spec->grf_prop.grffile, CBID_OBJECT_AUTOSLOPE, res)) return CommandCost(EXPENSES_CONSTRUCTION, _price[PR_BUILD_FOUNDATION]); - } else if (spec->enabled) { + } else if (spec->IsEnabled()) { /* allow autoslope */ return CommandCost(EXPENSES_CONSTRUCTION, _price[PR_BUILD_FOUNDATION]); } diff --git a/src/table/object_land.h b/src/table/object_land.h index ed26a0f500..e5371712e7 100644 --- a/src/table/object_land.h +++ b/src/table/object_land.h @@ -121,7 +121,7 @@ static const DrawTileSprites _object_hq[] = { #undef TILE_SPRITE_LINE -#define M(name, size, build_cost_multiplier, clear_cost_multiplier, height, climate, gen_amount, flags) { GRFFilePropsBase<2>(), INVALID_OBJECT_CLASS, name, climate, size, build_cost_multiplier, clear_cost_multiplier, 0, MAX_DAY + 1, flags, {0, 0, 0, 0}, 0, height, 1, gen_amount, true } +#define M(name, size, build_cost_multiplier, clear_cost_multiplier, height, climate, gen_amount, flags) { GRFFilePropsBase<2>(), {0, 0, 0, 0}, INVALID_OBJECT_CLASS, name, climate, size, build_cost_multiplier, clear_cost_multiplier, 0, MAX_DAY + 1, flags, 0, height, 1, gen_amount } /* Climates * T = Temperate From 1ed0b35520e7d0a5545dff3f73539fd8021522ef Mon Sep 17 00:00:00 2001 From: Rubidium Date: Sun, 15 Jan 2023 11:08:08 +0100 Subject: [PATCH 22/23] Fix #10009: bad overflow protection when taking out loans --- src/core/overflowsafe_type.hpp | 3 +++ src/misc_cmd.cpp | 6 ++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/core/overflowsafe_type.hpp b/src/core/overflowsafe_type.hpp index 14f0eaa7a7..0c3957aaad 100644 --- a/src/core/overflowsafe_type.hpp +++ b/src/core/overflowsafe_type.hpp @@ -176,6 +176,9 @@ public: inline constexpr bool operator <= (const int other) const { return !(*this > other); } inline constexpr operator T () const { return this->m_value; } + + static inline constexpr OverflowSafeInt max() { return T_MAX; } + static inline constexpr OverflowSafeInt min() { return T_MIN; } }; diff --git a/src/misc_cmd.cpp b/src/misc_cmd.cpp index 6422bcf3ac..a18504c1ef 100644 --- a/src/misc_cmd.cpp +++ b/src/misc_cmd.cpp @@ -60,8 +60,10 @@ CommandCost CmdIncreaseLoan(DoCommandFlag flags, LoanCommand cmd, Money amount) break; } - /* Overflow protection */ - if (c->money + c->current_loan + loan < c->money) return CMD_ERROR; + /* In case adding the loan triggers the overflow protection of Money, + * we would essentially be losing money as taking and repaying the loan + * immediately would not get us back to the same bank balance anymore. */ + if (c->money > Money::max() - loan) return CMD_ERROR; if (flags & DC_EXEC) { c->money += loan; From a4a819c983e3b63ca25ad7d97c64a20ce9ac451c Mon Sep 17 00:00:00 2001 From: Rubidium Date: Sun, 15 Jan 2023 10:53:32 +0100 Subject: [PATCH 23/23] Fix #9865: removing files with the console always failed --- src/console_cmds.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/console_cmds.cpp b/src/console_cmds.cpp index 88cfaa43e8..67d3cd03b0 100644 --- a/src/console_cmds.cpp +++ b/src/console_cmds.cpp @@ -458,8 +458,8 @@ DEF_CONSOLE_CMD(ConRemove) _console_file_list.ValidateFileList(); const FiosItem *item = _console_file_list.FindItem(file); if (item != nullptr) { - if (!FiosDelete(item->name)) { - IConsolePrint(CC_ERROR, "Failed to delete '{}'.", file); + if (unlink(item->name) != 0) { + IConsolePrint(CC_ERROR, "Failed to delete '{}'.", item->name); } } else { IConsolePrint(CC_ERROR, "'{}' could not be found.", file);