Merge lp:~widelands-dev/widelands/quarry_tweaks into lp:widelands

[TiborB]

Chanching syntax of quarries conf files (atlanteans and barbarians) to be like empire's one.

This is a peformance tweak - to insert a sleep time after failed search for stones.

And it breaks the savefile compatibility, it seems :)

[SirVer (sirver)]

lgtm, but I do not think that the productivity still drops fast now, does it? I do not think that is necessary a really bad problem, I think consistency is more important.

review: Approve

[wl-zocker (wl-zocker)]

I have tested it in game. The productivity does not drop as fast as before (5%/5ms, IIRC), but still rather quickly (about 5%/1s). "This order" refers to the fact that the sleep command comes after the worker=cut and is therefore not executed when the first command failes. I changed the order for all buildings to avoid busy waiting, except for the quarries, hence the comment. IMHO it is still valid to keep it.

Where does the additional timeout come from? Based on the conf files, I'd say the logic stays the same, it has only become more verbose.

[TiborB (tiborb95)]

My concern is not productivity drop, but too frequent calling run_findobject() function that is quite expensive, especially if it is called around without interruption.

I dont undestand the timing - I noticed there is 10 seconds of gap instead of 25 as I would expect, but for my purposes (performance) this does not make much difference. But 10 seconds is mentioned in stonemanson's conf file.

[wl-zocker (wl-zocker)]

The 10 seconds is how long the stonemason is hacking on the rocks (I did not measure that time). After having returned, he sleeps 25 seconds, that is what I have measured (5s at 5x speed).

I know your goal was to reduce the CPU consumption. I have done no profiling to see if that has reduced, but I guess whenever findobject fails, the productivity drops by 5%, so those two are related. My comment was mainly an answer to SirVer's diff comment.

[TiborB (tiborb95)]

I think things there do not work as supposed, but I have no time to tinker with this.

10 seconds period means that statistics should be updated once in 10 seconds and decreased by 5% - I believe...

=== modified file 'tribes/atlanteans/quarry/conf'
--- tribes/atlanteans/quarry/conf	2014-08-02 19:55:23 +0000
+++ tribes/atlanteans/quarry/conf	2015-03-30 18:56:08 +0000
@@ -16,13 +16,19 @@
 stonecutter=1
 
 [programs]
+# TRANSLATORS: "Completed/Skipped/Did not start quarrying stone because ...
+mine_stone=_quarrying stone
 # TRANSLATORS: "Completed/Skipped/Did not start working because ...
 work=_working
 
-[work]
+[mine_stone]
 worker=cut_stone  # This order is on purpose so that the productivity
 sleep=25000       # drops fast once all stones are gone.
 
+[work]
+call=mine_stone
+return=skipped
+
 [out_of_resource_notification]
 title=_Out of Rocks
 message=_The stonecutter working at this quarry can’t find any rocks in his working radius.

=== modified file 'tribes/barbarians/quarry/conf'
--- tribes/barbarians/quarry/conf	2015-02-12 19:46:19 +0000
+++ tribes/barbarians/quarry/conf	2015-03-30 18:56:08 +0000
@@ -15,12 +15,18 @@
 stonemason=1
 
 [programs]
+# TRANSLATORS: "Completed/Skipped/Did not start quarrying stone because ...
+mine_stone=_quarrying stone
 # TRANSLATORS: "Completed/Skipped/Did not start working because ...
 work=_working
 
+[mine_stone]
+worker=cut  # This order is on purpose so that the productivity
+sleep=25000       # drops fast once all stones are gone.
+
 [work]
-worker=cut   # This order is on purpose so that the productivity
-sleep=25000  # drops fast once all stones are gone.
+call=mine_stone
+return=skipped
 
 [out_of_resource_notification]
 title=_Out of Rocks