Codechange: Use vector and iterators to store old/new vehicles during autoreplace. (#11851)

This avoids malloc/free of 3 arrays along index counting, and the data for each part is kept together.

src/autoreplace_cmd.cpp

@@ -490,6 +490,21 @@ static CommandCost ReplaceFreeUnit(Vehicle **single_unit, DoCommandFlag flags, b
 	return cost;
 }
 
+/** Struct for recording vehicle chain replacement information. */
+struct ReplaceChainItem {
+	Vehicle *old_veh; ///< Old vehicle to replace.
+	Vehicle *new_veh; ///< Replacement vehicle, or nullptr if no replacement.
+	Money cost; /// Cost of buying and refitting replacement.
+
+	ReplaceChainItem(Vehicle *old_veh, Vehicle *new_veh, Money cost) : old_veh(old_veh), new_veh(new_veh), cost(cost) { }
+
+	/**
+	 * Get vehicle to use for this position.
+	 * @return Either the new vehicle, or the old vehicle if there is no replacement.
+	 */
+	Vehicle *GetVehicle() const { return new_veh == nullptr ? old_veh : new_veh; }
+};
+
 /**
  * Replace a whole vehicle chain
  * @param chain vehicle chain to let autoreplace/renew operator on
@@ -509,29 +524,21 @@ static CommandCost ReplaceChain(Vehicle **chain, DoCommandFlag flags, bool wagon
 		/* Store the length of the old vehicle chain, rounded up to whole tiles */
 		uint16_t old_total_length = CeilDiv(Train::From(old_head)->gcache.cached_total_length, TILE_SIZE) * TILE_SIZE;
 
-		int num_units = 0; ///< Number of units in the chain
+		std::vector<ReplaceChainItem> replacements;
-		for (Train *w = Train::From(old_head); w != nullptr; w = w->GetNextUnit()) num_units++;
-
-		Train **old_vehs = CallocT<Train *>(num_units); ///< Will store vehicles of the old chain in their order
-		Train **new_vehs = CallocT<Train *>(num_units); ///< New vehicles corresponding to old_vehs or nullptr if no replacement
-		Money *new_costs = MallocT<Money>(num_units);   ///< Costs for buying and refitting the new vehicles
 
 		/* Collect vehicles and build replacements
 		 * Note: The replacement vehicles can only successfully build as long as the old vehicles are still in their chain */
-		int i;
+		for (Train *w = Train::From(old_head); w != nullptr; w = w->GetNextUnit()) {
-		Train *w;
+			ReplaceChainItem &replacement = replacements.emplace_back(w, nullptr, 0);
-		for (w = Train::From(old_head), i = 0; w != nullptr; w = w->GetNextUnit(), i++) {
-			assert(i < num_units);
-			old_vehs[i] = w;
 
-			CommandCost ret = BuildReplacementVehicle(old_vehs[i], (Vehicle**)&new_vehs[i], true);
+			CommandCost ret = BuildReplacementVehicle(replacement.old_veh, &replacement.new_veh, true);
 			cost.AddCost(ret);
 			if (cost.Failed()) break;
 
-			new_costs[i] = ret.GetCost();
+			replacement.cost = ret.GetCost();
-			if (new_vehs[i] != nullptr) *nothing_to_do = false;
+			if (replacement.new_veh != nullptr) *nothing_to_do = false;
 		}
-		Train *new_head = (new_vehs[0] != nullptr ? new_vehs[0] : old_vehs[0]);
+		Vehicle *new_head = replacements.front().GetVehicle();
 
 		/* Note: When autoreplace has already failed here, old_vehs[] is not completely initialized. But it is also not needed. */
 		if (cost.Succeeded()) {
@@ -545,18 +552,18 @@ static CommandCost ReplaceChain(Vehicle **chain, DoCommandFlag flags, bool wagon
 			 * We do this from back to front, so that the head of the temporary vehicle chain does not change all the time.
 			 * That way we also have less trouble when exceeding the unitnumber limit.
 			 * OTOH the vehicle attach callback is more expensive this way :s */
-			Train *last_engine = nullptr; ///< Shall store the last engine unit after this step
+			Vehicle *last_engine = nullptr; ///< Shall store the last engine unit after this step
 			if (cost.Succeeded()) {
-				for (int i = num_units - 1; i > 0; i--) {
+				for (auto it = std::rbegin(replacements); it != std::rend(replacements); ++it) {
-					Train *append = (new_vehs[i] != nullptr ? new_vehs[i] : old_vehs[i]);
+					Vehicle *append = it->GetVehicle();
 
 					if (RailVehInfo(append->engine_type)->railveh_type == RAILVEH_WAGON) continue;
 
-					if (new_vehs[i] != nullptr) {
+					if (it->new_veh != nullptr) {
 						/* Move the old engine to a separate row with DC_AUTOREPLACE. Else
 						 * moving the wagon in front may fail later due to unitnumber limit.
 						 * (We have to attach wagons without DC_AUTOREPLACE.) */
-						CmdMoveVehicle(old_vehs[i], nullptr, DC_EXEC | DC_AUTOREPLACE, false);
+						CmdMoveVehicle(it->old_veh, nullptr, DC_EXEC | DC_AUTOREPLACE, false);
 					}
 
 					if (last_engine == nullptr) last_engine = append;
@@ -567,15 +574,15 @@ static CommandCost ReplaceChain(Vehicle **chain, DoCommandFlag flags, bool wagon
 			}
 
 			/* When wagon removal is enabled and the new engines without any wagons are already longer than the old, we have to fail */
-			if (cost.Succeeded() && wagon_removal && new_head->gcache.cached_total_length > old_total_length) cost = CommandCost(STR_ERROR_TRAIN_TOO_LONG_AFTER_REPLACEMENT);
+			if (cost.Succeeded() && wagon_removal && Train::From(new_head)->gcache.cached_total_length > old_total_length) cost = CommandCost(STR_ERROR_TRAIN_TOO_LONG_AFTER_REPLACEMENT);
 
 			/* Append/insert wagons into the new vehicle chain
 			 * We do this from back to front, so we can stop when wagon removal or maximum train length (i.e. from mammoth-train setting) is triggered.
 			 */
 			if (cost.Succeeded()) {
-				for (int i = num_units - 1; i > 0; i--) {
+				for (auto it = std::rbegin(replacements); it != std::rend(replacements); ++it) {
 					assert(last_engine != nullptr);
-					Vehicle *append = (new_vehs[i] != nullptr ? new_vehs[i] : old_vehs[i]);
+					Vehicle *append = it->GetVehicle();
 
 					if (RailVehInfo(append->engine_type)->railveh_type == RAILVEH_WAGON) {
 						/* Insert wagon after 'last_engine' */
@@ -584,7 +591,7 @@ static CommandCost ReplaceChain(Vehicle **chain, DoCommandFlag flags, bool wagon
 						/* When we allow removal of wagons, either the move failing due
 						 * to the train becoming too long, or the train becoming longer
 						 * would move the vehicle to the empty vehicle chain. */
-						if (wagon_removal && (res.Failed() ? res.GetErrorMessage() == STR_ERROR_TRAIN_TOO_LONG : new_head->gcache.cached_total_length > old_total_length)) {
+						if (wagon_removal && (res.Failed() ? res.GetErrorMessage() == STR_ERROR_TRAIN_TOO_LONG : Train::From(new_head)->gcache.cached_total_length > old_total_length)) {
 							CmdMoveVehicle(append, nullptr, DC_EXEC | DC_AUTOREPLACE, false);
 							break;
 						}
@@ -594,16 +601,16 @@ static CommandCost ReplaceChain(Vehicle **chain, DoCommandFlag flags, bool wagon
 					} else {
 						/* We have reached 'last_engine', continue with the next engine towards the front */
 						assert(append == last_engine);
-						last_engine = last_engine->GetPrevUnit();
+						last_engine = Train::From(last_engine)->GetPrevUnit();
 					}
 				}
 			}
 
 			/* Sell superfluous new vehicles that could not be inserted. */
 			if (cost.Succeeded() && wagon_removal) {
-				assert(new_head->gcache.cached_total_length <= _settings_game.vehicle.max_train_length * TILE_SIZE);
+				assert(Train::From(new_head)->gcache.cached_total_length <= _settings_game.vehicle.max_train_length * TILE_SIZE);
-				for (int i = 1; i < num_units; i++) {
+				for (auto it = std::next(std::begin(replacements)); it != std::end(replacements); ++it) {
-					Vehicle *wagon = new_vehs[i];
+					Vehicle *wagon = it->new_veh;
 					if (wagon == nullptr) continue;
 					if (wagon->First() == new_head) break;
 
@@ -612,11 +619,11 @@ static CommandCost ReplaceChain(Vehicle **chain, DoCommandFlag flags, bool wagon
 					/* Sell wagon */
 					[[maybe_unused]] CommandCost ret = Command<CMD_SELL_VEHICLE>::Do(DC_EXEC, wagon->index, false, false, INVALID_CLIENT_ID);
 					assert(ret.Succeeded());
-					new_vehs[i] = nullptr;
+					it->new_veh = nullptr;
 
 					/* Revert the money subtraction when the vehicle was built.
 					 * This value is different from the sell value, esp. because of refitting */
-					cost.AddCost(-new_costs[i]);
+					cost.AddCost(-it->cost);
 				}
 			}
 
@@ -631,8 +638,8 @@ static CommandCost ReplaceChain(Vehicle **chain, DoCommandFlag flags, bool wagon
 				}
 
 				/* Transfer cargo of old vehicles and sell them */
-				for (int i = 0; i < num_units; i++) {
+				for (auto it = std::begin(replacements); it != std::end(replacements); ++it) {
-					Vehicle *w = old_vehs[i];
+					Vehicle *w = it->old_veh;
 					/* Is the vehicle again part of the new chain?
 					 * Note: We cannot test 'new_vehs[i] != nullptr' as wagon removal might cause to remove both */
 					if (w->First() == new_head) continue;
@@ -644,8 +651,8 @@ static CommandCost ReplaceChain(Vehicle **chain, DoCommandFlag flags, bool wagon
 					 *       it from failing due to engine limits. */
 					cost.AddCost(Command<CMD_SELL_VEHICLE>::Do(flags | DC_AUTOREPLACE, w->index, false, false, INVALID_CLIENT_ID));
 					if ((flags & DC_EXEC) != 0) {
-						old_vehs[i] = nullptr;
+						it->old_veh = nullptr;
-						if (i == 0) old_head = nullptr;
+						if (it == std::begin(replacements)) old_head = nullptr;
 					}
 				}
 
@@ -662,8 +669,8 @@ static CommandCost ReplaceChain(Vehicle **chain, DoCommandFlag flags, bool wagon
 
 				assert(Train::From(old_head)->GetNextUnit() == nullptr);
 
-				for (int i = num_units - 1; i > 0; i--) {
+				for (auto it = std::rbegin(replacements); it != std::rend(replacements); ++it) {
-					[[maybe_unused]] CommandCost ret = CmdMoveVehicle(old_vehs[i], old_head, DC_EXEC | DC_AUTOREPLACE, false);
+					[[maybe_unused]] CommandCost ret = CmdMoveVehicle(it->old_veh, old_head, DC_EXEC | DC_AUTOREPLACE, false);
 					assert(ret.Succeeded());
 				}
 			}
@@ -671,17 +678,13 @@ static CommandCost ReplaceChain(Vehicle **chain, DoCommandFlag flags, bool wagon
 
 		/* Finally undo buying of new vehicles */
 		if ((flags & DC_EXEC) == 0) {
-			for (int i = num_units - 1; i >= 0; i--) {
+			for (auto it = std::rbegin(replacements); it != std::rend(replacements); ++it) {
-				if (new_vehs[i] != nullptr) {
+				if (it->new_veh != nullptr) {
-					Command<CMD_SELL_VEHICLE>::Do(DC_EXEC, new_vehs[i]->index, false, false, INVALID_CLIENT_ID);
+					Command<CMD_SELL_VEHICLE>::Do(DC_EXEC, it->new_veh->index, false, false, INVALID_CLIENT_ID);
-					new_vehs[i] = nullptr;
+					it->new_veh = nullptr;
 				}
 			}
 		}
-
-		free(old_vehs);
-		free(new_vehs);
-		free(new_costs);
 	} else {
 		/* Build and refit replacement vehicle */
 		Vehicle *new_head = nullptr;