Codechange: Add and use GetScrolledItemFromWidget to get a list item.

This function returns an iterator, either to the selected item or the
container's end.

This makes handling the result more robust as indices are not used.

src/autoreplace_gui.cpp

@@ -617,12 +617,11 @@ public:
 				} else {
 					click_side = 1;
 				}
-				uint i = this->vscroll[click_side]->GetScrolledRowFromWidget(pt.y, this, widget);
-				size_t engine_count = this->engines[click_side].size();
 
 				EngineID e = INVALID_ENGINE;
-				if (i < engine_count) {
+				const auto it = this->vscroll[click_side]->GetScrolledItemFromWidget(this->engines[click_side], pt.y, this, widget);
-					const auto &item = this->engines[click_side][i];
+				if (it != this->engines[click_side].end()) {
+					const auto &item = *it;
 					const Rect r = this->GetWidget<NWidgetBase>(widget)->GetCurrentRect().Shrink(WidgetDimensions::scaled.matrix).WithWidth(WidgetDimensions::scaled.hsep_indent * (item.indent + 1), _current_text_dir == TD_RTL);
 					if ((item.flags & EngineDisplayFlags::HasVariants) != EngineDisplayFlags::None && IsInsideMM(r.left, r.right, pt.x)) {
 						/* toggle folded flag on engine */

src/bridge_gui.cpp

@@ -264,9 +264,9 @@ public:
 		switch (widget) {
 			default: break;
 			case WID_BBS_BRIDGE_LIST: {
-				uint i = this->vscroll->GetScrolledRowFromWidget(pt.y, this, WID_BBS_BRIDGE_LIST);
+				auto it = this->vscroll->GetScrolledItemFromWidget(*this->bridges, pt.y, this, WID_BBS_BRIDGE_LIST);
-				if (i < this->bridges->size()) {
+				if (it != this->bridges->end()) {
-					this->BuildBridge(i);
+					this->BuildBridge(it - this->bridges->begin());
 					this->Close();
 				}
 				break;

src/build_vehicle_gui.cpp

@@ -1588,11 +1588,10 @@ struct BuildVehicleWindow : Window {
 				break;
 
 			case WID_BV_LIST: {
-				uint i = this->vscroll->GetScrolledRowFromWidget(pt.y, this, WID_BV_LIST);
-				size_t num_items = this->eng_list.size();
 				EngineID e = INVALID_ENGINE;
-				if (i < num_items) {
+				const auto it = this->vscroll->GetScrolledItemFromWidget(this->eng_list, pt.y, this, WID_BV_LIST);
-					const auto &item = this->eng_list[i];
+				if (it != this->eng_list.end()) {
+					const auto &item = *it;
 					const Rect r = this->GetWidget<NWidgetBase>(widget)->GetCurrentRect().Shrink(WidgetDimensions::scaled.matrix).WithWidth(WidgetDimensions::scaled.hsep_indent * (item.indent + 1), _current_text_dir == TD_RTL);
 					if ((item.flags & EngineDisplayFlags::HasVariants) != EngineDisplayFlags::None && IsInsideMM(r.left, r.right, pt.x)) {
 						/* toggle folded flag on engine */

src/graph_gui.cpp

@@ -999,13 +999,9 @@ struct PaymentRatesGraphWindow : BaseGraphWindow {
 			}
 
 			case WID_CPR_MATRIX: {
-				uint row = this->vscroll->GetScrolledRowFromWidget(pt.y, this, WID_CPR_MATRIX);
+				auto it = this->vscroll->GetScrolledItemFromWidget(_sorted_standard_cargo_specs, pt.y, this, WID_CPR_MATRIX);
-				if (row >= this->vscroll->GetCount()) return;
+				if (it != _sorted_standard_cargo_specs.end()) {
-
+					ToggleBit(_legend_excluded_cargo, (*it)->Index());
-				for (const CargoSpec *cs : _sorted_standard_cargo_specs) {
-					if (row-- > 0) continue;
-
-					ToggleBit(_legend_excluded_cargo, cs->Index());
 					this->UpdateExcludedData();
 					this->SetDirty();
 					break;

src/group_gui.cpp

@@ -691,10 +691,11 @@ public:
 				break;
 
 			case WID_GL_LIST_GROUP: { // Matrix Group
-				uint id_g = this->group_sb->GetScrolledRowFromWidget(pt.y, this, WID_GL_LIST_GROUP);
+				auto it = this->group_sb->GetScrolledItemFromWidget(this->groups, pt.y, this, WID_GL_LIST_GROUP);
-				if (id_g >= this->groups.size()) return;
+				if (it == this->groups.end()) return;
 
-				if (groups[id_g]->folded || (id_g + 1 < this->groups.size() && this->indents[id_g + 1] > this->indents[id_g])) {
+				size_t id_g = it - this->groups.begin();
+				if ((*it)->folded || (id_g + 1 < this->groups.size() && this->indents[id_g + 1] > this->indents[id_g])) {
 					/* The group has children, check if the user clicked the fold / unfold button. */
 					NWidgetCore *group_display = this->GetWidget<NWidgetCore>(widget);
 					int x = _current_text_dir == TD_RTL ?
@@ -731,10 +732,10 @@ public:
 			}
 
 			case WID_GL_LIST_VEHICLE: { // Matrix Vehicle
-				uint id_v = this->vscroll->GetScrolledRowFromWidget(pt.y, this, WID_GL_LIST_VEHICLE);
+				auto it = this->vscroll->GetScrolledItemFromWidget(this->vehgroups, pt.y, this, WID_GL_LIST_VEHICLE);
-				if (id_v >= this->vehgroups.size()) return; // click out of list bound
+				if (it == this->vehgroups.end()) return; // click out of list bound
 
-				const GUIVehicleGroup &vehgroup = this->vehgroups[id_v];
+				const GUIVehicleGroup &vehgroup = *it;
 
 				const Vehicle *v = nullptr;
 
@@ -845,8 +846,8 @@ public:
 				break;
 
 			case WID_GL_LIST_GROUP: { // Matrix group
-				uint id_g = this->group_sb->GetScrolledRowFromWidget(pt.y, this, WID_GL_LIST_GROUP);
+				auto it = this->group_sb->GetScrolledItemFromWidget(this->groups, pt.y, this, WID_GL_LIST_GROUP);
-				GroupID new_g = id_g >= this->groups.size() ? INVALID_GROUP : this->groups[id_g]->index;
+				GroupID new_g = it == this->groups.end() ? INVALID_GROUP : (*it)->index;
 
 				if (this->group_sel != new_g && g->parent != new_g) {
 					Command<CMD_ALTER_GROUP>::Post(STR_ERROR_GROUP_CAN_T_SET_PARENT, AlterGroupMode::SetParent, this->group_sel, new_g, {});
@@ -878,8 +879,8 @@ public:
 				this->group_over = INVALID_GROUP;
 				this->SetDirty();
 
-				uint id_g = this->group_sb->GetScrolledRowFromWidget(pt.y, this, WID_GL_LIST_GROUP);
+				auto it = this->group_sb->GetScrolledItemFromWidget(this->groups, pt.y, this, WID_GL_LIST_GROUP);
-				GroupID new_g = id_g >= this->groups.size() ? NEW_GROUP : this->groups[id_g]->index;
+				GroupID new_g = it == this->groups.end() ? NEW_GROUP : (*it)->index;
 
 				Command<CMD_ADD_VEHICLE_GROUP>::Post(STR_ERROR_GROUP_CAN_T_ADD_VEHICLE, new_g == NEW_GROUP ? CcAddVehicleNewGroup : nullptr, new_g, vindex, _ctrl_pressed || this->grouping == GB_SHARED_ORDERS);
 				break;
@@ -891,10 +892,10 @@ public:
 				this->group_over = INVALID_GROUP;
 				this->SetDirty();
 
-				uint id_v = this->vscroll->GetScrolledRowFromWidget(pt.y, this, WID_GL_LIST_VEHICLE);
+				auto it = this->vscroll->GetScrolledItemFromWidget(this->vehgroups, pt.y, this, WID_GL_LIST_VEHICLE);
-				if (id_v >= this->vehgroups.size()) return; // click out of list bound
+				if (it == this->vehgroups.end()) return; // click out of list bound
 
-				const GUIVehicleGroup &vehgroup = this->vehgroups[id_v];
+				const GUIVehicleGroup &vehgroup = *it;
 				switch (this->grouping) {
 					case GB_NONE: {
 						const Vehicle *v = vehgroup.GetSingleVehicle();
@@ -1023,8 +1024,8 @@ public:
 				break;
 
 			case WID_GL_LIST_GROUP: { // ... the list of custom groups.
-				uint id_g = this->group_sb->GetScrolledRowFromWidget(pt.y, this, WID_GL_LIST_GROUP);
+				auto it = this->group_sb->GetScrolledItemFromWidget(this->groups, pt.y, this, WID_GL_LIST_GROUP);
-				new_group_over = id_g >= this->groups.size() ? NEW_GROUP : this->groups[id_g]->index;
+				new_group_over = it == this->groups.end() ? NEW_GROUP : (*it)->index;
 				break;
 			}
 		}

src/industry_gui.cpp

@@ -632,9 +632,9 @@ public:
 			}
 
 			case WID_DPI_MATRIX_WIDGET: {
-				int y = this->vscroll->GetScrolledRowFromWidget(pt.y, this, WID_DPI_MATRIX_WIDGET);
+				auto it = this->vscroll->GetScrolledItemFromWidget(this->list, pt.y, this, WID_DPI_MATRIX_WIDGET);
-				if (y != INT_MAX) { // Is it within the boundaries of available data?
+				if (it != this->list.end()) { // Is it within the boundaries of available data?
-					this->selected_type = this->list[y];
+					this->selected_type = *it;
 					this->UpdateAvailability();
 
 					const IndustrySpec *indsp = GetIndustrySpec(this->selected_type);
@@ -1742,12 +1742,12 @@ public:
 				break;
 
 			case WID_ID_INDUSTRY_LIST: {
-				uint p = this->vscroll->GetScrolledRowFromWidget(pt.y, this, WID_ID_INDUSTRY_LIST, WidgetDimensions::scaled.framerect.top);
+				auto it = this->vscroll->GetScrolledItemFromWidget(this->industries, pt.y, this, WID_ID_INDUSTRY_LIST, WidgetDimensions::scaled.framerect.top);
-				if (p < this->industries.size()) {
+				if (it != this->industries.end()) {
 					if (_ctrl_pressed) {
-						ShowExtraViewportWindow(this->industries[p]->location.tile);
+						ShowExtraViewportWindow((*it)->location.tile);
 					} else {
-						ScrollMainWindowToTile(this->industries[p]->location.tile);
+						ScrollMainWindowToTile((*it)->location.tile);
 					}
 				}
 				break;

src/network/network_content_gui.cpp

@@ -799,11 +799,11 @@ public:
 
 		switch (widget) {
 			case WID_NCL_MATRIX: {
-				uint id_v = this->vscroll->GetScrolledRowFromWidget(pt.y, this, WID_NCL_MATRIX);
+				auto it = this->vscroll->GetScrolledItemFromWidget(this->content, pt.y, this, WID_NCL_MATRIX);
-				if (id_v >= this->content.size()) return; // click out of bounds
+				if (it == this->content.end()) return; // click out of bounds
 
-				this->selected = this->content[id_v];
+				this->selected = *it;
-				this->list_pos = id_v;
+				this->list_pos = it - this->content.begin();
 
 				const NWidgetBase *checkbox = this->GetWidget<NWidgetBase>(WID_NCL_CHECKBOX);
 				if (click_count > 1 || IsInsideBS(pt.x, checkbox->pos_x, checkbox->current_x)) {

src/network/network_gui.cpp

@@ -746,9 +746,9 @@ public:
 				break;
 
 			case WID_NG_MATRIX: { // Show available network games
-				uint id_v = this->vscroll->GetScrolledRowFromWidget(pt.y, this, WID_NG_MATRIX);
+				auto it = this->vscroll->GetScrolledItemFromWidget(this->servers, pt.y, this, WID_NG_MATRIX);
-				this->server = (id_v < this->servers.size()) ? this->servers[id_v] : nullptr;
+				this->server = (it != this->servers.end()) ? *it : nullptr;
-				this->list_pos = (server == nullptr) ? SLP_INVALID : id_v;
+				this->list_pos = (server == nullptr) ? SLP_INVALID : it - this->servers.begin();
 				this->SetDirty();
 
 				/* FIXME the disabling should go into some InvalidateData, which is called instead of the SetDirty */

src/newgrf_gui.cpp

@@ -1071,13 +1071,13 @@ struct NewGRFWindow : public Window, NewGRFScanCallback {
 			case WID_NS_AVAIL_LIST: { // Select a non-active GRF.
 				ResetObjectToPlace();
 
-				uint i = this->vscroll2->GetScrolledRowFromWidget(pt.y, this, WID_NS_AVAIL_LIST);
+				auto it = this->vscroll2->GetScrolledItemFromWidget(this->avails, pt.y, this, WID_NS_AVAIL_LIST);
 				this->active_sel = nullptr;
 				CloseWindowByClass(WC_GRF_PARAMETERS);
-				if (i < this->avails.size()) {
+				if (it != this->avails.end()) {
-					if (this->avail_sel != this->avails[i]) CloseWindowByClass(WC_TEXTFILE);
+					if (this->avail_sel != *it) CloseWindowByClass(WC_TEXTFILE);
-					this->avail_sel = this->avails[i];
+					this->avail_sel = *it;
-					this->avail_pos = i;
+					this->avail_pos = it - this->avails.begin();
 				}
 				this->InvalidateData();
 				if (click_count == 1) {
@@ -2123,10 +2123,10 @@ struct SavePresetWindow : public Window {
 	{
 		switch (widget) {
 			case WID_SVP_PRESET_LIST: {
-				uint row = this->vscroll->GetScrolledRowFromWidget(pt.y, this, WID_SVP_PRESET_LIST);
+				auto it = this->vscroll->GetScrolledItemFromWidget(this->presets, pt.y, this, WID_SVP_PRESET_LIST);
-				if (row < this->presets.size()) {
+				if (it != this->presets.end()) {
-					this->selected = row;
+					this->selected = it - this->presets.begin();
-					this->presetname_editbox.text.Assign(this->presets[row].c_str());
+					this->presetname_editbox.text.Assign(it->c_str());
 					this->SetWidgetDirty(WID_SVP_PRESET_LIST);
 					this->SetWidgetDirty(WID_SVP_EDITBOX);
 				}

src/rail_gui.cpp

@@ -1454,9 +1454,9 @@ public:
 				break;
 
 			case WID_BRAS_NEWST_LIST: {
-				int y = this->vscroll->GetScrolledRowFromWidget(pt.y, this, WID_BRAS_NEWST_LIST);
+				auto it = this->vscroll->GetScrolledItemFromWidget(this->station_classes, pt.y, this, WID_BRAS_NEWST_LIST);
-				if (y >= (int)this->station_classes.size()) return;
+				if (it == this->station_classes.end()) return;
-				StationClassID station_class_id = this->station_classes[y];
+				StationClassID station_class_id = *it;
 				if (_railstation.station_class != station_class_id) {
 					StationClass *station_class = StationClass::Get(station_class_id);
 					_railstation.station_class = station_class_id;

src/road_gui.cpp

@@ -1533,9 +1533,9 @@ public:
 				break;
 
 			case WID_BROS_NEWST_LIST: {
-				int y = this->vscrollList->GetScrolledRowFromWidget(pt.y, this, WID_BROS_NEWST_LIST);
+				auto it = this->vscrollList->GetScrolledItemFromWidget(this->roadstop_classes, pt.y, this, WID_BROS_NEWST_LIST);
-				if (y >= (int)this->roadstop_classes.size()) return;
+				if (it == this->roadstop_classes.end()) return;
-				RoadStopClassID class_id = this->roadstop_classes[y];
+				RoadStopClassID class_id = *it;
 				if (_roadstop_gui_settings.roadstop_class != class_id && GetIfClassHasNewStopsByType(RoadStopClass::Get(class_id), roadStopType, _cur_roadtype)) {
 					_roadstop_gui_settings.roadstop_class = class_id;
 					RoadStopClass *rsclass = RoadStopClass::Get(_roadstop_gui_settings.roadstop_class);

src/signs_gui.cpp

@@ -235,10 +235,10 @@ struct SignListWindow : Window, SignList {
 	{
 		switch (widget) {
 			case WID_SIL_LIST: {
-				uint id_v = this->vscroll->GetScrolledRowFromWidget(pt.y, this, WID_SIL_LIST, WidgetDimensions::scaled.framerect.top);
+				auto it = this->vscroll->GetScrolledItemFromWidget(this->signs, pt.y, this, WID_SIL_LIST, WidgetDimensions::scaled.framerect.top);
-				if (id_v == INT_MAX) return;
+				if (it == this->signs.end()) return;
 
-				const Sign *si = this->signs[id_v];
+				const Sign *si = *it;
 				ScrollMainWindowToTile(TileVirtXY(si->x, si->y));
 				break;
 			}

src/station_gui.cpp

@@ -507,10 +507,10 @@ public:
 	{
 		switch (widget) {
 			case WID_STL_LIST: {
-				uint id_v = this->vscroll->GetScrolledRowFromWidget(pt.y, this, WID_STL_LIST);
+				auto it = this->vscroll->GetScrolledItemFromWidget(this->stations, pt.y, this, WID_STL_LIST);
-				if (id_v >= this->stations.size()) return; // click out of list bound
+				if (it == this->stations.end()) return; // click out of list bound
 
-				const Station *st = this->stations[id_v];
+				const Station *st = *it;
 				/* do not check HasStationInUse - it is slow and may be invalid */
 				assert(st->owner == (Owner)this->window_number || st->owner == OWNER_NONE);
 

src/town_gui.cpp

@@ -942,10 +942,10 @@ public:
 				break;
 
 			case WID_TD_LIST: { // Click on Town Matrix
-				uint id_v = this->vscroll->GetScrolledRowFromWidget(pt.y, this, WID_TD_LIST, WidgetDimensions::scaled.framerect.top);
+				auto it = this->vscroll->GetScrolledItemFromWidget(this->towns, pt.y, this, WID_TD_LIST, WidgetDimensions::scaled.framerect.top);
-				if (id_v >= this->towns.size()) return; // click out of town bounds
+				if (it == this->towns.end()) return; // click out of town bounds
 
-				const Town *t = this->towns[id_v];
+				const Town *t = *it;
 				assert(t != nullptr);
 				if (_ctrl_pressed) {
 					ShowExtraViewportWindow(t->xy);

src/vehicle_gui.cpp

@@ -2024,10 +2024,10 @@ public:
 				break;
 
 			case WID_VL_LIST: { // Matrix to show vehicles
-				uint id_v = this->vscroll->GetScrolledRowFromWidget(pt.y, this, WID_VL_LIST);
+				auto it = this->vscroll->GetScrolledItemFromWidget(this->vehgroups, pt.y, this, WID_VL_LIST);
-				if (id_v >= this->vehgroups.size()) return; // click out of list bound
+				if (it == this->vehgroups.end()) return; // click out of list bound
 
-				const GUIVehicleGroup &vehgroup = this->vehgroups[id_v];
+				const GUIVehicleGroup &vehgroup = *it;
 				switch (this->grouping) {
 					case GB_NONE: {
 						const Vehicle *v = vehgroup.GetSingleVehicle();

src/widget_type.h

@@ -789,6 +789,28 @@ public:
 	}
 
 	int GetScrolledRowFromWidget(int clickpos, const Window * const w, int widget, int padding = 0) const;
+
+	/**
+	 * Return an iterator pointing to the element of a scrolled widget that a user clicked in.
+	 * @param container   Container of elements represented by the scrollbar.
+	 * @param clickpos    Vertical position of the mouse click (without taking scrolling into account).
+	 * @param w           The window the click was in.
+	 * @param widget      Widget number of the widget clicked in.
+	 * @param padding     Amount of empty space between the widget edge and the top of the first row. Default value is \c 0.
+	 * @return Iterator to the element clicked at. If clicked at a wrong position, returns as interator to the end of the container.
+	 */
+	template <typename Tcontainer>
+	typename Tcontainer::iterator GetScrolledItemFromWidget(Tcontainer &container, int clickpos, const Window * const w, int widget, int padding = 0) const
+	{
+		assert(this->GetCount() == container.size()); // Scrollbar and container size must match.
+		int row = this->GetScrolledRowFromWidget(clickpos, w, widget, padding);
+		if (row == INT_MAX) return std::end(container);
+
+		typename Tcontainer::iterator it = std::begin(container);
+		std::advance(it, row);
+		return it;
+	}
+
 	EventState UpdateListPositionOnKeyPress(int &list_position, uint16 keycode) const;
 };