Give lua_unit a put_map member function

This allows us to remove the destructor-avoidance hack in wesnoth.put_unit
and simplify the function's handing of various lua_unit types.

src/scripting/lua.cpp

@@ -2123,17 +2123,8 @@ static int intf_put_unit(lua_State *L)
 		lu = static_cast<lua_unit *>(lua_touserdata(L, unit_arg));
 		u = lu->get();
 		if (!u) return luaL_argerror(L, unit_arg, "unit not found");
-		if (lu->on_map()) {
-			if (unit_arg == 1 || u->get_location() == loc) return 0;
-			resources::units->erase(loc);
-			resources::units->move(u->get_location(), loc);
+		if (lu->on_map() && (unit_arg == 1 || u->get_location() == loc)) {
 			return 0;
-		} else if (int side = lu->on_recall_list()) {
-			team &t = (*resources::teams)[side - 1];
-			unit *v = new unit(*u);
-			std::vector<unit> &rl = t.recall_list();
-			rl.erase(rl.begin() + (u - &rl[0]));
-			u = v;
 		}
 		if (unit_arg == 1) {
 			loc = u->get_location();
@@ -2154,23 +2145,12 @@ static int intf_put_unit(lua_State *L)
 	}
 
 	resources::screen->invalidate(loc);
-	resources::units->erase(loc);
 	if (!u) return 0;
 
 	if (lu) {
-		u->set_location(loc);
-		resources::units->insert(u);
-		size_t uid = u->underlying_id();
-		// There are 3 types of content for lua_unit:
-		// -Recall list: the unit is copied and erased from the recall list earlier in this function
-		// -On map: this code isn't reached
-		// -Pointer: we need to take ownership of the pointer
-		// If we run lua_unit's destructor, any pointers get deleted.
-		// So we don't run the destructor, and just overwrite the memory with a new object
-		// The reason for this is that the using resources::units->add constructs unneccessary copies
-		// Inserting pointers instead improves wesnoth.put_unit's speed by over 2 orders of magnitude from lua's perspective.
-		new(lu) lua_unit(uid);
+		lu->put_map(loc);
 	} else {
+		resources::units->erase(loc);
 		u->set_location(loc);
 		resources::units->insert(u);
 	}

src/scripting/lua_api.cpp

@@ -403,6 +403,57 @@ unit *lua_unit::get()
 	return &*ui;
 }
 
+// Having this function here not only simplifies other code, it allows us to move
+// pointers around from one structure to another.
+// This makes bare pointer->map in particular about 2 orders of magnitude faster,
+// as benchmarked from Lua code.
+bool lua_unit::put_map(const map_location &loc)
+{
+	if (ptr) {
+		ptr->set_location(loc);
+		resources::units->erase(loc);
+		std::pair<unit_map::unit_iterator, bool> res = resources::units->insert(ptr);
+		if (res.second) {
+			ptr = NULL;
+			uid = res.first->underlying_id();
+		} else {
+			ERR_LUA << "Could not move unit " << ptr->underlying_id() << " onto map location " << loc << '\n';
+			return false;
+		}
+	} else if (side) { // recall list
+		std::vector<unit> &recall_list = (*resources::teams)[side - 1].recall_list();
+		std::vector<unit>::iterator it = recall_list.begin();
+		for(; it != recall_list.end(); ++it) {
+			if (it->underlying_id() == uid) {
+				break;
+			}
+		}
+		if (it != recall_list.end()) {
+			side = 0;
+			// uid may be changed by unit_map on insertion
+			uid = resources::units->replace(loc, *it).first->underlying_id();
+			recall_list.erase(it);
+		} else {
+			ERR_LUA << "Could not find unit " << uid << " on recall list of side " << side << '\n';
+			return false;
+		}
+	} else { // on map
+		unit_map::unit_iterator ui = resources::units->find(uid);
+		if (ui != resources::units->end()) {
+			map_location from = ui->get_location();
+			if (from != loc) { // This check is redundant in current usage
+				resources::units->erase(loc);
+				resources::units->move(from, loc);
+			}
+			// No need to change our contents
+		} else {
+			ERR_LUA << "Could not find unit " << uid << " on the map\n";
+			return false;
+		}
+	}
+	return true;
+}
+
 unit *luaW_tounit(lua_State *L, int index, bool only_on_map)
 {
 	if (!luaW_hasmetatable(L, index, getunitKey)) return NULL;

src/scripting/lua_api.hpp

@@ -125,6 +125,8 @@ bool luaW_getglobal(lua_State *L, ...);
  */
 unit *luaW_checkunit(lua_State *L, int index, bool only_on_map = false);
 
+struct map_location;
+
 /**
  * Storage for a unit, either owned by the Lua code (#ptr != 0), on a
  * recall list (#side != 0), or on the map. Shared units are represented
@@ -145,6 +147,9 @@ class lua_unit
 	bool on_map() const { return !ptr && side == 0; }
 	int on_recall_list() const { return side; }
 	unit *get();
+
+	// Clobbers loc
+	bool put_map(const map_location &loc);
 };
 
 #endif