Merge lp:~widelands-dev/widelands/bug-1675179-lua-hide-fields into lp:widelands

[GunChleoc]

Added option to Lua function player:hide_fields to mark them as unexplored.

New Lua function for the Barbarians 3 scenario.

For testing, one can change the Empire 1 scenario like this:

 -- Show the sea
   p1:reveal_fields(sea:region(6))
   local ship = p1:place_ship(sea)
   sleep(1000)
   p1:reveal_fields(wl.Game().map.player_slots[1].starting_field:region(50))
   sleep(4000)
   p1:hide_fields(wl.Game().map.player_slots[1].starting_field:region(50), true)
   campaign_message_box(diary_page_2)
   -- Hide the sea after 5 seconds

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 2062. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/214787145.
Appveyor build 1897. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1675179_lua_hide_fields-1897.

[kaputtnik (franku)]

I played a bit with this new feature and it works great :-)

One could hide now also the starting position and reveal it after the map is scrolled. An imagination of blending in and out different scenes. A nice to have would be to make the blending more smooth, but a lua function that hide/relveal one of the fields of a region clockwise until the complete region is hidden/revealed could be implemented now.

review: Approve (testing)

[GunChleoc (gunchleoc)]

You code doesn't support blending at all at the moment - you see it, or you don't.

The clockwise reveal is a nice alternative idea and could be implemented entirely in Lua. Or maybe hide from the outside in and reveal from the inside out.

[SirVer (sirver)]

Some thoughts inlined.

review: Needs Fixing

[GunChleoc (gunchleoc)]

Good idea about the enum class and the variable name, I will change that :)

I don't think that we will have desyncs here though:

- It's an additional option to an already existing function, so we should have seen desyncs before if this was a problem
- It only affects the current player, other players' and critters' states shouldn't care about what I see
- We would have functions like Bob::set_position and Building:set_seeing trigger a playercommand down the line just for their current vision - all their state changes should have been handled already at that point

Feel free to convince me otherwise, I'm not that familiar with all the networking stuff.

[SirVer (sirver)]

- It only affects the current player, other players' and critters' states shouldn't care about what I see

afaik the only thing that does look at your current vision is the scout. And if you have a different vision table than some other host on the game around the area of the scout, the scout will make different decisions and desync the game.

I think this branch does not change the dangerous around this (as you said), but the feature could be used to potentially desync multiplayer scenarios if some reveal code only runs on one of the networked machines. I think this can be ignored for now.

lgtm.

@bunnybot merge

review: Approve

=== modified file 'src/logic/player.cc'
--- src/logic/player.cc	2017-03-03 18:13:55 +0000
+++ src/logic/player.cc	2017-04-20 08:41:02 +0000
@@ -1024,9 +1024,10 @@
 	field.vision = fvision;
 }
 
-void Player::unsee_node(MapIndex const i, Time const gametime, bool const forward) {
+/// If 'mode' = UnseeMode::kUnexplore, fields will be marked as unexplored. Else, player no longer sees what's currently going on.
+void Player::unsee_node(MapIndex const i, Time const gametime, const UnseeNodeMode mode,  bool const forward) {
 	Field& field = fields_[i];
-	if (field.vision <= 1)  //  Already does not see this
+	if ((mode == UnseeNodeMode::kUnsee && field.vision <= 1) || field.vision < 1)  //  Already does not see this
 		return;
 
 	//  If this is not already a forwarded call, we should inform allied players
@@ -1035,13 +1036,18 @@
 		update_team_players();
 	if (!forward && !team_player_.empty()) {
 		for (uint8_t j = 0; j < team_player_.size(); ++j)
-			team_player_[j]->unsee_node(i, gametime, true);
+			team_player_[j]->unsee_node(i, gametime, mode, true);
 	}
 
-	--field.vision;
-	if (field.vision == 1)
+	if (mode == UnseeNodeMode::kUnexplore) {
+		field.vision = 0;
+	} else {
+		--field.vision;
+		assert(1 <= field.vision);
+	}
+	if (field.vision < 2) {
 		field.time_node_last_unseen = gametime;
-	assert(1 <= field.vision);
+	}
 }
 
 /**

=== modified file 'src/logic/player.h'
--- src/logic/player.h	2017-01-25 18:55:59 +0000
+++ src/logic/player.h	2017-04-20 08:41:02 +0000
@@ -445,7 +445,11 @@
 	              const bool forward = false);
 
 	/// Decrement this player's vision for a node.
-	void unsee_node(const MapIndex, const Time, const bool forward = false);
+	enum class UnseeNodeMode {
+		kUnsee,
+		kUnexplore
+	};
+	void unsee_node(const MapIndex, const Time, UnseeNodeMode mode = UnseeNodeMode::kUnsee, const bool forward = false);
 
 	/// Call see_node for each node in the area.
 	void see_area(const Area<FCoords>& area) {

=== modified file 'src/scripting/lua_game.cc'
--- src/scripting/lua_game.cc	2017-01-25 18:55:59 +0000
+++ src/scripting/lua_game.cc	2017-04-20 08:41:02 +0000
@@ -620,6 +620,9 @@
       :arg fields: The fields to hide
       :type fields: :class:`array` of :class:`wl.map.Fields`
 
+      :arg hide_completely: *Optional*. The fields will be marked as unexplored.
+      :type hide_completely: :class:`boolean`
+
       :returns: :const:`nil`
 */
 int LuaPlayer::hide_fields(lua_State* L) {
@@ -628,11 +631,12 @@
 	Map& m = egbase.map();
 
 	luaL_checktype(L, 2, LUA_TTABLE);
+	const bool mode = !lua_isnone(L, 3) && luaL_checkboolean(L, 3);
 
 	lua_pushnil(L); /* first key */
 	while (lua_next(L, 2) != 0) {
-		p.unsee_node(
-		   (*get_user_class<LuaField>(L, -1))->fcoords(L).field - &m[0], egbase.get_gametime());
+		p.unsee_node((*get_user_class<LuaField>(L, -1))->fcoords(L).field - &m[0],
+		             egbase.get_gametime(), mode ? Player::UnseeNodeMode::kUnexplore : Player::UnseeNodeMode::kUnsee);
 		lua_pop(L, 1);
 	}
 

=== modified file 'test/maps/lua_testsuite.wmf/scripting/gplayer.lua'
--- test/maps/lua_testsuite.wmf/scripting/gplayer.lua	2017-01-17 20:07:46 +0000
+++ test/maps/lua_testsuite.wmf/scripting/gplayer.lua	2017-04-20 08:41:02 +0000
@@ -59,6 +59,30 @@
    assert_equal(true, player1:sees_field(self.f))
 end
 
+-- This test must go last, because we change the state of player1:seen_field.
+function player_vision_tests:test_hide_completely()
+   player1.see_all = true
+   assert_equal(true, player1:sees_field(self.f))
+   player1.see_all = false
+   assert_equal(false, player1:sees_field(self.f))
+
+   player1:hide_fields(self.f:region(1), false)
+   game.desired_speed = 0;
+   assert_equal(false, player1:sees_field(self.f))
+   assert_equal(true, player1:seen_field(self.f))
+
+   player1.see_all = true
+   assert_equal(true, player1:sees_field(self.f))
+   player1.see_all = false
+   assert_equal(false, player1:sees_field(self.f))
+
+   player1:hide_fields(self.f:region(1), true)
+   game.desired_speed = 0;
+   assert_equal(false, player1:sees_field(self.f))
+   assert_equal(false, player1:seen_field(self.f))
+   player1:reveal_fields(self.f:region(1))
+end
+
 
 -- =========================
 -- Forbid & Allow buildings