# HG changeset patch # User Peter Nelson # Date 2023-05-12 11:58:31 # Node ID 5391a05e2771700503aca0153348677d3f29108a # Parent 8eb15d885acd5fb2570b41e702e7ebc1462d813b 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. diff --git a/src/autoreplace_gui.cpp b/src/autoreplace_gui.cpp --- a/src/autoreplace_gui.cpp +++ b/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 &item = this->engines[click_side][i]; + const auto it = this->vscroll[click_side]->GetScrolledItemFromWidget(this->engines[click_side], pt.y, this, widget); + if (it != this->engines[click_side].end()) { + const auto &item = *it; const Rect r = this->GetWidget(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 */ diff --git a/src/bridge_gui.cpp b/src/bridge_gui.cpp --- a/src/bridge_gui.cpp +++ b/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); - if (i < this->bridges->size()) { - this->BuildBridge(i); + auto it = this->vscroll->GetScrolledItemFromWidget(*this->bridges, pt.y, this, WID_BBS_BRIDGE_LIST); + if (it != this->bridges->end()) { + this->BuildBridge(it - this->bridges->begin()); this->Close(); } break; diff --git a/src/build_vehicle_gui.cpp b/src/build_vehicle_gui.cpp --- a/src/build_vehicle_gui.cpp +++ b/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 &item = this->eng_list[i]; + const auto it = this->vscroll->GetScrolledItemFromWidget(this->eng_list, pt.y, this, WID_BV_LIST); + if (it != this->eng_list.end()) { + const auto &item = *it; const Rect r = this->GetWidget(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 */ diff --git a/src/graph_gui.cpp b/src/graph_gui.cpp --- a/src/graph_gui.cpp +++ b/src/graph_gui.cpp @@ -999,13 +999,9 @@ struct PaymentRatesGraphWindow : BaseGra } case WID_CPR_MATRIX: { - uint row = this->vscroll->GetScrolledRowFromWidget(pt.y, this, WID_CPR_MATRIX); - if (row >= this->vscroll->GetCount()) return; - - for (const CargoSpec *cs : _sorted_standard_cargo_specs) { - if (row-- > 0) continue; - - ToggleBit(_legend_excluded_cargo, cs->Index()); + auto it = this->vscroll->GetScrolledItemFromWidget(_sorted_standard_cargo_specs, pt.y, this, WID_CPR_MATRIX); + if (it != _sorted_standard_cargo_specs.end()) { + ToggleBit(_legend_excluded_cargo, (*it)->Index()); this->UpdateExcludedData(); this->SetDirty(); break; diff --git a/src/group_gui.cpp b/src/group_gui.cpp --- a/src/group_gui.cpp +++ b/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); - if (id_g >= this->groups.size()) return; + auto it = this->group_sb->GetScrolledItemFromWidget(this->groups, pt.y, this, WID_GL_LIST_GROUP); + 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(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); - if (id_v >= this->vehgroups.size()) return; // click out of list bound + auto it = this->vscroll->GetScrolledItemFromWidget(this->vehgroups, pt.y, this, WID_GL_LIST_VEHICLE); + 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); - GroupID new_g = id_g >= this->groups.size() ? INVALID_GROUP : this->groups[id_g]->index; + auto it = this->group_sb->GetScrolledItemFromWidget(this->groups, pt.y, this, WID_GL_LIST_GROUP); + GroupID new_g = it == this->groups.end() ? INVALID_GROUP : (*it)->index; if (this->group_sel != new_g && g->parent != new_g) { Command::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); - GroupID new_g = id_g >= this->groups.size() ? NEW_GROUP : this->groups[id_g]->index; + auto it = this->group_sb->GetScrolledItemFromWidget(this->groups, pt.y, this, WID_GL_LIST_GROUP); + GroupID new_g = it == this->groups.end() ? NEW_GROUP : (*it)->index; Command::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); - if (id_v >= this->vehgroups.size()) return; // click out of list bound + auto it = this->vscroll->GetScrolledItemFromWidget(this->vehgroups, pt.y, this, WID_GL_LIST_VEHICLE); + 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); - new_group_over = id_g >= this->groups.size() ? NEW_GROUP : this->groups[id_g]->index; + auto it = this->group_sb->GetScrolledItemFromWidget(this->groups, pt.y, this, WID_GL_LIST_GROUP); + new_group_over = it == this->groups.end() ? NEW_GROUP : (*it)->index; break; } } diff --git a/src/industry_gui.cpp b/src/industry_gui.cpp --- a/src/industry_gui.cpp +++ b/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); - if (y != INT_MAX) { // Is it within the boundaries of available data? - this->selected_type = this->list[y]; + auto it = this->vscroll->GetScrolledItemFromWidget(this->list, pt.y, this, WID_DPI_MATRIX_WIDGET); + if (it != this->list.end()) { // Is it within the boundaries of available data? + 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); - if (p < this->industries.size()) { + auto it = this->vscroll->GetScrolledItemFromWidget(this->industries, pt.y, this, WID_ID_INDUSTRY_LIST, WidgetDimensions::scaled.framerect.top); + 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; diff --git a/src/network/network_content_gui.cpp b/src/network/network_content_gui.cpp --- a/src/network/network_content_gui.cpp +++ b/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); - if (id_v >= this->content.size()) return; // click out of bounds + auto it = this->vscroll->GetScrolledItemFromWidget(this->content, pt.y, this, WID_NCL_MATRIX); + if (it == this->content.end()) return; // click out of bounds - this->selected = this->content[id_v]; - this->list_pos = id_v; + this->selected = *it; + this->list_pos = it - this->content.begin(); const NWidgetBase *checkbox = this->GetWidget(WID_NCL_CHECKBOX); if (click_count > 1 || IsInsideBS(pt.x, checkbox->pos_x, checkbox->current_x)) { diff --git a/src/network/network_gui.cpp b/src/network/network_gui.cpp --- a/src/network/network_gui.cpp +++ b/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); - this->server = (id_v < this->servers.size()) ? this->servers[id_v] : nullptr; - this->list_pos = (server == nullptr) ? SLP_INVALID : id_v; + auto it = this->vscroll->GetScrolledItemFromWidget(this->servers, pt.y, this, WID_NG_MATRIX); + this->server = (it != this->servers.end()) ? *it : nullptr; + 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 */ diff --git a/src/newgrf_gui.cpp b/src/newgrf_gui.cpp --- a/src/newgrf_gui.cpp +++ b/src/newgrf_gui.cpp @@ -1071,13 +1071,13 @@ struct NewGRFWindow : public Window, New 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 (this->avail_sel != this->avails[i]) CloseWindowByClass(WC_TEXTFILE); - this->avail_sel = this->avails[i]; - this->avail_pos = i; + if (it != this->avails.end()) { + if (this->avail_sel != *it) CloseWindowByClass(WC_TEXTFILE); + this->avail_sel = *it; + 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); - if (row < this->presets.size()) { - this->selected = row; - this->presetname_editbox.text.Assign(this->presets[row].c_str()); + auto it = this->vscroll->GetScrolledItemFromWidget(this->presets, pt.y, this, WID_SVP_PRESET_LIST); + if (it != this->presets.end()) { + this->selected = it - this->presets.begin(); + this->presetname_editbox.text.Assign(it->c_str()); this->SetWidgetDirty(WID_SVP_PRESET_LIST); this->SetWidgetDirty(WID_SVP_EDITBOX); } diff --git a/src/rail_gui.cpp b/src/rail_gui.cpp --- a/src/rail_gui.cpp +++ b/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); - if (y >= (int)this->station_classes.size()) return; - StationClassID station_class_id = this->station_classes[y]; + auto it = this->vscroll->GetScrolledItemFromWidget(this->station_classes, pt.y, this, WID_BRAS_NEWST_LIST); + if (it == this->station_classes.end()) return; + 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; diff --git a/src/road_gui.cpp b/src/road_gui.cpp --- a/src/road_gui.cpp +++ b/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); - if (y >= (int)this->roadstop_classes.size()) return; - RoadStopClassID class_id = this->roadstop_classes[y]; + auto it = this->vscrollList->GetScrolledItemFromWidget(this->roadstop_classes, pt.y, this, WID_BROS_NEWST_LIST); + if (it == this->roadstop_classes.end()) return; + 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); diff --git a/src/signs_gui.cpp b/src/signs_gui.cpp --- a/src/signs_gui.cpp +++ b/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); - if (id_v == INT_MAX) return; + auto it = this->vscroll->GetScrolledItemFromWidget(this->signs, pt.y, this, WID_SIL_LIST, WidgetDimensions::scaled.framerect.top); + if (it == this->signs.end()) return; - const Sign *si = this->signs[id_v]; + const Sign *si = *it; ScrollMainWindowToTile(TileVirtXY(si->x, si->y)); break; } diff --git a/src/station_gui.cpp b/src/station_gui.cpp --- a/src/station_gui.cpp +++ b/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); - if (id_v >= this->stations.size()) return; // click out of list bound + auto it = this->vscroll->GetScrolledItemFromWidget(this->stations, pt.y, this, WID_STL_LIST); + 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); diff --git a/src/town_gui.cpp b/src/town_gui.cpp --- a/src/town_gui.cpp +++ b/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); - if (id_v >= this->towns.size()) return; // click out of town bounds + auto it = this->vscroll->GetScrolledItemFromWidget(this->towns, pt.y, this, WID_TD_LIST, WidgetDimensions::scaled.framerect.top); + 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); diff --git a/src/vehicle_gui.cpp b/src/vehicle_gui.cpp --- a/src/vehicle_gui.cpp +++ b/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); - if (id_v >= this->vehgroups.size()) return; // click out of list bound - - const GUIVehicleGroup &vehgroup = this->vehgroups[id_v]; + auto it = this->vscroll->GetScrolledItemFromWidget(this->vehgroups, pt.y, this, WID_VL_LIST); + if (it == this->vehgroups.end()) return; // click out of list bound + + const GUIVehicleGroup &vehgroup = *it; switch (this->grouping) { case GB_NONE: { const Vehicle *v = vehgroup.GetSingleVehicle(); diff --git a/src/widget_type.h b/src/widget_type.h --- a/src/widget_type.h +++ b/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::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; };