(svn r17897) -Fix [FS#3255]: CB15 and CB36 (capacity) were not always called when they should.

-Codechange: Move capacity calculation to a single function for all vehicle types, so the behaviour can be kept consistent easier.

8 files changed, 114 insertions, 163 deletions

diff --git a/src/aircraft.h b/src/aircraft.h
index 9d745f52e..d1664886a 100644
--- a/src/aircraft.h
+++ b/src/aircraft.h
@@ -29,15 +29,6 @@ enum AircraftSubType {
 
 
 /**
- * Calculates cargo capacity based on an aircraft's passenger
- * and mail capacities.
- * @param cid Which cargo type to calculate a capacity for.
- * @param avi Which engine to find a cargo capacity for.
- * @return New cargo capacity value.
- */
-uint16 AircraftDefaultCargoCapacity(CargoID cid, const AircraftVehicleInfo *avi);
-
-/**
  * This is the Callback method after the construction attempt of an aircraft
  * @param success indicates completion (or not) of the operation
  * @param tile of depot where aircraft is built
diff --git a/src/aircraft_cmd.cpp b/src/aircraft_cmd.cpp
index f7cfa17f4..c5a422c8e 100644
--- a/src/aircraft_cmd.cpp
+++ b/src/aircraft_cmd.cpp
@@ -223,31 +223,6 @@ void GetAircraftSpriteSize(EngineID engine, uint &width, uint &height)
 	height = spr->height;
 }
 
-/**
- * Calculates cargo capacity based on an aircraft's passenger
- * and mail capacities.
- * @param cid Which cargo type to calculate a capacity for.
- * @param avi Which engine to find a cargo capacity for.
- * @return New cargo capacity value.
- */
-uint16 AircraftDefaultCargoCapacity(CargoID cid, const AircraftVehicleInfo *avi)
-{
-	assert(cid != CT_INVALID);
-
-	/* An aircraft can carry twice as much goods as normal cargo,
-	 * and four times as many passengers. */
-	switch (cid) {
-		case CT_PASSENGERS:
-			return avi->passenger_capacity;
-		case CT_MAIL:
-			return avi->passenger_capacity + avi->mail_capacity;
-		case CT_GOODS:
-			return (avi->passenger_capacity + avi->mail_capacity) / 2;
-		default:
-			return (avi->passenger_capacity + avi->mail_capacity) / 4;
-	}
-}
-
 /** Build an aircraft.
  * @param tile tile of depot where aircraft is built
  * @param flags for command
@@ -385,20 +360,8 @@ CommandCost CmdBuildAircraft(TileIndex tile, DoCommandFlag flags, uint32 p1, uin
 
 		v->InvalidateNewGRFCacheOfChain();
 
+		v->cargo_cap = GetVehicleCapacity(v);
 		if (v->cargo_type != CT_PASSENGERS) {
-			uint16 callback = CALLBACK_FAILED;
-
-			if (HasBit(e->info.callback_mask, CBM_VEHICLE_REFIT_CAPACITY)) {
-				callback = GetVehicleCallback(CBID_VEHICLE_REFIT_CAPACITY, 0, 0, v->engine_type, v);
-			}
-
-			if (callback == CALLBACK_FAILED) {
-				/* Callback failed, or not executed; use the default cargo capacity */
-				v->cargo_cap = AircraftDefaultCargoCapacity(v->cargo_type, avi);
-			} else {
-				v->cargo_cap = callback;
-			}
-
 			/* Set the 'second compartent' capacity to none */
 			u->cargo_cap = 0;
 		}
@@ -540,31 +503,20 @@ CommandCost CmdRefitAircraft(TileIndex tile, DoCommandFlag flags, uint32 p1, uin
 	if (new_cid >= NUM_CARGO || !CanRefitTo(v->engine_type, new_cid)) return CMD_ERROR;
 
 	const Engine *e = Engine::Get(v->engine_type);
+	v->InvalidateNewGRFCacheOfChain();
 
-	/* Check the refit capacity callback */
-	uint16 callback = CALLBACK_FAILED;
-	if (HasBit(e->info.callback_mask, CBM_VEHICLE_REFIT_CAPACITY)) {
-		/* Back up the existing cargo type */
-		CargoID temp_cid = v->cargo_type;
-		byte temp_subtype = v->cargo_subtype;
-		v->cargo_type = new_cid;
-		v->cargo_subtype = new_subtype;
+	/* Back up the existing cargo type */
+	CargoID temp_cid = v->cargo_type;
+	byte temp_subtype = v->cargo_subtype;
+	v->cargo_type = new_cid;
+	v->cargo_subtype = new_subtype;
 
-		callback = GetVehicleCallback(CBID_VEHICLE_REFIT_CAPACITY, 0, 0, v->engine_type, v);
+	uint pass = GetVehicleCapacity(v);
 
-		/* Restore the cargo type */
-		v->cargo_type = temp_cid;
-		v->cargo_subtype = temp_subtype;
-	}
+	/* Restore the cargo type */
+	v->cargo_type = temp_cid;
+	v->cargo_subtype = temp_subtype;
 
-	uint pass;
-	if (callback == CALLBACK_FAILED) {
-		/* If the callback failed, or wasn't executed, use the aircraft's
-		 * default cargo capacity */
-		pass = AircraftDefaultCargoCapacity(new_cid, &e->u.air);
-	} else {
-		pass = callback;
-	}
 	_returned_refit_capacity = pass;
 
 	CommandCost cost;
diff --git a/src/engine.cpp b/src/engine.cpp
index d58124824..5ea56efdb 100644
--- a/src/engine.cpp
+++ b/src/engine.cpp
@@ -162,6 +162,7 @@ bool Engine::CanCarryCargo() const
  * For multiheaded engines this is the capacity of both heads.
  * For articulated engines use GetCapacityOfArticulatedParts
  *
+ * @note Keep this function consistent with GetVehicleCapacity().
  * @return The default capacity
  * @see GetDefaultCargoType
  */
@@ -179,7 +180,12 @@ uint Engine::GetDisplayDefaultCapacity() const
 			return GetEngineProperty(this->index, PROP_SHIP_CARGO_CAPACITY, this->u.ship.capacity);
 
 		case VEH_AIRCRAFT:
-			return AircraftDefaultCargoCapacity(this->GetDefaultCargoType(), &this->u.air);
+			switch (this->GetDefaultCargoType()) {
+				case CT_PASSENGERS: return this->u.air.passenger_capacity;
+				case CT_MAIL:       return this->u.air.passenger_capacity + this->u.air.mail_capacity;
+				case CT_GOODS:      return (this->u.air.passenger_capacity + this->u.air.mail_capacity) / 2;
+				default:            return (this->u.air.passenger_capacity + this->u.air.mail_capacity) / 4;
+			}
 
 		default: NOT_REACHED();
 	}
diff --git a/src/roadveh_cmd.cpp b/src/roadveh_cmd.cpp
index 74271a1d7..bbdbb874b 100644
--- a/src/roadveh_cmd.cpp
+++ b/src/roadveh_cmd.cpp
@@ -283,8 +283,7 @@ CommandCost CmdBuildRoadVeh(TileIndex tile, DoCommandFlag flags, uint32 p1, uint
 
 		/* Call various callbacks after the whole consist has been constructed */
 		for (RoadVehicle *u = v; u != NULL; u = u->Next()) {
-			/* Cargo capacity is zero if and only if the vehicle cannot carry anything */
-			if (u->cargo_cap != 0) u->cargo_cap = GetVehicleProperty(u, PROP_ROADVEH_CARGO_CAPACITY, u->cargo_cap);
+			u->cargo_cap = GetVehicleCapacity(u);
 			v->InvalidateNewGRFCache();
 			u->InvalidateNewGRFCache();
 		}
@@ -2008,7 +2007,6 @@ CommandCost CmdRefitRoadVeh(TileIndex tile, DoCommandFlag flags, uint32 p1, uint
 	CargoID new_cid = GB(p2, 0, 8);
 	byte new_subtype = GB(p2, 8, 8);
 	bool only_this = HasBit(p2, 16);
-	uint16 capacity = CALLBACK_FAILED;
 	uint total_capacity = 0;
 
 	RoadVehicle *v = RoadVehicle::GetIfValid(p1);
@@ -2019,6 +2017,7 @@ CommandCost CmdRefitRoadVeh(TileIndex tile, DoCommandFlag flags, uint32 p1, uint
 
 	if (new_cid >= NUM_CARGO) return CMD_ERROR;
 
+	v->InvalidateNewGRFCacheOfChain();
 	for (; v != NULL; v = (only_this ? NULL : v->Next())) {
 		/* XXX: We refit all the attached wagons en-masse if they can be
 		 * refitted. This is how TTDPatch does it.  TODO: Have some nice
@@ -2028,43 +2027,17 @@ CommandCost CmdRefitRoadVeh(TileIndex tile, DoCommandFlag flags, uint32 p1, uint
 		const Engine *e = Engine::Get(v->engine_type);
 		if (!e->CanCarryCargo()) continue;
 
-		if (HasBit(e->info.callback_mask, CBM_VEHICLE_REFIT_CAPACITY)) {
-			/* Back up the cargo type */
-			CargoID temp_cid = v->cargo_type;
-			byte temp_subtype = v->cargo_subtype;
-			v->cargo_type = new_cid;
-			v->cargo_subtype = new_subtype;
-
-			/* Check the refit capacity callback */
-			capacity = GetVehicleCallback(CBID_VEHICLE_REFIT_CAPACITY, 0, 0, v->engine_type, v);
-
-			/* Restore the original cargo type */
-			v->cargo_type = temp_cid;
-			v->cargo_subtype = temp_subtype;
-		}
+		/* Back up the cargo type */
+		CargoID temp_cid = v->cargo_type;
+		byte temp_subtype = v->cargo_subtype;
+		v->cargo_type = new_cid;
+		v->cargo_subtype = new_subtype;
 
-		if (capacity == CALLBACK_FAILED) {
-			/* callback failed or not used, use default capacity */
+		uint capacity = GetVehicleCapacity(v);
 
-			CargoID old_cid = e->GetDefaultCargoType();
-			/* normally, the capacity depends on the cargo type, a vehicle can
-			 * carry twice as much mail/goods as normal cargo, and four times as
-			 * many passengers
-			 */
-			capacity = GetVehicleProperty(v, PROP_ROADVEH_CARGO_CAPACITY, e->u.road.capacity);
-			switch (old_cid) {
-				case CT_PASSENGERS: break;
-				case CT_MAIL:
-				case CT_GOODS: capacity *= 2; break;
-				default:       capacity *= 4; break;
-			}
-			switch (new_cid) {
-				case CT_PASSENGERS: break;
-				case CT_MAIL:
-				case CT_GOODS: capacity /= 2; break;
-				default:       capacity /= 4; break;
-			}
-		}
+		/* Restore the original cargo type */
+		v->cargo_type = temp_cid;
+		v->cargo_subtype = temp_subtype;
 
 		total_capacity += capacity;
 
diff --git a/src/ship_cmd.cpp b/src/ship_cmd.cpp
index 0025aabac..7c64e4ee5 100644
--- a/src/ship_cmd.cpp
+++ b/src/ship_cmd.cpp
@@ -813,7 +813,7 @@ CommandCost CmdBuildShip(TileIndex tile, DoCommandFlag flags, uint32 p1, uint32
 
 		v->InvalidateNewGRFCacheOfChain();
 
-		v->cargo_cap = GetVehicleProperty(v, PROP_SHIP_CARGO_CAPACITY, svi->capacity);
+		v->cargo_cap = GetVehicleCapacity(v);
 
 		v->InvalidateNewGRFCacheOfChain();
 
@@ -912,7 +912,6 @@ CommandCost CmdRefitShip(TileIndex tile, DoCommandFlag flags, uint32 p1, uint32
 	CommandCost cost(EXPENSES_SHIP_RUN);
 	CargoID new_cid = GB(p2, 0, 8); // gets the cargo number
 	byte new_subtype = GB(p2, 8, 8);
-	uint16 capacity = CALLBACK_FAILED;
 
 	Ship *v = Ship::GetIfValid(p1);
 
@@ -920,29 +919,23 @@ CommandCost CmdRefitShip(TileIndex tile, DoCommandFlag flags, uint32 p1, uint32
 	if (!v->IsStoppedInDepot()) return_cmd_error(STR_ERROR_SHIP_MUST_BE_STOPPED_IN_DEPOT);
 	if (v->vehstatus & VS_CRASHED) return_cmd_error(STR_ERROR_CAN_T_REFIT_DESTROYED_VEHICLE);
 
-	const Engine *e = Engine::Get(v->engine_type);
-
 	/* Check cargo */
 	if (new_cid >= NUM_CARGO || !CanRefitTo(v->engine_type, new_cid)) return CMD_ERROR;
 
-	/* Check the refit capacity callback */
-	if (HasBit(e->info.callback_mask, CBM_VEHICLE_REFIT_CAPACITY)) {
-		/* Back up the existing cargo type */
-		CargoID temp_cid = v->cargo_type;
-		byte temp_subtype = v->cargo_subtype;
-		v->cargo_type = new_cid;
-		v->cargo_subtype = new_subtype;
+	v->InvalidateNewGRFCacheOfChain();
 
-		capacity = GetVehicleCallback(CBID_VEHICLE_REFIT_CAPACITY, 0, 0, v->engine_type, v);
+	/* Back up the existing cargo type */
+	CargoID temp_cid = v->cargo_type;
+	byte temp_subtype = v->cargo_subtype;
+	v->cargo_type = new_cid;
+	v->cargo_subtype = new_subtype;
 
-		/* Restore the cargo type */
-		v->cargo_type = temp_cid;
-		v->cargo_subtype = temp_subtype;
-	}
+	uint capacity = GetVehicleCapacity(v);
+
+	/* Restore the cargo type */
+	v->cargo_type = temp_cid;
+	v->cargo_subtype = temp_subtype;
 
-	if (capacity == CALLBACK_FAILED) {
-		capacity = GetVehicleProperty(v, PROP_SHIP_CARGO_CAPACITY, e->u.ship.capacity);
-	}
 	_returned_refit_capacity = capacity;
 
 	if (new_cid != v->cargo_type) {
diff --git a/src/train_cmd.cpp b/src/train_cmd.cpp
index e0e32efd5..be4c66f3e 100644
--- a/src/train_cmd.cpp
+++ b/src/train_cmd.cpp
@@ -316,10 +316,7 @@ void TrainConsistChanged(Train *v, bool same_length)
 			}
 		}
 
-		if (e_u->CanCarryCargo() && u->cargo_type == e_u->GetDefaultCargoType() && u->cargo_subtype == 0) {
-			/* Set cargo capacity if we've not been refitted */
-			u->cargo_cap = GetVehicleProperty(u, PROP_TRAIN_CARGO_CAPACITY, rvi_u->capacity);
-		}
+		u->cargo_cap = GetVehicleCapacity(u);
 
 		/* check the vehicle length (callback) */
 		uint16 veh_len = CALLBACK_FAILED;
@@ -2119,6 +2116,7 @@ CommandCost CmdRefitRailVehicle(TileIndex tile, DoCommandFlag flags, uint32 p1,
 	CommandCost cost(EXPENSES_TRAIN_RUN);
 	uint num = 0;
 
+	v->InvalidateNewGRFCacheOfChain();
 	do {
 		/* XXX: We also refit all the attached wagons en-masse if they
 		 * can be refitted. This is how TTDPatch does it.  TODO: Have
@@ -2128,40 +2126,17 @@ CommandCost CmdRefitRailVehicle(TileIndex tile, DoCommandFlag flags, uint32 p1,
 		const Engine *e = Engine::Get(v->engine_type);
 		if (!e->CanCarryCargo()) continue;
 
-		uint16 amount = CALLBACK_FAILED;
-		if (HasBit(e->info.callback_mask, CBM_VEHICLE_REFIT_CAPACITY)) {
-			/* Back up the vehicle's cargo type */
-			CargoID temp_cid = v->cargo_type;
-			byte temp_subtype = v->cargo_subtype;
-			v->cargo_type = new_cid;
-			v->cargo_subtype = new_subtype;
-			/* Check the refit capacity callback */
-			amount = GetVehicleCallback(CBID_VEHICLE_REFIT_CAPACITY, 0, 0, v->engine_type, v);
-			/* Restore the original cargo type */
-			v->cargo_type = temp_cid;
-			v->cargo_subtype = temp_subtype;
-		}
+		/* Back up the vehicle's cargo type */
+		CargoID temp_cid = v->cargo_type;
+		byte temp_subtype = v->cargo_subtype;
+		v->cargo_type = new_cid;
+		v->cargo_subtype = new_subtype;
 
-		if (amount == CALLBACK_FAILED) { // callback failed or not used, use default
-			CargoID old_cid = e->GetDefaultCargoType();
-			/* normally, the capacity depends on the cargo type, a rail vehicle can
-			 * carry twice as much mail/goods as normal cargo, and four times as
-			 * many passengers
-			 */
-			amount = e->u.rail.capacity;
-			switch (old_cid) {
-				case CT_PASSENGERS: break;
-				case CT_MAIL:
-				case CT_GOODS: amount *= 2; break;
-				default:       amount *= 4; break;
-			}
-			switch (new_cid) {
-				case CT_PASSENGERS: break;
-				case CT_MAIL:
-				case CT_GOODS: amount /= 2; break;
-				default:       amount /= 4; break;
-			}
-		}
+		uint amount = GetVehicleCapacity(v);
+
+		/* Restore the original cargo type */
+		v->cargo_type = temp_cid;
+		v->cargo_subtype = temp_subtype;
 
 		if (new_cid != v->cargo_type) {
 			cost.AddCost(GetRefitCost(v->engine_type));
diff --git a/src/vehicle.cpp b/src/vehicle.cpp
index c1e99f85b..2fbc9a6ac 100644
--- a/src/vehicle.cpp
+++ b/src/vehicle.cpp
@@ -1418,6 +1418,65 @@ SpriteID GetVehiclePalette(const Vehicle *v)
 	return GetEngineColourMap(v->engine_type, v->owner, INVALID_ENGINE, v);
 }
 
+/**
+ * Determines capacity of a given vehicle from scratch.
+ * For aircraft the main capacity is determined. Mail might be present as well.
+ * @note Keep this function consistent with Engine::GetDisplayDefaultCapacity().
+ * @param v Vehicle of interest
+ * @return Capacity
+ */
+uint GetVehicleCapacity(const Vehicle *v)
+{
+	const Engine *e = Engine::Get(v->engine_type);
+
+	if (!e->CanCarryCargo()) return 0;
+
+	CargoID default_cargo = e->GetDefaultCargoType();
+
+	/* Check the refit capacity callback if we are not in the default configuration.
+	 * Note: This might change to become more consistent/flexible/sane, esp. when default cargo is first refittable. */
+	if (HasBit(e->info.callback_mask, CBM_VEHICLE_REFIT_CAPACITY) &&
+			(default_cargo != v->cargo_type || v->cargo_subtype != 0)) {
+		uint16 callback = GetVehicleCallback(CBID_VEHICLE_REFIT_CAPACITY, 0, 0, v->engine_type, v);
+		if (callback != CALLBACK_FAILED) return callback;
+	}
+
+	/* Get capacity according to property resp. CB */
+	uint capacity;
+	switch (e->type) {
+		case VEH_TRAIN:    capacity = GetVehicleProperty(v, PROP_TRAIN_CARGO_CAPACITY,   e->u.rail.capacity); break;
+		case VEH_ROAD:     capacity = GetVehicleProperty(v, PROP_ROADVEH_CARGO_CAPACITY, e->u.road.capacity); break;
+		case VEH_SHIP:     capacity = GetVehicleProperty(v, PROP_SHIP_CARGO_CAPACITY,    e->u.ship.capacity); break;
+		case VEH_AIRCRAFT: capacity = e->u.air.passenger_capacity; break;
+		default: NOT_REACHED();
+	}
+
+	/* Apply multipliers depending on cargo- and vehicletype.
+	 * Note: This might change to become more consistent/flexible. */
+	if (e->type != VEH_SHIP) {
+		if (e->type == VEH_AIRCRAFT) {
+			if (v->cargo_type == CT_PASSENGERS) return capacity;
+			capacity += e->u.air.mail_capacity;
+			if (v->cargo_type == CT_MAIL) return capacity;
+		} else {
+			switch (default_cargo) {
+				case CT_PASSENGERS: break;
+				case CT_MAIL:
+				case CT_GOODS: capacity *= 2; break;
+				default:       capacity *= 4; break;
+			}
+		}
+		switch (v->cargo_type) {
+			case CT_PASSENGERS: break;
+			case CT_MAIL:
+			case CT_GOODS: capacity /= 2; break;
+			default:       capacity /= 4; break;
+		}
+	}
+
+	return capacity;
+}
+
 
 void Vehicle::BeginLoading()
 {
diff --git a/src/vehicle_func.h b/src/vehicle_func.h
index b2d4df70c..718feb426 100644
--- a/src/vehicle_func.h
+++ b/src/vehicle_func.h
@@ -112,6 +112,8 @@ SpriteID GetEnginePalette(EngineID engine_type, CompanyID company);
  */
 SpriteID GetVehiclePalette(const Vehicle *v);
 
+uint GetVehicleCapacity(const Vehicle *v);
+
 extern const uint32 _veh_build_proc_table[];
 extern const uint32 _veh_sell_proc_table[];
 extern const uint32 _veh_refit_proc_table[];