Cleaned up implementation of image::locator

- Used inline default values to reduce ctor mess
- Used a forwarded ctor for locator::value instead of duplicating them between the main class and the value class
- Removed trailing underscores from public struct members
- Removed const char* ctor to make locator use more explicit instead of relying on implicit conversions
- Moved parse_arguments to a value ctor since it's more appropriate there
- Added locator::clone to replace the ctor with optional modifications

src/addon/info.cpp

@@ -235,7 +235,7 @@ std::string addon_info::display_icon() const
 
 	if(ret.empty()) {
 		ERR_AC << "add-on '" << id << "' doesn't have an icon path set";
-	} else if(!image::exists(ret)) {
+	} else if(!image::exists(image::locator{ret})) {
 		ERR_AC << "add-on '" << id << "' has an icon which cannot be found: '" << ret << "'";
 	} else if(ret.find("units/") != std::string::npos && ret.find_first_of('~') == std::string::npos) {
 		// HACK: prevent magenta icons, because they look awful

src/display.cpp

@@ -2797,8 +2797,8 @@ void display::draw_hex(const map_location& loc)
 	}
 
 	if(debug_flag_set(DEBUG_FOREGROUND)) {
-		drawing_buffer_add(
-			LAYER_UNIT_DEFAULT, loc, [tex = image::get_texture("terrain/foreground.png", image::TOD_COLORED)](const rect& dest) {
+		drawing_buffer_add(LAYER_UNIT_DEFAULT, loc,
+			[tex = image::get_texture(image::locator{"terrain/foreground.png"}, image::TOD_COLORED)](const rect& dest) {
 				draw::blit(tex, dest);
 			});
 	}

src/editor/action/mouse/mouse_action_select.cpp

@@ -64,9 +64,9 @@ void mouse_action_select::set_mouse_overlay(editor_display& disp)
 {
 	texture tex;
 	if (has_shift_modifier()) {
-		tex = image::get_texture("editor/tool-overlay-select-wand.png");
+		tex = image::get_texture(image::locator{"editor/tool-overlay-select-wand.png"});
 	} else {
-		tex = image::get_texture("editor/tool-overlay-select-brush.png");
+		tex = image::get_texture(image::locator{"editor/tool-overlay-select-brush.png"});
 	}
 	disp.set_mouseover_hex_overlay(tex);
 }

src/editor/editor_display.cpp

@@ -81,7 +81,7 @@ void editor_display::draw_hex(const map_location& loc)
 
 	if(map().in_selection(loc)) {
 		drawing_buffer_add(LAYER_FOG_SHROUD, loc,
-			[tex = image::get_texture("editor/selection-overlay.png", image::TOD_COLORED)](const rect& d) {
+			[tex = image::get_texture(image::locator{"editor/selection-overlay.png"}, image::TOD_COLORED)](const rect& d) {
 				draw::blit(tex, d);
 			});
 	}

src/game_display.cpp

@@ -414,15 +414,15 @@ void game_display::draw_movement_info(const map_location& loc)
 			drawing_buffer_add(LAYER_MOVE_INFO, loc,
 				[inv = w->second.invisible, zoc = w->second.zoc, cap = w->second.capture](const rect& dest) {
 					if(inv) {
-						draw::blit(image::get_texture("misc/hidden.png", image::HEXED), dest);
+						draw::blit(image::get_texture(image::locator{"misc/hidden.png"}, image::HEXED), dest);
 					}
 
 					if(zoc) {
-						draw::blit(image::get_texture("misc/zoc.png", image::HEXED), dest);
+						draw::blit(image::get_texture(image::locator{"misc/zoc.png"}, image::HEXED), dest);
 					}
 
 					if(cap) {
-						draw::blit(image::get_texture("misc/capture.png", image::HEXED), dest);
+						draw::blit(image::get_texture(image::locator{"misc/capture.png"}, image::HEXED), dest);
 					}
 				});
 

src/game_launcher.cpp

@@ -339,7 +339,7 @@ bool game_launcher::init_video()
 	video::set_window_title(game_config::get_default_title_string());
 
 #if !(defined(__APPLE__))
-	surface icon(image::get_surface("icons/icon-game.png", image::UNSCALED));
+	surface icon(image::get_surface(image::locator{"icons/icon-game.png"}, image::UNSCALED));
 	if(icon != nullptr) {
 		video::set_window_icon(icon);
 	}

src/picture.cpp

@@ -58,18 +58,18 @@ struct std::hash<image::locator::value>
 {
 	std::size_t operator()(const image::locator::value& val) const
 	{
-		std::size_t hash = std::hash<unsigned>{}(val.type_);
+		std::size_t hash = std::hash<unsigned>{}(val.type);
 
-		if(val.type_ == image::locator::FILE || val.type_ == image::locator::SUB_FILE) {
-			boost::hash_combine(hash, val.filename_);
+		if(val.type == image::locator::FILE || val.type == image::locator::SUB_FILE) {
+			boost::hash_combine(hash, val.filename);
 		}
 
-		if(val.type_ == image::locator::SUB_FILE) {
-			boost::hash_combine(hash, val.loc_.x);
-			boost::hash_combine(hash, val.loc_.y);
-			boost::hash_combine(hash, val.center_x_);
-			boost::hash_combine(hash, val.center_y_);
-			boost::hash_combine(hash, val.modifications_);
+		if(val.type == image::locator::SUB_FILE) {
+			boost::hash_combine(hash, val.loc.x);
+			boost::hash_combine(hash, val.loc.y);
+			boost::hash_combine(hash, val.center_x);
+			boost::hash_combine(hash, val.center_y);
+			boost::hash_combine(hash, val.modifications);
 		}
 
 		return hash;
@@ -243,83 +243,17 @@ void flush_cache()
 	precached_dirs.clear();
 }
 
-void locator::parse_arguments()
-{
-	std::string& fn = val_.filename_;
-	if(fn.empty()) {
-		return;
-	}
-
-	if(boost::algorithm::starts_with(fn, data_uri_prefix)) {
-		parsed_data_URI parsed{fn};
-
-		if(!parsed.good) {
-			std::string_view view{ fn };
-			std::string_view stripped = view.substr(0, view.find(","));
-			ERR_IMG << "Invalid data URI: " << stripped;
-		}
-
-		val_.is_data_uri_ = true;
-	}
-
-	std::size_t markup_field = fn.find('~');
-
-	if(markup_field != std::string::npos) {
-		val_.type_ = SUB_FILE;
-		val_.modifications_ = fn.substr(markup_field, fn.size() - markup_field);
-		fn = fn.substr(0, markup_field);
-	}
-}
-
-locator::locator()
-	: val_()
-{
-}
-
-locator::locator(const locator& a, const std::string& mods)
-	: val_(a.val_)
+
+
+locator locator::clone(const std::string& mods) const
 {
+	locator res = *this;
 	if(!mods.empty()) {
-		val_.modifications_ += mods;
-		val_.type_ = SUB_FILE;
+		res.val_.modifications += mods;
+		res.val_.type = SUB_FILE;
 	}
-}
 
-locator::locator(const char* filename)
-	: val_(filename)
-{
-	parse_arguments();
-}
-
-locator::locator(const std::string& filename)
-	: val_(filename)
-{
-	parse_arguments();
-}
-
-locator::locator(const std::string& filename, const std::string& modifications)
-	: val_(filename, modifications)
-{
-}
-
-locator::locator(const char* filename, const char* modifications)
-	: val_(filename, modifications)
-{
-}
-
-locator::locator(const std::string& filename,
-		const map_location& loc,
-		int center_x,
-		int center_y,
-		const std::string& modifications)
-	: val_(filename, loc, center_x, center_y, modifications)
-{
-}
-
-locator& locator::operator=(const locator& a)
-{
-	val_ = a.val_;
-	return *this;
+	return res;
 }
 
 std::ostream& operator<<(std::ostream& s, const locator& l)
@@ -334,85 +268,62 @@ std::ostream& operator<<(std::ostream& s, const locator& l)
 	return s;
 }
 
-locator::value::value()
-	: type_(NONE)
-	, is_data_uri_(false)
-	, filename_()
-	, loc_()
-	, modifications_()
-	, center_x_(0)
-	, center_y_(0)
+locator::value::value(const std::string& fn)
+	: type(FILE)
+	, filename(fn)
 {
-}
+	if(filename.empty()) {
+		return;
+	}
 
-locator::value::value(const char* filename)
-	: type_(FILE)
-	, is_data_uri_(false)
-	, filename_(filename)
-	, loc_()
-	, modifications_()
-	, center_x_(0)
-	, center_y_(0)
-{
-}
+	if(boost::algorithm::starts_with(filename, data_uri_prefix)) {
+		if(parsed_data_URI parsed{ filename }; !parsed.good) {
+			std::string_view view{ filename };
+			std::string_view stripped = view.substr(0, view.find(","));
+			ERR_IMG << "Invalid data URI: " << stripped;
+		}
 
-locator::value::value(const std::string& filename)
-	: type_(FILE)
-	, is_data_uri_(false)
-	, filename_(filename)
-	, loc_()
-	, modifications_()
-	, center_x_(0)
-	, center_y_(0)
-{
+		is_data_uri = true;
+	}
+
+	if(const std::size_t markup_field = filename.find('~'); markup_field != std::string::npos) {
+		type = SUB_FILE;
+		modifications = filename.substr(markup_field, filename.size() - markup_field);
+		filename = filename.substr(0, markup_field);
+	}
 }
 
 locator::value::value(const std::string& filename, const std::string& modifications)
-	: type_(SUB_FILE)
-	, is_data_uri_(false)
-	, filename_(filename)
-	, loc_()
-	, modifications_(modifications)
-	, center_x_(0)
-	, center_y_(0)
+	: type(SUB_FILE)
+	, filename(filename)
+	, modifications(modifications)
 {
 }
 
-locator::value::value(const char* filename, const char* modifications)
-	: type_(FILE)
-	, is_data_uri_(false)
-	, filename_(filename)
-	, loc_()
-	, modifications_(modifications)
-	, center_x_(0)
-	, center_y_(0)
-{
-}
-
-locator::value::value(const std::string& filename,
+locator::value::value(
+		const std::string& filename,
 		const map_location& loc,
 		int center_x,
 		int center_y,
 		const std::string& modifications)
-	: type_(SUB_FILE)
-	, is_data_uri_(false)
-	, filename_(filename)
-	, loc_(loc)
-	, modifications_(modifications)
-	, center_x_(center_x)
-	, center_y_(center_y)
+	: type(SUB_FILE)
+	, filename(filename)
+	, modifications(modifications)
+	, loc(loc)
+	, center_x(center_x)
+	, center_y(center_y)
 {
 }
 
 bool locator::value::operator==(const value& a) const
 {
-	if(a.type_ != type_) {
+	if(a.type != type) {
 		return false;
-	} else if(type_ == FILE) {
-		return filename_ == a.filename_;
-	} else if(type_ == SUB_FILE) {
-		return std::tie(filename_, loc_, modifications_, center_x_, center_y_) ==
-			std::tie(a.filename_, a.loc_, a.modifications_, a.center_x_, a.center_y_);
+	} else if(type == FILE) {
+		return filename == a.filename;
+	} else if(type == SUB_FILE) {
+		return std::tie(filename, loc, modifications, center_x, center_y) ==
+			std::tie(a.filename, a.loc, a.modifications, a.center_x, a.center_y);
 	}
 
 	return false;
@@ -420,13 +331,13 @@ bool locator::value::operator==(const value& a) const
 
 bool locator::value::operator<(const value& a) const
 {
-	if(type_ != a.type_) {
-		return type_ < a.type_;
-	} else if(type_ == FILE) {
-		return filename_ < a.filename_;
-	} else if(type_ == SUB_FILE) {
-		return std::tie(filename_, loc_, modifications_, center_x_, center_y_) <
-			std::tie(a.filename_, a.loc_, a.modifications_, a.center_x_, a.center_y_);
+	if(type != a.type) {
+		return type < a.type;
+	} else if(type == FILE) {
+		return filename < a.filename;
+	} else if(type == SUB_FILE) {
+		return std::tie(filename, loc, modifications, center_x, center_y) <
+			std::tie(a.filename, a.loc, a.modifications, a.center_x, a.center_y);
 	}
 
 	return false;
@@ -665,9 +576,9 @@ static surface apply_light(surface surf, const light_string& ls)
 
 bool locator::file_exists() const
 {
-	return val_.is_data_uri_
-		? parsed_data_URI{val_.filename_}.good
-		: !filesystem::get_binary_file_location("images", val_.filename_).empty();
+	return val_.is_data_uri
+		? parsed_data_URI{val_.filename}.good
+		: !filesystem::get_binary_file_location("images", val_.filename).empty();
 }
 
 static surface load_from_disk(const locator& loc)

src/picture.hpp

@@ -65,31 +65,36 @@ class locator
 public:
 	enum type { NONE, FILE, SUB_FILE };
 
-	locator();
-	locator(const locator& a, const std::string& mods = "");
-	locator(const char* filename);
-	locator(const std::string& filename);
-	locator(const char* filename, const char* modifications);
-	locator(const std::string& filename, const std::string& modifications);
-	locator(const std::string& filename, const map_location& loc, int center_x, int center_y, const std::string& modifications = "");
+	locator() = default;
+	locator(locator&&) noexcept = default;
+	locator(const locator&) = default;
 
-	locator& operator=(const locator& a);
+	template<typename... Args>
+	locator(Args&&... args) : val_(std::forward<Args>(args)...)
+	{
+	}
+
+	locator& operator=(const locator& a) = default;
+	locator& operator=(locator&&) = default;
+
+	/** Returns a copy of this locator with the given IPF */
+	locator clone(const std::string& mods) const;
 
 	bool operator==(const locator& a) const { return val_ == a.val_; }
 	bool operator!=(const locator& a) const { return !operator==(a); }
 
-	const std::string& get_filename() const { return val_.filename_; }
-	bool is_data_uri() const { return val_.is_data_uri_; }
-	const map_location& get_loc() const { return val_.loc_ ; }
-	int get_center_x() const { return val_.center_x_; }
-	int get_center_y() const { return val_.center_y_; }
-	const std::string& get_modifications() const {return val_.modifications_;}
-	type get_type() const { return val_.type_; }
+	const std::string& get_filename() const { return val_.filename; }
+	bool is_data_uri() const { return val_.is_data_uri; }
+	const map_location& get_loc() const { return val_.loc ; }
+	int get_center_x() const { return val_.center_x; }
+	int get_center_y() const { return val_.center_y; }
+	const std::string& get_modifications() const { return val_.modifications; }
+	type get_type() const { return val_.type; }
 
 	/**
 	 * Returns @a true if the locator does not correspond to an actual image.
 	 */
-	bool is_void() const { return val_.type_ == NONE; }
+	bool is_void() const { return val_.type == NONE; }
 
 	/**
 	 * Tests whether the file the locator points at exists.
@@ -120,27 +125,24 @@ public:
 	void add_to_cache(cache_type<T>& cache, T data) const;
 
 private:
-	void parse_arguments();
-
 	struct value
 	{
-		value();
-		value(const char *filename);
+		value() = default;
+
 		value(const std::string& filename);
-		value(const char *filename, const char* modifications);
 		value(const std::string& filename, const std::string& modifications);
-		value(const std::string& filename, const map_location& loc, int center_x, int center_y, const std::string& modifications);
+		value(const std::string& filename, const map_location& loc, int center_x, int center_y, const std::string& modifications = "");
 
 		bool operator==(const value& a) const;
 		bool operator<(const value& a) const;
 
-		type type_;
-		bool is_data_uri_;
-		std::string filename_;
-		map_location loc_;
-		std::string modifications_;
-		int center_x_;
-		int center_y_;
+		locator::type type = NONE;
+		bool is_data_uri = false;
+		std::string filename{};
+		std::string modifications{};
+		map_location loc{};
+		int center_x = 0;
+		int center_y = 0;
 	};
 
 	value val_;

src/units/frame.cpp

@@ -600,11 +600,11 @@ void unit_frame::redraw(const int frame_time, bool on_start_time, bool in_scope_
 
 	image::locator image_loc;
 	if(direction != map_location::NORTH && direction != map_location::SOUTH) {
-		image_loc = image::locator(current_data.image_diagonal, current_data.image_mod);
+		image_loc = current_data.image_diagonal.clone(current_data.image_mod);
 	}
 
 	if(image_loc.is_void() || image_loc.get_filename().empty()) { // invalid diag image, or not diagonal
-		image_loc = image::locator(current_data.image, current_data.image_mod);
+		image_loc = current_data.image.clone(current_data.image_mod);
 	}
 
 	point image_size {0, 0};
@@ -753,11 +753,11 @@ std::set<map_location> unit_frame::get_overlaped_hex(const int frame_time, const
 
 	image::locator image_loc;
 	if(direction != map_location::NORTH && direction != map_location::SOUTH) {
-		image_loc = image::locator(current_data.image_diagonal, current_data.image_mod);
+		image_loc = current_data.image_diagonal.clone(current_data.image_mod);
 	}
 
 	if(image_loc.is_void() || image_loc.get_filename().empty()) { // invalid diag image, or not diagonal
-		image_loc = image::locator(current_data.image, current_data.image_mod);
+		image_loc = current_data.image.clone(current_data.image_mod);
 	}
 
 	// We always invalidate our own hex because we need to be called at redraw time even

src/whiteboard/suppose_dead.cpp

@@ -145,7 +145,7 @@ void suppose_dead::draw_hex(const map_location& hex)
 	const display::drawing_layer layer = display::LAYER_ARROWS;
 
 	display::get_singleton()->drawing_buffer_add(
-		layer, loc_, [tex = image::get_texture("whiteboard/suppose_dead.png", image::HEXED)](const rect& d) {
+		layer, loc_, [tex = image::get_texture(image::locator{"whiteboard/suppose_dead.png"}, image::HEXED)](const rect& d) {
 			draw::blit(tex, d);
 		});
 }

src/widgets/menu.cpp

@@ -913,8 +913,9 @@ void menu::draw_row(const std::size_t row_index, const SDL_Rect& loc, ROW_TYPE t
 					(type == HEADING_ROW ? xpos+padding : xpos), y);
 
 				if(type == HEADING_ROW && sortby_ == int(i)) {
-					const texture sort_tex(image::get_texture(sortreversed_ ? "buttons/sliders/slider_arrow_blue.png" :
-					                                   "buttons/sliders/slider_arrow_blue.png~ROTATE(180)"));
+					const texture sort_tex(image::get_texture(
+						image::locator{sortreversed_ ? "buttons/sliders/slider_arrow_blue.png"
+													 : "buttons/sliders/slider_arrow_blue.png~ROTATE(180)"}));
 					if(sort_tex && sort_tex.w() <= widths[i] && sort_tex.h() <= loc.h) {
 						const int sort_x = xpos + widths[i] - sort_tex.w() - padding;
 						const int sort_y = loc.y + loc.h/2 - sort_tex.h()/2;