Editor: Refresh named locations list when map context changes (fixes #1023)

This solution is a bit of a hack, but odds are this code will be
majorly refactored in 1.15 anyway. The important thing is that
this works.

(cherry-picked from commit 8e4de9db67)

changelog.md

@@ -92,6 +92,7 @@
    * Fixed assertion when saving game events mid-event.
    * Fix MP password save error (#2745)
    * Fixed sometimes being unable to join MP games with non-required eras.
+   * Fixed locations not being added to the palette when loading a map (#1023)
 
 ## Version 1.13.12
  ### Security fixes

src/editor/controller/editor_controller.cpp

@@ -81,6 +81,7 @@ editor_controller::editor_controller(const config &game_config)
 	init_gui();
 	toolkit_.reset(new editor_toolkit(*gui_.get(), key_, game_config_, *context_manager_.get()));
 	help_manager_.reset(new help::help_manager(&game_config));
+	context_manager_->locs_ = toolkit_->get_palette_manager()->location_palette_.get();
 	context_manager_->switch_context(0, true);
 	init_tods(game_config);
 	init_music(game_config);
@@ -926,6 +927,10 @@ bool editor_controller::do_execute_command(const hotkey::hotkey_command& cmd, in
 
 		// Side specific ones
 		case HOTKEY_EDITOR_SIDE_NEW:
+			if(get_current_map_context().teams().size() >= 9) {
+				size_t new_side_num = get_current_map_context().teams().size() + 1;
+				toolkit_->get_palette_manager()->location_palette_->add_item(std::to_string(new_side_num));
+			}
 			get_current_map_context().new_side();
 			gui_->init_flags();
 			return true;

src/editor/map/context_manager.cpp

@@ -64,7 +64,8 @@ static const std::string get_menu_marker(const bool changed)
 }
 
 context_manager::context_manager(editor_display& gui, const config& game_config)
-	: gui_(gui)
+	: locs_(nullptr)
+	, gui_(gui)
 	, game_config_(game_config)
 	, default_dir_(preferences::editor::default_dir())
 	, map_generators_()
@@ -121,6 +122,19 @@ void context_manager::refresh_all()
 	gui_.invalidate_all();
 	get_map_context().clear_changed_locations();
 	gui_.recalculate_minimap();
+	if(locs_) {
+		for(const auto& loc : get_map_context().map().special_locations().left) {
+			locs_->add_item(loc.first);
+		}
+		if(!get_map_context().is_pure_map()) {
+			// If the scenario has more than 9 teams, add locations for them
+			// (First 9 teams are always in the list)
+			size_t n_teams = get_map_context().teams().size();
+			for(size_t i = 10; i <= n_teams; i++) {
+				locs_->add_item(std::to_string(i));
+			}
+		}
+	}
 }
 
 void context_manager::reload_map()

src/editor/map/context_manager.hpp

@@ -191,6 +191,8 @@ public:
 		return nullptr; // No Lua kernel in the editor.
 	}
 
+	// TODO: Make this private with an accessor or something
+	class location_palette* locs_;
 private:
 	/** init available random map generators */
 	void init_map_generators(const config& game_config);

src/editor/palette/location_palette.cpp

@@ -25,6 +25,11 @@
 #include "gui/dialogs/edit_text.hpp"
 
 #include "formula/string_utils.hpp"
+
+static bool is_positive_integer(const std::string& str) {
+	return str != "0" && std::find_if(str.begin(), str.end(), [](char c) { return !std::isdigit(c); }) == str.end();
+}
+
 class location_palette_item : public gui::widget
 {
 public:
@@ -106,8 +111,7 @@ public:
 	void set_item_id(const std::string& id)
 	{
 		id_ = id;
-		bool is_number = std::find_if(id.begin(), id.end(), [](char c) { return !std::isdigit(c); }) == id.end();
-		if (is_number) {
+		if (is_positive_integer(id)) {
 			desc_ = VGETTEXT("Player $side_num", utils::string_map{ {"side_num", id} });
 		}
 		else {
@@ -268,7 +272,10 @@ void location_palette::adjust_size(const SDL_Rect& target)
 
 	const int space_for_items = bottom - target.y;
 	const int items_fitting = space_for_items / item_space_;
-	if (num_visible_items() != items_fitting) {
+	// 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);
 		//Why does this need a pointer to a non-const as second paraeter?
 		//TODO: we should write our own ptr_vector class, boost::ptr_vector has a lot of flaws.
@@ -375,20 +382,39 @@ location_palette::~location_palette()
 {
 }
 
+// Sort numbers before all other strings.
+static bool loc_id_comp(const std::string& lhs, const std::string& rhs) {
+	if(is_positive_integer(lhs)) {
+		if(is_positive_integer(rhs)) {
+			return std::stoi(lhs) < std::stoi(rhs);
+		} else {
+			return true;
+		}
+	}
+	if(is_positive_integer(rhs)) {
+		return false;
+	}
+	return lhs < rhs;
+}
+
 void location_palette::add_item(const std::string& id)
 {
 	int pos;
-	const auto itor = std::find(items_.begin(), items_.end(), id);
-	if (itor == items_.end()) {
-		items_.push_back(id);
-		pos = items_.size() - 1;
+	// 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) {
+		pos = std::distance(items_.begin(), items_.insert(itor, id));
 	}
 	else {
 		pos = std::distance(items_.begin(), itor);
 	}
 	selected_item_ = id;
-	items_start_ = std::max(pos - num_visible_items() + 1, items_start_);
-	items_start_ = std::min(pos, items_start_);
+	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_);
+	}
 	adjust_size(location());
 }