Editor palettes: allow keyboard scrolling to see the last row

src/editor/palette/common_palette.hpp

@@ -78,7 +78,7 @@ class common_palette  : public gui::widget {
 	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;
 

src/editor/palette/editor_palettes.cpp

@@ -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_;
-	}
-	if(items_start_ >= decrement) {
-		items_start_ -= decrement;
-		draw();
-		return true;
+	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);
 	}
-	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,40 +251,35 @@ 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;
 
 		palette_menu_button->set_tooltip_string(name);
 		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;
 	}
 }
 

src/editor/palette/editor_palettes.hpp

@@ -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 @@ class editor_palette : public tristate_palette {
 	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 @@ class editor_palette : public tristate_palette {
 
 	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:

src/editor/palette/empty_palette.hpp

@@ -70,7 +70,7 @@ class empty_palette : public common_palette {
 	}
 
     //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; }

src/editor/palette/item_palette.cpp

@@ -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");