Handle unit overlays as modifications, allow [effect] to remove them

Possible fix for #4058, with the following logic:

* If non-empty, [unit]overlay= is handled by adding modifications
* unit::write will always output an empty overlay=
* The Lua API's get_units() will still provide the list of overlays
* [effect]apply_to=overlay can now remove as well as add overlays
* [remove_unit_overlay] is implemented with [effect]apply_to=overlay

Using [object]s with durations hasn't been tested, but expected effects:
* An expired add= followed by a non-expired remove= will simply cause the remove=
    to have no effect when std::remove(overlays_ ...) is called.
* A remove= followed by [remove_unit_overlay] cause the [remove_unit_overlay] to be a no-op,
    and the overlay will reappear when the first remove= expires. This edge case is already
	documented as unsupported on the wiki.

data/lua/wml-tags.lua

@@ -368,18 +368,20 @@ end
 
 function wml_actions.remove_unit_overlay(cfg)
 	local img = cfg.image or helper.wml_error( "[remove_unit_overlay] missing required image= attribute" )
-
-	-- Loop through all matching units.
 	for i,u in ipairs(wesnoth.get_units(cfg)) do
-		local ucfg = u.__cfg
+		local has_already = false
-		local t = utils.parenthetical_split(ucfg.overlays)
+		for i, w in ipairs(u.overlays) do
-		-- Remove the specified image from the overlays.
+			if w == img then has_already = true end
-		for i = #t,1,-1 do
+		end
-			if t[i] == img then table.remove(t, i) end
+		if has_already then
+			u:add_modification("object", {
+				id = cfg.object_id,
+				wml.tag.effect {
+					apply_to = "overlay",
+					remove = img,
+				}
+			})
 		end
-		-- Reassemble the list of remaining overlays.
-		ucfg.overlays = table.concat(t, ',')
-		wesnoth.put_unit(ucfg)
 	end
 end
 

data/schema/units/modifications.cfg

@@ -155,12 +155,18 @@
 				{LINK_TAG "units/unit_type/extra_anim"}
 			[/case]
 			[case]
-				value=image_mod,overlay
+				value=image_mod
 				{SIMPLE_KEY replace string}
 				{SIMPLE_KEY add string}
 				{LINK_TAG "game_config/color_range"}
 				{LINK_TAG "game_config/color_palette"}
 			[/case]
+			[case]
+				value=overlay
+				{SIMPLE_KEY replace string}
+				{SIMPLE_KEY add string}
+				{SIMPLE_KEY remove string}
+			[/case]
 			[case]
 				value=ellipse
 				{SIMPLE_KEY ellipse string}

src/units/unit.cpp

@@ -512,9 +512,15 @@ void unit::init(const config& cfg, bool use_traits, const vconfig* vcfg)
 	advance_to(*type_, use_traits);
 
 	if(const config::attribute_value* v = cfg.get("overlays")) {
-		overlays_ = utils::parenthetical_split(v->str(), ',');
+		auto overlays = utils::parenthetical_split(v->str(), ',');
-		if(overlays_.size() == 1 && overlays_.front().empty()) {
+		if(overlays.size() > 0) {
-			overlays_.clear();
+			deprecated_message("[unit]overlays", DEP_LEVEL::PREEMPTIVE, {1, 15, 0}, "Add overlays via objects instead.");
+			config effect;
+			config o;
+			effect["apply_to"] = "overlay";
+			effect["add"] = v->str();
+			o.add_child("effect", effect);
+			add_modification("object", o);
 		}
 	}
 
@@ -1496,7 +1502,10 @@ void unit::write(config& cfg, bool write_all) const
 	cfg.clear_children("events");
 	cfg.append(events_);
 
-	cfg["overlays"] = utils::join(overlays_);
+	// Overlays are exported as the modifications that add them, not as an overlays= value,
+	// however removing the key breaks the Gui Debug Tools.
+	// \todo does anything depend on the key's value, other than the re-import code in unit::init?
+	cfg["overlays"] = "";
 
 	cfg["name"] = name_;
 	cfg["id"] = id_;
@@ -2097,15 +2106,19 @@ void unit::apply_builtin_effect(std::string apply_to, const config& effect)
 	} else if(apply_to == "overlay") {
 		const std::string& add = effect["add"];
 		const std::string& replace = effect["replace"];
+		const std::string& remove = effect["remove"];
 
 		if(!add.empty()) {
-			std::vector<std::string> temp_overlays = utils::parenthetical_split(add, ',');
+			for(const auto& to_add : utils::parenthetical_split(add, ',')) {
-			std::vector<std::string>::iterator it;
+				overlays_.push_back(to_add);
-			for(it=temp_overlays.begin();it<temp_overlays.end();++it) {
-				overlays_.push_back(*it);
 			}
 		}
-		else if(!replace.empty()) {
+		if(!remove.empty()) {
+			for(const auto& to_remove : utils::parenthetical_split(remove, ',')) {
+				overlays_.erase(std::remove(overlays_.begin(), overlays_.end(), to_remove), overlays_.end());
+			}
+		}
+		if(add.empty() && remove.empty() && !replace.empty()) {
 			overlays_ = utils::parenthetical_split(replace, ',');
 		}
 	} else if(apply_to == "new_advancement") {