cpp/openttd-patchpack/source Changeset - r28743:23b9eb52e276

Changeset - r28743:23b9eb52e276

Parent rev.

Child rev.

[Not reviewed]

master

0 1 0

Peter Nelson - 3 months ago 2024-02-11 23:05:29
peter1138@openttd.org

Codefix: Incorrect storage type in cargo field of industry cargo chains window. (#12051)

`supp_cargoes` and `cust_cargoes` actually contains a column index, however this index is always stored at the indexed position...

Replace with a bitmask instead, which stores if the column indices are linked.

1 file changed with 23 insertions and 24 deletions:

src/industry_gui.cpp

0 comments (0 inline, 0 general)

src/industry_gui.cpp

➞

Show inline comments

@@ @@ -1978,25 +1978,28 @@ struct CargoesField { @@
 	static int small_height, normal_height;
 	static int cargo_field_width;
 	static int industry_width;
 	static uint max_cargoes;
 	using Cargoes = uint16_t;
 	static_assert(std::numeric_limits<Cargoes>::digits >= MAX_CARGOES);
 	CargoesFieldType type; ///< Type of field.
 	union {
 		struct {
 			IndustryType ind_type;                 ///< Industry type (#NUM_INDUSTRYTYPES means 'houses').
 			CargoID other_produced[MAX_CARGOES];   ///< Cargoes produced but not used in this figure.
 			CargoID other_accepted[MAX_CARGOES];   ///< Cargoes accepted but not used in this figure.
 		} industry; ///< Industry data (for #CFT_INDUSTRY).
 		struct {
 			CargoID vertical_cargoes[MAX_CARGOES]; ///< Cargoes running from top to bottom (cargo ID or #INVALID_CARGO).
 			Cargoes supp_cargoes; ///< Cargoes in \c vertical_cargoes entering from the left.
 			Cargoes cust_cargoes; ///< Cargoes in \c vertical_cargoes leaving to the right.
 			uint8_t num_cargoes;                   ///< Number of cargoes.
 			CargoID supp_cargoes[MAX_CARGOES];     ///< Cargoes entering from the left (index in #vertical_cargoes, or #INVALID_CARGO).
 			uint8_t top_end;                       ///< Stop at the top of the vertical cargoes.
 			CargoID cust_cargoes[MAX_CARGOES];     ///< Cargoes leaving to the right (index in #vertical_cargoes, or #INVALID_CARGO).
 			uint8_t bottom_end;                    ///< Stop at the bottom of the vertical cargoes.
 		} cargo; ///< Cargo data (for #CFT_CARGO).
 		struct {
 			CargoID cargoes[MAX_CARGOES];          ///< Cargoes to display (or #INVALID_CARGO).
 			bool left_align;                       ///< Align all cargo texts to the left (else align to the right).
 		} cargo_label;   ///< Label data (for #CFT_CARGO_LABEL).
@@ @@ -2044,34 +2047,30 @@ struct CargoesField { @@
 				break;
+			}
+		}
 		if (column < 0) return -1;
 		if (producer) {
 			assert(!IsValidCargoID(this->u.cargo.supp_cargoes[column]));
 			this->u.cargo.supp_cargoes[column]  = column;
 			assert(!HasBit(this->u.cargo.supp_cargoes, column));
 			SetBit(this->u.cargo.supp_cargoes, column);
 		} else {
 			assert(!IsValidCargoID(this->u.cargo.cust_cargoes[column]));
 			this->u.cargo.cust_cargoes[column] = column;
 			assert(!HasBit(this->u.cargo.cust_cargoes, column));
 			SetBit(this->u.cargo.cust_cargoes, column);
+		}
 		return column;
+	}
 	/**
 	 * Does this #CFT_CARGO field have a horizontal connection?
 	 * @return \c true if a horizontal connection exists, \c false otherwise.
 	 */
 	bool HasConnection()
+	{
 		assert(this->type == CFT_CARGO);
 		for (uint i = 0; i < MAX_CARGOES; i++) {
 			if (IsValidCargoID(this->u.cargo.supp_cargoes[i])) return true;
 			if (IsValidCargoID(this->u.cargo.cust_cargoes[i])) return true;
+		}
 		return false;
 		return this->u.cargo.supp_cargoes != 0 || this->u.cargo.cust_cargoes != 0;
+	}
 	/**
 	 * Make a piece of cargo column.
 	 * @param cargoes    Array of #CargoID (may contain #INVALID_CARGO).
 	 * @param length     Number of cargoes in \a cargoes.
@@ @@ -2093,14 +2092,14 @@ struct CargoesField { @@
 		this->u.cargo.num_cargoes = (count < 0) ? static_cast<uint8_t>(insert - std::begin(this->u.cargo.vertical_cargoes)) : count;
 		CargoIDComparator comparator;
 		std::sort(std::begin(this->u.cargo.vertical_cargoes), insert, comparator);
 		std::fill(insert, std::end(this->u.cargo.vertical_cargoes), INVALID_CARGO);
 		this->u.cargo.top_end = top_end;
 		this->u.cargo.bottom_end = bottom_end;
 		std::fill(std::begin(this->u.cargo.supp_cargoes), std::end(this->u.cargo.supp_cargoes), INVALID_CARGO);
 		std::fill(std::begin(this->u.cargo.cust_cargoes), std::end(this->u.cargo.cust_cargoes), INVALID_CARGO);
 		this->u.cargo.supp_cargoes = 0;
 		this->u.cargo.cust_cargoes = 0;
+	}
 	/**
 	 * Make a field displaying cargo type names.
 	 * @param cargoes    Array of #CargoID (may contain #INVALID_CARGO).
 	 * @param length     Number of cargoes in \a cargoes.
@@ @@ -2222,35 +2221,35 @@ struct CargoesField { @@
 					GfxFillRect(colpos, top, colpos + CargoesField::cargo_line.width - 2, bot, csp->legend_colour, FILLRECT_OPAQUE);
 					colpos += CargoesField::cargo_line.width - 2;
 					GfxDrawLine(colpos, top, colpos, bot, CARGO_LINE_COLOUR);
 					colpos += 1 + CargoesField::cargo_space.width;
+				}
-				const CargoID *hor_left, *hor_right;
+				Cargoes hor_left, hor_right;
 				if (_current_text_dir == TD_RTL) {
 					hor_left  = this->u.cargo.cust_cargoes;
 					hor_right = this->u.cargo.supp_cargoes;
 				} else {
 					hor_left  = this->u.cargo.supp_cargoes;
 					hor_right = this->u.cargo.cust_cargoes;
+				}
 				ypos += CargoesField::cargo_border.height + vert_inter_industry_space / 2 + (GetCharacterHeight(FS_NORMAL) - CargoesField::cargo_line.height) / 2;
 				for (uint i = 0; i < MAX_CARGOES; i++) {
 					if (IsValidCargoID(hor_left[i])) {
 						int col = hor_left[i];
 					if (HasBit(hor_left, i)) {
 						int col = i;
 						int dx = 0;
 						const CargoSpec *csp = CargoSpec::Get(this->u.cargo.vertical_cargoes[col]);
 						for (; col > 0; col--) {
 							int lf = cargo_base + col * CargoesField::cargo_line.width + (col - 1) * CargoesField::cargo_space.width;
 							DrawHorConnection(lf, lf + CargoesField::cargo_space.width - dx, ypos, csp);
 							dx = 1;
+						}
 						DrawHorConnection(xpos, cargo_base - dx, ypos, csp);
+					}
 					if (IsValidCargoID(hor_right[i])) {
 						int col = hor_right[i];
 					if (HasBit(hor_right, i)) {
 						int col = i;
 						int dx = 0;
 						const CargoSpec *csp = CargoSpec::Get(this->u.cargo.vertical_cargoes[col]);
 						for (; col < this->u.cargo.num_cargoes - 1; col++) {
 							int lf = cargo_base + (col + 1) * CargoesField::cargo_line.width + col * CargoesField::cargo_space.width;
 							DrawHorConnection(lf + dx - 1, lf + CargoesField::cargo_space.width - 1, ypos, csp);
 							dx = 1;
@@ @@ -2308,37 +2307,37 @@ struct CargoesField { @@
 			vpos += GetCharacterHeight(FS_NORMAL) + CargoesField::cargo_space.width;
+		}
 		if (row == MAX_CARGOES) return INVALID_CARGO;
 		/* row = 0 -> at first horizontal row, row = 1 -> second horizontal row, 2 = 3rd horizontal row. */
 		if (col == 0) {
-			if (IsValidCargoID(this->u.cargo.supp_cargoes[row])) return this->u.cargo.vertical_cargoes[this->u.cargo.supp_cargoes[row]];
+			if (HasBit(this->u.cargo.supp_cargoes, row)) return this->u.cargo.vertical_cargoes[row];
 			if (left != nullptr) {
 				if (left->type == CFT_INDUSTRY) return left->u.industry.other_produced[row];
 				if (left->type == CFT_CARGO_LABEL && !left->u.cargo_label.left_align) return left->u.cargo_label.cargoes[row];
+			}
 			return INVALID_CARGO;
+		}
 		if (col == this->u.cargo.num_cargoes) {
-			if (IsValidCargoID(this->u.cargo.cust_cargoes[row])) return this->u.cargo.vertical_cargoes[this->u.cargo.cust_cargoes[row]];
+			if (HasBit(this->u.cargo.cust_cargoes, row)) return this->u.cargo.vertical_cargoes[row];
 			if (right != nullptr) {
 				if (right->type == CFT_INDUSTRY) return right->u.industry.other_accepted[row];
 				if (right->type == CFT_CARGO_LABEL && right->u.cargo_label.left_align) return right->u.cargo_label.cargoes[row];
+			}
 			return INVALID_CARGO;
+		}
 		if (row >= col) {
 			/* Clicked somewhere in-between vertical cargo connection.
 			 * Since the horizontal connection is made in the same order as the vertical list, the above condition
 			 * ensures we are left-below the main diagonal, thus at the supplying side.
 			 */
-			if (IsValidCargoID(this->u.cargo.supp_cargoes[row])) return this->u.cargo.vertical_cargoes[this->u.cargo.supp_cargoes[row]];
+			if (HasBit(this->u.cargo.supp_cargoes, row)) return this->u.cargo.vertical_cargoes[row];
 			return INVALID_CARGO;
+		}
 		/* Clicked at a customer connection. */
-		if (IsValidCargoID(this->u.cargo.cust_cargoes[row])) return this->u.cargo.vertical_cargoes[this->u.cargo.cust_cargoes[row]];
+		if (HasBit(this->u.cargo.cust_cargoes, row)) return this->u.cargo.vertical_cargoes[row];
 		return INVALID_CARGO;
+	}
 	/**
 	 * Decide what cargo the user clicked in the cargo label field.
 	 * @param pt Click position in the cargo label field.
@@ @@ -2422,13 +2421,13 @@ struct CargoesRow { @@
 				int col = cargo_fld->ConnectCargo(indsp->produced_cargo[i], true);
 				if (col < 0) others[other_count++] = indsp->produced_cargo[i];
+			}
 			/* Allocate other cargoes in the empty holes of the horizontal cargo connections. */
 			for (uint i = 0; i < CargoesField::max_cargoes && other_count > 0; i++) {
-				if (!IsValidCargoID(cargo_fld->u.cargo.supp_cargoes[i])) ind_fld->u.industry.other_produced[i] = others[--other_count];
+				if (HasBit(cargo_fld->u.cargo.supp_cargoes, i)) ind_fld->u.industry.other_produced[i] = others[--other_count];
+			}
 		} else {
 			/* Houses only display cargo that towns produce. */
 			for (uint i = 0; i < cargo_fld->u.cargo.num_cargoes; i++) {
 				CargoID cid = cargo_fld->u.cargo.vertical_cargoes[i];
 				TownProductionEffect tpe = CargoSpec::Get(cid)->town_production_effect;
@@ @@ -2481,13 +2480,13 @@ struct CargoesRow { @@
 				int col = cargo_fld->ConnectCargo(indsp->accepts_cargo[i], false);
 				if (col < 0) others[other_count++] = indsp->accepts_cargo[i];
+			}
 			/* Allocate other cargoes in the empty holes of the horizontal cargo connections. */
 			for (uint i = 0; i < CargoesField::max_cargoes && other_count > 0; i++) {
-				if (!IsValidCargoID(cargo_fld->u.cargo.cust_cargoes[i])) ind_fld->u.industry.other_accepted[i] = others[--other_count];
+				if (!HasBit(cargo_fld->u.cargo.cust_cargoes, i)) ind_fld->u.industry.other_accepted[i] = others[--other_count];
+			}
 		} else {
 			/* Houses only display what is demanded. */
 			for (uint i = 0; i < cargo_fld->u.cargo.num_cargoes; i++) {
 				for (uint h = 0; h < NUM_HOUSES; h++) {
 					HouseSpec *hs = HouseSpec::Get(h);

0 comments (0 inline, 0 general)