[Widelands-dev] [Merge] lp:~widelands-dev/widelands/allows_seafaring_performance into lp:widelands

2018-04-11 Thread noreply
The proposal to merge lp:~widelands-dev/widelands/allows_seafaring_performance 
into lp:widelands has been updated.

Status: Needs review => Merged

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/allows_seafaring_performance/+merge/342987
-- 
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/allows_seafaring_performance.

___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp


Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/allows_seafaring_performance into lp:widelands

2018-04-11 Thread Klaus Halfmann
Review: Approve review, compile, automated test

allows_seafaring_performance$ ./regression_test.py -b ./widelands 
...
test/maps/two_ponds.wmf/scripting/test_seafaring.lua ... 
  Running Widelands ... done.
  Loading savegame: port_spaces ... done.
ok
...
Ran 41 tests in 1204.108s

@bunnybot merge
-- 
https://code.launchpad.net/~widelands-dev/widelands/allows_seafaring_performance/+merge/342987
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/allows_seafaring_performance.

___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp


Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/allows_seafaring_performance into lp:widelands

2018-04-11 Thread GunChleoc
Thanks for the review :)

Can you trigger the merge once your testing is done?
-- 
https://code.launchpad.net/~widelands-dev/widelands/allows_seafaring_performance/+merge/342987
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/allows_seafaring_performance into lp:widelands.

___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp


Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/allows_seafaring_performance into lp:widelands

2018-04-11 Thread Klaus Halfmann
This is a prerequisite for the smaller_building_statistics branch.
checking this out now ...

Code is reasonable.

As this change carries its own testcase it can go in 
once the test works for me.
-- 
https://code.launchpad.net/~widelands-dev/widelands/allows_seafaring_performance/+merge/342987
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/allows_seafaring_performance into lp:widelands.

___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp


[Widelands-dev] [Merge] lp:~widelands-dev/widelands/allows_seafaring_performance into lp:widelands

2018-04-11 Thread bunnybot
Continuous integration builds have changed state:

Travis build 3369. State: passed. Details: 
https://travis-ci.org/widelands/widelands/builds/364989186.
Appveyor build 3175. State: success. Details: 
https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_allows_seafaring_performance-3175.
-- 
https://code.launchpad.net/~widelands-dev/widelands/allows_seafaring_performance/+merge/342987
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/allows_seafaring_performance into lp:widelands.

___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp


Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/smaller_building_statistics into lp:widelands

2018-04-11 Thread GunChleoc
Review: Resubmit

I know - I have taken care of it. Branch is ready; I'll put it up for review as 
soon as Launchpad has finished parsing it.

https://code.launchpad.net/~widelands-dev/widelands/allows_seafaring_performance
-- 
https://code.launchpad.net/~widelands-dev/widelands/smaller_building_statistics/+merge/342828
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/smaller_building_statistics.

___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp


[Widelands-dev] [Merge] lp:~widelands-dev/widelands/allows_seafaring_performance into lp:widelands

2018-04-11 Thread GunChleoc
GunChleoc has proposed merging 
lp:~widelands-dev/widelands/allows_seafaring_performance into lp:widelands.

Commit message:
Only recalculate whether a map allows seafaring when something has changed

- New function Map::recalculate_allows_seafaring()
- Map::allows_seafaring() is recalculated:
  - On map load
  - On editor save
  - When a port space is changed via Lua scripting
  - When it is triggered by LuaMap::recalculate_seafaring() or recalculate()
- Added test
- Some documentation tweaks to lua_map.cc.
- Fixed the check for invalid terrains in LuaField::get_terr.

Requested reviews:
  Widelands Developers (widelands-dev)

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/allows_seafaring_performance/+merge/342987

Only recalculate whether a map allows seafaring when something has changed. 
This new design is a bit more fragile, but I think the performance gain is 
worth it, because the AI needs this function.
-- 
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/allows_seafaring_performance into lp:widelands.
=== modified file 'src/logic/map.cc'
--- src/logic/map.cc	2018-04-07 16:59:00 +
+++ src/logic/map.cc	2018-04-11 07:11:49 +
@@ -70,7 +70,8 @@
  scenario_types_(NO_SCENARIO),
  width_(0),
  height_(0),
- pathfieldmgr_(new PathfieldManager) {
+ pathfieldmgr_(new PathfieldManager),
+	  allows_seafaring_(false) {
 }
 
 Map::~Map() {
@@ -157,6 +158,7 @@
 			f = get_fcoords(Coords(x, y));
 			recalc_nodecaps_pass2(world, f);
 		}
+	recalculate_allows_seafaring();
 }
 
 void Map::recalc_default_resources(const World& world) {
@@ -278,6 +280,7 @@
 	objectives_.clear();
 
 	port_spaces_.clear();
+	allows_seafaring_ = false;
 
 	// TODO(meitis): should be done here ... but WidelandsMapLoader::preload_map calls
 	// this cleanup AFTER assigning filesystem_ in WidelandsMapLoader::WidelandsMapLoader
@@ -1316,7 +1319,7 @@
 	return port_spaces_.count(c);
 }
 
-bool Map::set_port_space(const World& world, const Coords& c, bool set, bool force) {
+bool Map::set_port_space(const World& world, const Coords& c, bool set, bool force, bool recalculate_seafaring) {
 	bool success = false;
 	if (set) {
 		success = force || is_port_space_allowed(world, get_fcoords(c));
@@ -1327,6 +1330,9 @@
 		port_spaces_.erase(c);
 		success = true;
 	}
+	if (recalculate_seafaring) {
+		recalculate_allows_seafaring();
+	}
 	return success;
 }
 
@@ -1973,10 +1979,16 @@
 }
 
 bool Map::allows_seafaring() const {
+	return allows_seafaring_;
+}
+
+// This check can become very expensive, so we only recalculate this on relevant map changes.
+void Map::recalculate_allows_seafaring() {
 
 	// There need to be at least 2 port spaces for seafaring to make sense
 	if (get_port_spaces().size() < 2) {
-		return false;
+		allows_seafaring_ = false;
+		return;
 	}
 
 	std::set reachable_from_previous_ports;
@@ -2000,7 +2012,8 @@
 
 			// Found one
 			if (reachable_from_previous_ports.count(current_position) > 0) {
-return true;
+allows_seafaring_ = true;
+return;
 			}
 
 			// Adding the neighbors to the list
@@ -2021,7 +2034,7 @@
 			reachable_from_previous_ports.insert(reachable_coord);
 		}
 	}
-	return false;
+	allows_seafaring_ = false;
 }
 
 void Map::cleanup_port_spaces(const World& world) {
@@ -2031,6 +2044,7 @@
 			continue;
 		}
 	}
+	recalculate_allows_seafaring();
 }
 
 bool Map::has_artifacts() {

=== modified file 'src/logic/map.h'
--- src/logic/map.h	2018-04-07 16:59:00 +
+++ src/logic/map.h	2018-04-11 07:11:49 +
@@ -408,7 +408,8 @@
 	/***
 	 * Changes the given triangle's terrain. This happens in the editor and might
 	 * happen in the game too if some kind of land increasement is implemented (like
-	 * drying swamps). The nodecaps need to be recalculated
+	 * drying swamps). The nodecaps need to be recalculated. If terrain was changed from
+	 * or to water, we need to recalculate_allows_seafaring too, depending on the situation.
 	 *
 	 * @return the radius of changes.
 	 */
@@ -448,14 +449,16 @@
 	/// 'force' sets the port space even if it isn't viable, and is to be used for map loading only.
 	/// Returns whether the port space was set/unset successfully.
 	bool
-	set_port_space(const World& world, const Widelands::Coords& c, bool set, bool force = false);
+	set_port_space(const World& world, const Widelands::Coords& c, bool set, bool force = false, bool recalculate_seafaring = false);
 	const PortSpacesSet& get_port_spaces() const {
 		return port_spaces_;
 	}
 	std::vector find_portdock(const Widelands::Coords& c) const;
 
-	/// Check whether there are at least 2 port spaces that can be reached from each other by water
+	/// Return true if there are at least 2 port spaces that can be reached from each other by water
 	bool allows_seafaring() const;
+	/// Calculate whether there are at least 2 port spaces that can be reached from each other by water and set the allows_seafaring property
+	void