0ct0pu5/wesnoth
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Editor palettes: allow keyboard scrolling to see the last row

Browse source 
Fixes #6724, where scrolling with the mousewheel could reach the last row, but
scrolling with keys or buttons couldn't. Although can_scroll_up() and
can_scroll_down() already existed, can_scroll_down() didn't allow access to the
last line when that line had less than the full number of items. The mousewheel
still worked, because it ignored that logic.

Remove a feature that was seen when the last row is visible - things jumped
between columns to fill the spaces. If you have 10 items, with 4 columns and 2
visible rows then it will appear like this:
    a b c d
    e f g h
when scrolling down it used to move items sideways to fill the last line, which
seems bad UI because it made items harder to find:
    c d e f
    g h i j
the new code is simpler, and will instead show the following when scrolling down:
    e f g h
    i j

Move the layout code into adjust_size(), so that it runs one time when the number
of buttons changes. That could be separated from this commit, but the code
would still be touched in this commit (`counter_from_zero` would still be
replaced by `i`), so doing it and testing the changes together made sense.

Note for the master branch only: scrolling the palettes up and down is
noticeably laggy, which is a regression from last week. That's not caused by
this commit, we're just at a point in the rendering refactor where
`surface_restorer::update()` and thus `gui::widget::hide()` are slow. The fix
for that is already in progress and going to be in 1.17.5.
This commit is contained in:
Steve Cotton 2022-05-27 13:14:51 +02:00 • committed by Steve Cotton
parent 4dc2c342f6
commit 8edd8f2292
9 changed files with 160 additions and 149 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/editor/palette/common_palette.hpp
View file

@ -78,7 +78,7 @@ public:
virtual void expand_palette_groups_menu(std::vector<config>& items, int i) = 0;
//item
virtual int num_items() = 0;
virtual std::size_t num_items() = 0;
virtual std::size_t start_num() = 0;
virtual void set_start_item(std::size_t index) = 0;

116
src/editor/palette/editor_palettes.cpp
View file

@ -75,16 +75,19 @@ void editor_palette<Item>::expand_palette_groups_menu(std::vector<config>& items
template<class Item>
bool editor_palette<Item>::scroll_up()
{
int decrement = item_width_;
if (items_start_ + num_visible_items() == num_items() && num_items() % item_width_ != 0) {
decrement = num_items() % item_width_;
bool scrolled = false;
if(can_scroll_up()) {
// This should only be reachable with items_start_ being a multiple of columns_, but guard against underflow anyway.
if(items_start_ < columns_) {
items_start_ = 0;
} else {
items_start_ -= columns_;
}
scrolled = true;
set_dirty(true);
}
if(items_start_ >= decrement) {
items_start_ -= decrement;
draw();
return true;
}
return false;
draw();
return scrolled;
}
template<class Item>
@ -96,25 +99,18 @@ bool editor_palette<Item>::can_scroll_up()
template<class Item>
bool editor_palette<Item>::can_scroll_down()
{
return (items_start_ + nitems_ + item_width_ <= num_items());
return (items_start_ + buttons_.size() < num_items());
}
template<class Item>
bool editor_palette<Item>::scroll_down()
{
bool end_reached = (!(items_start_ + nitems_ + item_width_ <= num_items()));
bool scrolled = false;
// move downwards
if(!end_reached) {
items_start_ += item_width_;
if(can_scroll_down()) {
items_start_ += columns_;
scrolled = true;
set_dirty(true);
}
else if (items_start_ + nitems_ + (num_items() % item_width_) <= num_items()) {
items_start_ += num_items() % item_width_;
scrolled = true;
}
set_dirty(scrolled);
draw();
return scrolled;
}
@ -168,14 +164,32 @@ std::size_t editor_palette<Item>::active_group_index()
template<class Item>
void editor_palette<Item>::adjust_size(const SDL_Rect& target)
{
palette_x_ = target.x;
palette_y_ = target.y;
const int space_for_items = target.h;
const int items_fitting = (space_for_items / item_space_) * item_width_;
nitems_ = std::min(items_fitting, nmax_items_);
if (num_visible_items() != nitems_) {
buttons_.resize(nitems_, gui::tristate_button(gui_.video(), this));
const int items_fitting = (target.h / item_space_) * columns_;
// This might be called while the palette is not visible onscreen.
// If that happens, no items will fit and we'll have a negative number here.
// Just skip it in that case.
//
// New items can be added via the add_item function, so this creates as
// many buttons as can fit, even if there aren't yet enough items to need
// that many buttons.
if(items_fitting > 0) {
const auto buttons_needed = static_cast<std::size_t>(items_fitting);
if(buttons_.size() != buttons_needed) {
buttons_.resize(buttons_needed, gui::tristate_button(gui_.video(), this));
}
}
// Update button locations and sizes. Needs to be done even if the number of buttons hasn't changed,
// because adjust_size() also handles moving left and right when the window's width is changed.
SDL_Rect dstrect;
dstrect.w = item_size_ + 2;
dstrect.h = item_size_ + 2;
for(std::size_t i = 0; i < buttons_.size(); ++i) {
dstrect.x = target.x + (i % columns_) * item_space_;
dstrect.y = target.y + (i / columns_) * item_space_;
buttons_[i].set_location(dstrect);
}
set_location(target);
set_dirty(true);
gui_.video().clear_help_string(help_handle_);
@ -214,7 +228,7 @@ void editor_palette<Item>::swap()
}
template<class Item>
int editor_palette<Item>::num_items()
std::size_t editor_palette<Item>::num_items()
{
return group_map_[active_group_].size();
}
@ -237,8 +251,7 @@ void editor_palette<Item>::draw_contents()
toolkit_.set_mouseover_overlay(gui_);
std::shared_ptr<gui::button> palette_menu_button = gui_.find_menu_button("menu-editor-terrain");
if (palette_menu_button) {
if(palette_menu_button) {
t_string& name = groups_[active_group_index()].name;
std::string& icon = groups_[active_group_index()].icon;
@ -246,31 +259,27 @@ void editor_palette<Item>::draw_contents()
palette_menu_button->set_overlay(icon);
}
unsigned int y = palette_y_;
unsigned int x = palette_x_;
int starting = items_start_;
int ending = std::min<int>(starting + nitems_, num_items());
// The hotkey system will automatically enable and disable the buttons when it runs, but it doesn't
// get triggered when handling mouse-wheel scrolling. Therefore duplicate that functionality here.
std::shared_ptr<gui::button> upscroll_button = gui_.find_action_button("upscroll-button-editor");
if (upscroll_button)
upscroll_button->enable(starting != 0);
if(upscroll_button)
upscroll_button->enable(can_scroll_up());
std::shared_ptr<gui::button> downscroll_button = gui_.find_action_button("downscroll-button-editor");
if (downscroll_button)
downscroll_button->enable(ending != num_items());
int counter = starting;
for (int i = 0, size = num_visible_items(); i < size ; ++i) {
//TODO check if the conditions still hold for the counter variable
//for (unsigned int counter = starting; counter < ending; counter++)
if(downscroll_button)
downscroll_button->enable(can_scroll_down());
for(std::size_t i = 0; i < buttons_.size(); ++i) {
const auto item_index = items_start_ + i;
gui::tristate_button& tile = buttons_[i];
tile.hide(true);
if (i >= ending) continue;
// If we've scrolled to the end of the list, or if there aren't many items, leave the button hidden
if(item_index >= num_items()) {
continue;
}
const std::string item_id = active_group()[counter];
const std::string item_id = active_group()[item_index];
//typedef std::map<std::string, Item> item_map_wurscht;
typename item_map::iterator item = item_map_.find(item_id);
@ -287,14 +296,6 @@ void editor_palette<Item>::draw_contents()
<< "</span>";
}
const int counter_from_zero = counter - starting;
SDL_Rect dstrect;
dstrect.x = x + (counter_from_zero % item_width_) * item_space_;
dstrect.y = y;
dstrect.w = item_size_ + 2;
dstrect.h = item_size_ + 2;
tile.set_location(dstrect);
tile.set_tooltip_string(tooltip_text.str());
tile.set_item_image(item_image);
tile.set_item_id(item_id);
@ -324,11 +325,6 @@ void editor_palette<Item>::draw_contents()
tile.set_dirty(true);
tile.hide(false);
tile.draw();
// Adjust location
if (counter_from_zero % item_width_ == item_width_ - 1)
y += item_space_;
++counter;
}
}

34
src/editor/palette/editor_palettes.hpp
View file

@ -29,20 +29,16 @@ class editor_palette : public tristate_palette {
public:
editor_palette(editor_display &gui, const game_config_view& /*cfg*/
, std::size_t item_size, std::size_t item_width, editor_toolkit &toolkit)
, std::size_t item_size, std::size_t columns, editor_toolkit &toolkit)
: tristate_palette(gui.video())
, groups_()
, gui_(gui)
, item_size_(item_size)
, item_width_(item_width)
//TODO avoid magic number
, item_space_(item_size + 3)
, palette_y_(0)
, palette_x_(0)
, columns_(columns)
, group_map_()
, item_map_()
, nitems_(0)
, nmax_items_(0)
, items_start_(0)
, non_core_items_()
, active_group_()
@ -118,11 +114,8 @@ private:
virtual bool is_selected_fg_item(const std::string& id);
virtual bool is_selected_bg_item(const std::string& id);
/** Return the number of items in the palette. */
int num_items() override;
/** Return the number of items in the palette. */
int num_visible_items() { return buttons_.size(); }
/** Return the number of items in the currently-active group. */
std::size_t num_items() override;
void hide(bool hidden) override {
widget::hide(hidden);
@ -158,20 +151,29 @@ protected:
editor_display &gui_;
/**
* Both the width and the height of the square buttons.
*/
int item_size_;
int item_width_;
/**
* item_space_ plus some padding.
*/
int item_space_;
private:
unsigned int palette_y_;
unsigned int palette_x_;
/**
* Number of items per row.
*/
std::size_t columns_;
protected:
std::map<std::string, std::vector<std::string>> group_map_;
typedef std::map<std::string, Item> item_map;
item_map item_map_;
int nitems_, nmax_items_, items_start_;
/**
* Index of the item at the top-left of the visible area, used for scrolling up and down.
*/
std::size_t items_start_;
std::set<std::string> non_core_items_;
private:

2
src/editor/palette/empty_palette.hpp
View file

@ -70,7 +70,7 @@ public:
}
//item
virtual int num_items() override {return 0;}
virtual std::size_t num_items() override {return 0;}
virtual std::size_t start_num() override {return 0;}
virtual void set_start_item(std::size_t /*index*/) override {}
virtual bool supports_swap() override { return false; }

1
src/editor/palette/item_palette.cpp
View file

@ -43,7 +43,6 @@ void item_palette::setup(const game_config_view& cfg)
if(!group["core"].to_bool(false))
non_core_items_.insert(item["id"]);
}
nmax_items_ = std::max<int>(nmax_items_, group_map_[group["id"]].size());
}
select_fg_item("anvil");

128
src/editor/palette/location_palette.cpp
View file

@ -165,8 +165,6 @@ location_palette::location_palette(editor_display &gui, const game_config_view&
, item_size_(20)
//TODO avoid magic number
, item_space_(20 + 3)
, palette_y_(0)
, palette_x_(0)
, items_start_(0)
, selected_item_()
, items_()
@ -213,13 +211,14 @@ void location_palette::hide(bool hidden)
bool location_palette::scroll_up()
{
int decrement = 1;
if(items_start_ >= decrement) {
items_start_ -= decrement;
draw();
return true;
bool scrolled = false;
if(can_scroll_up()) {
--items_start_;
scrolled = true;
set_dirty(true);
}
return false;
draw();
return scrolled;
}
bool location_palette::can_scroll_up()
{
@ -233,12 +232,9 @@ bool location_palette::can_scroll_down()
bool location_palette::scroll_down()
{
bool end_reached = (!(items_start_ + num_visible_items() + 1 <= num_items()));
bool scrolled = false;
// move downwards
if(!end_reached) {
items_start_ += 1;
if(can_scroll_down()) {
++items_start_;
scrolled = true;
set_dirty(true);
}
@ -248,8 +244,6 @@ bool location_palette::scroll_down()
void location_palette::adjust_size(const SDL_Rect& target)
{
palette_x_ = target.x;
palette_y_ = target.y;
const int button_height = 22;
const int button_y = 30;
int bottom = target.y + target.h;
@ -291,9 +285,26 @@ void location_palette::adjust_size(const SDL_Rect& target)
// This might be called while the palette is not visible onscreen.
// If that happens, no items will fit and we'll have a negative number here.
// Just skip it in that case.
if(items_fitting > 0 && num_visible_items() != items_fitting) {
location_palette_item lpi(disp_.video(), this);
buttons_.resize(items_fitting, lpi);
if(items_fitting > 0) {
// Items may be added dynamically via add_item(), so this creates all the buttons that
// fit in the space, even if some of them will be hidden until more items are added.
// This simplifies the scrolling code in add_item.
const std::size_t buttons_needed = items_fitting;
if(buttons_.size() != buttons_needed) {
location_palette_item lpi(disp_.video(), this);
buttons_.resize(buttons_needed, lpi);
}
}
// Update button locations and sizes. Needs to be done even if the number of buttons hasn't changed,
// because adjust_size() also handles moving left and right when the window's width is changed.
SDL_Rect dstrect;
dstrect.w = target.w - 10;
dstrect.h = item_size_ + 2;
for(std::size_t i = 0; i < buttons_.size(); ++i) {
dstrect.x = target.x;
dstrect.y = target.y + i * item_space_;
buttons_[i].set_location(dstrect);
}
set_location(target);
@ -312,11 +323,11 @@ void location_palette::select_item(const std::string& item_id)
help_handle_ = disp_.video().set_help_string(get_help_string());
}
int location_palette::num_items()
std::size_t location_palette::num_items()
{
return items_.size();
}
int location_palette::num_visible_items()
std::size_t location_palette::num_visible_items()
{
return buttons_.size();
}
@ -329,57 +340,49 @@ bool location_palette::is_selected_item(const std::string& id)
void location_palette::draw_contents()
{
toolkit_.set_mouseover_overlay(disp_);
int y = palette_y_;
const int x = palette_x_;
const int starting = items_start_;
int ending = std::min<int>(starting + num_visible_items(), num_items());
std::shared_ptr<gui::button> upscroll_button = disp_.find_action_button("upscroll-button-editor");
if (upscroll_button)
upscroll_button->enable(starting != 0);
std::shared_ptr<gui::button> downscroll_button = disp_.find_action_button("downscroll-button-editor");
if (downscroll_button)
downscroll_button->enable(ending != num_items());
if (button_goto_) {
// The hotkey system will automatically enable and disable the buttons when it runs, but it doesn't
// get triggered when handling mouse-wheel scrolling. Therefore duplicate that functionality here.
std::shared_ptr<gui::button> upscroll_button = disp_.find_action_button("upscroll-button-editor");
if(upscroll_button)
upscroll_button->enable(can_scroll_up());
std::shared_ptr<gui::button> downscroll_button = disp_.find_action_button("downscroll-button-editor");
if(downscroll_button)
downscroll_button->enable(can_scroll_down());
if(button_goto_) {
button_goto_->set_dirty(true);
}
if (button_add_) {
if(button_add_) {
button_add_->set_dirty(true);
}
if (button_delete_) {
if(button_delete_) {
button_delete_->set_dirty(true);
}
for (int i = 0, size = num_visible_items(); i < size; i++) {
location_palette_item & tile = buttons_[i];
for(std::size_t i = 0; i < num_visible_items(); ++i) {
const auto item_index = items_start_ + i;
location_palette_item& tile = buttons_[i];
tile.hide(true);
if (i >= ending) {
//We want to hide all following buttons so we cannot use break here.
// If we've scrolled to the end of the list, or if there aren't many items, leave the button hidden
if(item_index >= num_items()) {
// We want to hide all following buttons so we cannot use break here.
continue;
}
const std::string item_id = items_[starting + i];
const std::string item_id = items_[item_index];
// These could have tooltips, but currently don't. Adding their hex co-ordinates would be an option,
// and for player starts adding the raw ID next might be good.
std::stringstream tooltip_text;
SDL_Rect dstrect;
dstrect.x = x;
dstrect.y = y;
dstrect.w = location().w - 10;
dstrect.h = item_size_ + 2;
tile.set_location(dstrect);
tile.set_tooltip_string(tooltip_text.str());
tile.set_item_id(item_id);
tile.set_selected(is_selected_item(item_id));
tile.set_dirty(true);
tile.hide(false);
tile.draw();
// Adjust location
y += item_space_;
}
}
@ -413,7 +416,8 @@ static bool loc_id_comp(const std::string& lhs, const std::string& rhs) {
void location_palette::add_item(const std::string& id)
{
int pos;
decltype(items_)::difference_type pos;
// Insert the new ID at the sorted location, unless it's already in the list
const auto itor = std::upper_bound(items_.begin(), items_.end(), id, loc_id_comp);
if(itor == items_.begin() || *(itor - 1) != id) {
@ -422,13 +426,25 @@ void location_palette::add_item(const std::string& id)
pos = std::distance(items_.begin(), itor);
}
selected_item_ = id;
if(num_visible_items() == 0) {
items_start_ = 0;
} else {
items_start_ = std::max(pos - num_visible_items() + 1, items_start_);
items_start_ = std::min(pos, items_start_);
// pos will always be positive because begin() was used as the first arg of std::distance(),
// but we need this (or casts) to prevent warnings about signed/unsigned comparisons.
const std::size_t unsigned_pos = pos;
// Scroll if necessary so that the new item is visible
if(unsigned_pos < items_start_ || unsigned_pos >= items_start_ + num_visible_items()) {
if(unsigned_pos < num_visible_items()) {
items_start_ = 0;
} else if(unsigned_pos + num_visible_items() > num_items()) {
// This can't underflow, because unsigned_pos < num_items() and the
// previous conditional block would have been entered instead.
items_start_ = num_items() - num_visible_items();
} else {
items_start_ = unsigned_pos - num_visible_items() / 2;
}
}
adjust_size(location());
// No need to call adjust_size(), because initialisation creates all possible buttons even when num_visible_items() > num_items().
}
} // end namespace editor

22
src/editor/palette/location_palette.hpp
View file

@ -26,6 +26,10 @@ namespace editor {
class editor_toolkit;
/**
* List of starting locations and location ids. Shows a single-column list, with buttons to add
* new items to the list and to jump to that location on the map.
*/
class location_palette : public common_palette {
public:
@ -83,7 +87,6 @@ public:
void hide(bool hidden) override;
private:
/** Scroll the editor-palette to the top. */
void scroll_top();
@ -93,22 +96,21 @@ private:
virtual bool is_selected_item(const std::string& id);
/** Return the number of items in the palette. */
int num_items() override;
/** Return the maximum number of items shown at the same time. */
int num_visible_items();
std::size_t num_items() override;
/**
* Return the number of GUI elements that can show items. Some of these may be hidden, if there
* are more of them than items to show, or if the palette has been scrolled to the bottom.
*/
std::size_t num_visible_items();
protected:
int item_size_;
// the height of a row, the size of an item including borders.
int item_space_;
private:
unsigned int palette_y_;
unsigned int palette_x_;
protected:
//the current scrolling position
int items_start_;
// the current scrolling position
std::size_t items_start_;
private:
std::string selected_item_;

2
src/editor/palette/terrain_palettes.cpp
View file

@ -133,7 +133,6 @@ void terrain_palette::setup(const game_config_view& cfg)
for (const std::string& k : keys) {
group_map_[k].push_back(get_id(t));
nmax_items_ = std::max<int>(nmax_items_, group_map_[k].size());
std::map<std::string, item_group*>::iterator i = id_to_group.find(k);
if (i != id_to_group.end()) {
if (i->second->core) {
@ -147,7 +146,6 @@ void terrain_palette::setup(const game_config_view& cfg)
if (core) {
// Add the terrain to the default group
group_map_["all"].push_back(get_id(t));
nmax_items_ = std::max<int>(nmax_items_, group_map_["all"].size());
} else {
non_core_items_.insert(get_id(t));
}

2
src/editor/palette/unit_palette.cpp
View file

@ -39,10 +39,8 @@ void unit_palette::setup(const game_config_view& /*cfg*/)
continue;
item_map_.emplace(i.second.id(), i.second);
group_map_[i.second.race_id()].push_back(i.second.id());
nmax_items_ = std::max<int>(nmax_items_, group_map_[i.second.race_id()].size());
// Add the unit to the default group
group_map_["all"].push_back(i.second.id());
nmax_items_ = std::max<int>(nmax_items_, group_map_["all"].size());
}
for(const race_map::value_type &i : unit_types.races()) {