cpp/openttd-patchpack/source Changeset - r28534:0ad94de38e5b

Changeset - r28534:0ad94de38e5b

Parent rev.

Child rev.

[Not reviewed]

master

0 1 0

Peter Nelson - 3 months ago 2024-01-21 09:21:22
peter1138@openttd.org

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.

1 file changed with 47 insertions and 44 deletions:

src/autoreplace_cmd.cpp

0 comments (0 inline, 0 general)

src/autoreplace_cmd.cpp

➞

Show inline comments

@@ @@ -490,6 +490,21 @@ static CommandCost ReplaceFreeUnit(Vehic @@
 	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 @@
 		/* 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
 		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
 		std::vector<ReplaceChainItem> replacements;
 		/* 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;
 		Train *w;
 		for (w = Train::From(old_head), i = 0; w != nullptr; w = w->GetNextUnit(), i++) {
 			assert(i < num_units);
 			old_vehs[i] = w;
 		for (Train *w = Train::From(old_head); w != nullptr; w = w->GetNextUnit()) {
 			ReplaceChainItem &replacement = replacements.emplace_back(w, nullptr, 0);
-			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();
 			if (new_vehs[i] != nullptr) *nothing_to_do = false;
 			replacement.cost = ret.GetCost();
 			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 @@
 			 * 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--) {
 					Train *append = (new_vehs[i] != nullptr ? new_vehs[i] : old_vehs[i]);
 				for (auto it = std::rbegin(replacements); it != std::rend(replacements); ++it) {
 					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 @@
+			}
 			/* 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 @@
 						/* 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 @@
 					} 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);
 				for (int i = 1; i < num_units; i++) {
 					Vehicle *wagon = new_vehs[i];
 				assert(Train::From(new_head)->gcache.cached_total_length <= _settings_game.vehicle.max_train_length * TILE_SIZE);
 				for (auto it = std::next(std::begin(replacements)); it != std::end(replacements); ++it) {
 					Vehicle *wagon = it->new_veh;
 					if (wagon == nullptr) continue;
 					if (wagon->First() == new_head) break;
@@ @@ -612,11 +619,11 @@ static CommandCost ReplaceChain(Vehicle @@
 					/* 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 @@
+				}
 				/* Transfer cargo of old vehicles and sell them */
 				for (int i = 0; i < num_units; i++) {
 					Vehicle *w = old_vehs[i];
 				for (auto it = std::begin(replacements); it != std::end(replacements); ++it) {
 					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 @@
 					 *       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;
 						if (i == 0) old_head = nullptr;
 						it->old_veh = nullptr;
 						if (it == std::begin(replacements)) old_head = nullptr;
+					}
+				}
@@ @@ -662,8 +669,8 @@ static CommandCost ReplaceChain(Vehicle @@
 				assert(Train::From(old_head)->GetNextUnit() == nullptr);
 				for (int i = num_units - 1; i > 0; i--) {
 					[[maybe_unused]] CommandCost ret = CmdMoveVehicle(old_vehs[i], old_head, DC_EXEC | DC_AUTOREPLACE, false);
 				for (auto it = std::rbegin(replacements); it != std::rend(replacements); ++it) {
 					[[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 @@
 		/* Finally undo buying of new vehicles */
 		if ((flags & DC_EXEC) == 0) {
 			for (int i = num_units - 1; i >= 0; i--) {
 				if (new_vehs[i] != nullptr) {
 					Command<CMD_SELL_VEHICLE>::Do(DC_EXEC, new_vehs[i]->index, false, false, INVALID_CLIENT_ID);
 					new_vehs[i] = nullptr;
 			for (auto it = std::rbegin(replacements); it != std::rend(replacements); ++it) {
 				if (it->new_veh != nullptr) {
 					Command<CMD_SELL_VEHICLE>::Do(DC_EXEC, it->new_veh->index, false, false, INVALID_CLIENT_ID);
 					it->new_veh = nullptr;
+				}
+			}
+		}
 		free(old_vehs);
 		free(new_vehs);
 		free(new_costs);
 	} else {
 		/* Build and refit replacement vehicle */
 		Vehicle *new_head = nullptr;

0 comments (0 inline, 0 general)