From 09906135b5604a372db967c41508a72fb881f814 Mon Sep 17 00:00:00 2001
From: Charles Dang <exodia339@gmail.com>
Date: Thu, 15 Feb 2018 14:39:41 +1100
Subject: [PATCH] Refactor wesnoth.set_village_owner implementation

@gfgtdf pointed out that I didn't actually fix the underlying porblem that was causing
#2505 in 3e9654c3e408: if new_side was 0, board().team_is_defeated(board().get_team(new_side))
would throw an out-of-range exception and the village would never be un-assigned from the
old side.

I'm totally refactored the function to be easier to understand and match the desired behavior.
---
 src/scripting/game_lua_kernel.cpp | 36 ++++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/src/scripting/game_lua_kernel.cpp b/src/scripting/game_lua_kernel.cpp
index 963229c0aef7..990f07342c59 100644
--- a/src/scripting/game_lua_kernel.cpp
+++ b/src/scripting/game_lua_kernel.cpp
@@ -1138,23 +1138,35 @@ int game_lua_kernel::intf_set_village_owner(lua_State *L)
 		return 0;
 	}
 
-	const int new_side = lua_isnoneornil(L, 2) ? 0 : luaL_checkinteger(L, 2);
-	const int old_side = board().village_owner(loc) + 1;
+	const int old_side_num = board().village_owner(loc) + 1;
+	const int new_side_num = lua_isnoneornil(L, 2) ? 0 : luaL_checkinteger(L, 2);
+
+	if(old_side_num == new_side_num) {
+		return 0;
+	}
+
+	team* old_side = nullptr;
+	team* new_side = nullptr;
 
 	try {
-		if(new_side == old_side || board().team_is_defeated(board().get_team(new_side))) {
-			return 0;
-		}
+		old_side = &board().get_team(old_side_num);
+	} catch(const std::out_of_range&) {
+		// old_side_num is invalid (which should really never happen), exit.
+		return 0;
+	}
 
-		if(old_side > 0) {
-			board().get_team(old_side).lose_village(loc);
-		}
+	old_side->lose_village(loc);
 
-		if(new_side > 0) {
-			board().get_team(new_side).get_village(loc, old_side, (luaW_toboolean(L, 4) ? &gamedata() : nullptr) );
-		}
+	try {
+		new_side = &board().get_team(new_side_num);
 	} catch(const std::out_of_range&) {
-		// new_side was invalid, or old_side was 0 (de-assign village).
+		// new_side_num is invalid, exit. This also covers the case where it equals 0,
+		// which is supposed to only un-assign the side (done above).
+		return 0;
+	}
+
+	if(!board().team_is_defeated(*new_side)) {
+		new_side->get_village(loc, old_side_num, (luaW_toboolean(L, 4) ? &gamedata() : nullptr));
 	}
 
 	return 0;