diff options
| -rw-r--r-- | src/autoreplace_cmd.cpp | 21 | ||||
| -rw-r--r-- | src/command.cpp | 8 | ||||
| -rw-r--r-- | src/vehicle.cpp | 43 |
3 files changed, 43 insertions, 29 deletions
diff --git a/src/autoreplace_cmd.cpp b/src/autoreplace_cmd.cpp index 2eafe9812..ae49128d7 100644 --- a/src/autoreplace_cmd.cpp +++ b/src/autoreplace_cmd.cpp @@ -199,12 +199,21 @@ static CommandCost ReplaceVehicle(Vehicle **w, byte flags, Money total_cost) */ /* Get the vehicle in front of the one we move out */ Vehicle *front = old_v->Previous(); - /* If the vehicle in front is the rear end of a dualheaded engine, then we need to use the one in front of that one */ - if (IsRearDualheaded(front)) front = front->Previous(); - /* Now we move the old one out of the train */ - DoCommand(0, (INVALID_VEHICLE << 16) | old_v->index, 0, DC_EXEC, CMD_MOVE_RAIL_VEHICLE); - /* Add the new vehicle */ - DoCommand(0, (front->index << 16) | new_v->index, 1, DC_EXEC, CMD_MOVE_RAIL_VEHICLE); + if (front == NULL) { + /* It would appear that we have the front wagon of a row of wagons without engines */ + Vehicle *next = old_v->Next(); + if (next != NULL) { + /* Move the chain to the new front wagon */ + DoCommand(0, (new_v->index << 16) | next->index, 1, DC_EXEC, CMD_MOVE_RAIL_VEHICLE); + } + } else { + /* If the vehicle in front is the rear end of a dualheaded engine, then we need to use the one in front of that one */ + if (IsRearDualheaded(front)) front = front->Previous(); + /* Now we move the old one out of the train */ + DoCommand(0, (INVALID_VEHICLE << 16) | old_v->index, 0, DC_EXEC, CMD_MOVE_RAIL_VEHICLE); + /* Add the new vehicle */ + DoCommand(0, (front->index << 16) | new_v->index, 1, DC_EXEC, CMD_MOVE_RAIL_VEHICLE); + } } else { // copy/clone the orders DoCommand(0, (old_v->index << 16) | new_v->index, old_v->IsOrderListShared() ? CO_SHARE : CO_COPY, DC_EXEC, CMD_CLONE_ORDER); diff --git a/src/command.cpp b/src/command.cpp index bb519cbd3..c00a40eaa 100644 --- a/src/command.cpp +++ b/src/command.cpp @@ -542,12 +542,16 @@ bool DoCommandP(TileIndex tile, uint32 p1, uint32 p2, CommandCallback *callback, * fact will trigger an assertion failure. --pasky * CMD_CLONE_VEHICLE: Both building new vehicles and refitting them can be * influenced by newgrf callbacks, which makes it impossible to accurately - * estimate the cost of cloning a vehicle. */ + * estimate the cost of cloning a vehicle. + * CMD_DEPOT_MASS_AUTOREPLACE: we can't predict wagon removal so + * the test will not include income from any sold wagons. + * This means that the costs can sometimes be lower than estimated. */ notest = (cmd & 0xFF) == CMD_CLEAR_AREA || (cmd & 0xFF) == CMD_LEVEL_LAND || (cmd & 0xFF) == CMD_REMOVE_LONG_ROAD || - (cmd & 0xFF) == CMD_CLONE_VEHICLE; + (cmd & 0xFF) == CMD_CLONE_VEHICLE || + (cmd & 0xFF) == CMD_DEPOT_MASS_AUTOREPLACE; _docommand_recursive = 1; diff --git a/src/vehicle.cpp b/src/vehicle.cpp index a7185f076..4fd3e0d42 100644 --- a/src/vehicle.cpp +++ b/src/vehicle.cpp @@ -1687,24 +1687,28 @@ CommandCost CmdDepotSellAllVehicles(TileIndex tile, uint32 flags, uint32 p1, uin } /** Autoreplace all vehicles in the depot + * Note: this command can make incorrect cost estimations + * Luckily the final price can only drop, not increase. This is due to the fact that + * estimation can't predict wagon removal so it presumes worst case which is no income from selling wagons. * @param tile Tile of the depot where the vehicles are * @param flags type of operation * @param p1 Type of vehicle - * @param p2 Unused + * @param p2 If bit 0 is set, then either replace all or nothing (instead of replacing until money runs out) */ CommandCost CmdDepotMassAutoReplace(TileIndex tile, uint32 flags, uint32 p1, uint32 p2) { Vehicle **vl = NULL; uint16 engine_list_length = 0; uint16 engine_count = 0; - uint i, x = 0, y = 0, z = 0; - CommandCost cost; + uint i; + CommandCost cost = CommandCost(EXPENSES_NEW_VEHICLES); VehicleType vehicle_type = (VehicleType)GB(p1, 0, 8); + bool all_or_nothing = HasBit(p2, 0); if (!IsDepotTile(tile) || !IsTileOwner(tile, _current_player)) return CMD_ERROR; /* Get the list of vehicles in the depot */ - BuildDepotVehicleList(vehicle_type, tile, &vl, &engine_list_length, &engine_count, NULL, NULL, NULL); + BuildDepotVehicleList(vehicle_type, tile, &vl, &engine_list_length, &engine_count, &vl, &engine_list_length, &engine_count); for (i = 0; i < engine_count; i++) { @@ -1715,10 +1719,6 @@ CommandCost CmdDepotMassAutoReplace(TileIndex tile, uint32 flags, uint32 p1, uin /* Ensure that the vehicle completely in the depot */ if (!v->IsInDepot()) continue; - x = v->x_pos; - y = v->y_pos; - z = v->z_pos; - if (stopped) { v->vehstatus |= VS_STOPPED; // Stop the vehicle v->leave_depot_instantly = true; @@ -1727,24 +1727,25 @@ CommandCost CmdDepotMassAutoReplace(TileIndex tile, uint32 flags, uint32 p1, uin if (CmdSucceeded(ret)) { cost.AddCost(ret); - if (!(flags & DC_EXEC)) break; - /* There is a problem with autoreplace and newgrf - * It's impossible to tell the length of a train after it's being replaced before it's actually done - * Because of this, we can't estimate costs due to wagon removal and we will have to always return 0 and pay manually - * Since we pay after each vehicle is replaced and MaybeReplaceVehicle() check if the player got enough money - * we should never reach a condition where the player will end up with negative money from doing this */ - SubtractMoneyFromPlayer(ret); + } else { + if (all_or_nothing) { + /* We failed to replace a vehicle even though we set all or nothing. + * We should never reach this if DC_EXEC is set since then it should + * have failed the estimation guess. */ + assert(!(flags & DC_EXEC)); + /* Now we will have to return an error. + * This goto will leave the loop and it's ok to do so because + * there is no point in the rest of the loop. */ + goto error; + } } } if (cost.GetCost() == 0) { +error: + /* Either we didn't replace anything or something went wrong. + * Either way we want to return an error and not execute this command. */ cost = CMD_ERROR; - } else { - if (flags & DC_EXEC) { - /* Display the cost animation now that DoCommandP() can't do it for us (see previous comments) */ - if (IsLocalPlayer()) ShowCostOrIncomeAnimation(x, y, z, cost.GetCost()); - } - cost = CommandCost(); } free(vl); |