Merge lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

Proposed by Toni Förster
Status: Merged
Merged at revision: 8993
Proposed branch: lp:~widelands-dev/widelands/bug-1810062-territorial-calculations
Merge into: lp:widelands
Diff against target: 962 lines (+428/-112)
16 files modified
data/scripting/win_conditions/artifacts.lua (+17/-16)
data/scripting/win_conditions/territorial_functions.lua (+0/-23)
data/scripting/win_conditions/territorial_lord.lua (+8/-3)
data/scripting/win_conditions/territorial_time.lua (+6/-3)
data/scripting/win_conditions/wood_gnome.lua (+9/-15)
src/logic/field.h (+6/-2)
src/logic/game.cc (+19/-3)
src/logic/game.h (+5/-0)
src/logic/map.cc (+111/-3)
src/logic/map.h (+6/-0)
src/logic/replay.cc (+0/-1)
src/scripting/lua_map.cc (+149/-38)
src/scripting/lua_map.h (+4/-0)
src/wui/interactive_gamebase.cc (+2/-0)
test/maps/lua_testsuite.wmf/scripting/cfield.lua (+78/-5)
test/maps/two_ponds.wmf/scripting/test_seafaring.lua (+8/-0)
To merge this branch: bzr merge lp:~widelands-dev/widelands/bug-1810062-territorial-calculations
Reviewer Review Type Date Requested Status
hessenfarmer Approve
GunChleoc Approve
Toni Förster testing Approve
kaputtnik (community) testing Approve
Review via email: mp+361366@code.launchpad.net

Commit message

Refactor initialization of win conditions:

- Separate optional "init" coroutine that is executed on the loading
  screen
- Shift calculations for territorial functions and Wood Gnome to C++
  to improve performance
- Add immovables to the table for territory calculation
- Expose port spaces and max caps to Lua interface
- Remove superfluous "postload" call from replay

Description of the change

This adds fields covered by trees, stones, artifacts and buildings to calculated territory.

To post a comment you must log in.
Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4379. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/474645761.
Appveyor build 4171. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1810062_territorial_calculations-4171.

8957. By Toni Förster

- rename function to get_valuable_fields()
- buildable fields and immovables are added to table
- for all other fields this is evaluated by whether they are reachable

Revision history for this message
Toni Förster (stonerl) wrote :

This revision should also fix Galcier Lake and other maps.

Here is the idea: Fields that can have a small, medium, or big building on it will automatically be added. Big includes ports. Also fields that currently have immovables on it, like stones & trees are added as well, since they might cover fields that have one of the aforementioned building spaces.

For all other walkable fields, this includes fields with flags and mines, we evaluate whether they can be reached by certain types of buildings. If reachable within 6 fields by small, medium & big it is added since it can be reached at least by the smallest military building.

If within 8 field a medium and big building is buildable then it will be added as well. Also if within 12 fields a big building is buildable.

The calculation is done in x y, x -y, -x y & -x -y direction. On minor issue is that if we reach 0 on the xy-axes or the xy maximum the evaluation stops. Instead of starting by at the coordinate 1 or counting backwards from xy-max.

Here are the 100% that are calculated for different implementations on the Galcier Lake map, with the implementation in the merge request the size is reduced by ~30%.

6713: current implementation
4718: when counting up 12 fields from the field in question
4715: when counting up 8 fields from the field in question
4702: when counting up 6 fields from the field in question

For Oasis Triangle the size it reduced by 75%:
36064: current implementation
12646: when counting up 12 fields from the field in question
12612: when counting up 8 fields from the field in question
12490: when counting up 6 fields from the field in question

With the current merge request, up to 12 fields are checked. The other cases can be tested by commenting the elseifs out. As I said the before: there might be some shortcomings, but they are be negligible in my opinion.

Glacier Lake & Oasis Triangle seems playable when looking at the numbers. I also run a quick simulation with AIs and it seems to be fine. So please test as thoroughly as possible.

We could also only check for 6 fields to be on the safe side. But by looking at the numbers it is just a small difference between the 12 and 6 fields calculation (1.3% on Oasis Triangle & 0.3% on Glacier Lake) so it should be find to test for the whole 12 fields.

8958. By Toni Förster

add 2 if-clauses to avoid unnecessary calculations

8959. By Toni Förster

test immovables for reachabilty

Revision history for this message
Toni Förster (stonerl) wrote :

A minor change. We don't assume that immovables can be added without checks. We need to test whether a field can be reached. This reduces the available size on Oasis from 12646 to 12395 fields and on Glacier from 4718 to 4697. We might miss some fields though...

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4382. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/475124699.
Appveyor build 4174. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1810062_territorial_calculations-4174.

8960. By Toni Förster

cleaned up comments

8961. By Toni Förster

- added evaluate_surrounding_fields()
- this calculates for all 4 quadrants even when hitting the map boarders

Revision history for this message
Toni Förster (stonerl) wrote :

Revision 8961 should be more accurate no that hitting the map boarders don't stop the calculation. The number of fields have increased.

Glacier Lake might be on the edge of being playable in territorial. Also calculations might take some time "The Oasis Triangle" took about 30 seconds on my system, since it has so man empty fields.

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4384. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/475608975.
Appveyor build 4176. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1810062_territorial_calculations-4176.

Revision history for this message
kaputtnik (franku) wrote :

30 seconds is definitely to long.

What about the idea mentioned in https://bugs.launchpad.net/widelands/+bug/1622307/comments/2 ? So we have a check in c++, which should be faster than lua. We can add an additional tag in the elemental file like we have for 'seafaring': When the map is saved, it will be checked for possibility to play territorial and the tag is added. This way we can also have a filter on the "Choose map" page, and exclude the winconditions from the list in the 'Launch Game' page. In my theory ;)

I think the map 'The Nile' is also affected.

Revision history for this message
Toni Förster (stonerl) wrote :

The checks here not only "minimize" a map. They also increase it. The map Astoria 2.R goes from 37602 to 42610 fields. 12% more fields that were covered by trees and stones before and weren't counted. Calculation time here is not really noticeable.

The Map the Nile already can be played in territorial. With the current implementation it has 56801 fields in total, and 55325 fields with this revision. This map is quite well designed. But calculations also take some time, because it has allot of mines and flag-only fields.

Regarding your idea for territorial checks in the Editor. I think we don't need that flag. Instead, the Editor should calculate the amount of fields for territorial and store this table in the map file. When starting a new game we then just need to load the table instead of calculation the fields.

(But this is beyond my current abilities, sadly)

If someone has a neat idea on how to speed this code here up... I guess math would help :-)

Revision history for this message
kaputtnik (franku) wrote :

> Instead, the Editor should calculate the amount of fields for territorial and store this table in the map file. When starting a new game we then just need to load the table instead of calculation the fields.

+1 :-)

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

+1 for calculating these amounts in the editor but we should have this code as well to provide backwards compatibility for old maps without having to reedit them in the mapeditor.

as far as I understand this code the algorithm isn't exactly correct as it checks a quadratic pattern whether a building can be build there to calculate accesability but the conquer function is circular

so for the following pattern the o's will be falsely declared positive possibilities to reach the center field (c) with a conquer radius of 1.

  o x x o
   x c x
  o x x o

Revision history for this message
kaputtnik (franku) wrote :

> but we should have this code as well to provide backwards compatibility for old maps without having to reedit them in the mapeditor.

From my understanding the code for calculating the fields a player could reach is executed automatically whenever a map is saved. So loading and resaving all shipped maps would do the trick and there is no need to edit a map at all. Except external maps, like we have on the website.

Not sure what you meant with 'backwards compatibility'.

Anyway, we should discuss this on the bug report.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

in my installation the vast majority of available maps are such external maps (including S2 maps)

I explained this also in the bug.

While thinking about it I came along a possible solution for the long calculation time.
It might help to have the calculation in a parallel thread (e.g. running automatically after including theterritorial functions) delivering a global variable Valuable fields. Only thing we need to consider then is not calling the variable too early in the game (i.e. sending a status message) This can be achieved by sleeping the main loop probably.

By this we can start playing while the calculation is done in the background.

Revision history for this message
Toni Förster (stonerl) wrote :

>It might help to have the calculation in a parallel thread

This might help indeed. But first of all I try to get the current code to perform as fast as possible. As you pointed out, currently squares are calculated. Also I just realized that some coordinates are calculated twice. Getting rid of these unnecessary calculations would free allot of time (roughly up to 100 calculations per field).

I do have another idea in mind on how to avoid some more calculations, but these are only vague ideas.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

some more ideas to speed it up:
1. use the region lua command instead of going through the quadrant coordinates.
a) first it is working in a circular manner which reflects better the conquer radius.
b) Second it will not evaluate the superfluous corner fields (which are lot in a radius 12 case).
c) Third it can be used in tranches from the inside to the outside using r1 and r2 which will allow to break the calculation as soon as a nearby suitable building will be found.

2. we might reconsider which field is valuable (in my perspective just walkable fields are not valuable, only if there can be build a flag/road) which might reduce the number of fields to be calculated in total.

some problems of the algorithm to be suitable for every map:
1. taking seafaring into account currently all small islands will be counted as long as there are two building spots, although they won't be accessible due to no port.
2. same is for hidden valleys in the mountains or similar.

one question:
are you sure that an immovable on the map will hide a caps. in my understanding of the documentation f.immovable is independent of has_caps.

Revision history for this message
Toni Förster (stonerl) wrote :

>one question:
>are you sure that an immovable on the map will hide a caps. in my
>understanding of the documentation f.immovable is independent of has_caps.

Yes they are, try it yourself in the editor. Some do some don't.

Here are the ones that do:
- Wasteland Trees
- Palm Trees
- Deciduous Trees
- Coniferous Trees
- Rocks
- Standing Stones
- Artifacts
- The 4 Ruins building from Miscellaneous

Here are the ones that don't hide caps:
- Dead Trees
- Plants
- Miscellaneous

>(in my perspective just walkable fields are not valuable, only if there
>can be build a flag/road)

Every walkable field is buildable since it can hold a flag. That's why
"The 3 Oasis" and "Glaciers Lake" are so big when you only check for
f.buildable. Hence, we need to check whether the field is accessible within
a certain radius.

Also fields around immovables, or when the map has certain topographically
properties are walkable, but can only hold only a flag. When you remove the
immovable (chopping tree or rock) the field becomes buildable.

Of course, we cannot only check for walkable, we need to check for other
properties as well. But it helps to distinguish between swimmable and un-
walkable fields. Which are not considered valuable.

>some problems of the algorithm to be suitable for every map:
>1. taking seafaring into account currently all small islands will be
>counted as long as there are two building spots, although they won't be
>accessible due to no port.
>2. same is for hidden valleys in the mountains or similar.

These are the problems we face with "Glacier Lakes". It has hidden valleys
and one Island. But the current at least improves the situation, although
there are still false positives.

I just played around with the editor to get a better understanding.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

Ok thanks for the explanation about caps. I will recheck in the code and via debug console why this is the case. From my perspective an immovable shouldn't alter the build caps information,
cause this behaviour would mean that the check for reachability would be compromised as well.

regarding the issues of islands I fully recognize and appreciate that your code is a major improvement, I just wanted to point out that there is still room for improvement.

While thinking about this I once had the thought to start evaluating the fields from the starting positions and ports to check if they are reachable But I don't have a proper solution how to achieve this yet.

Revision history for this message
kaputtnik (franku) wrote :

I think it is very important that the build caps can change temporarily. Otherwise one can build a building/flag on a node where e.g. a tree stands on it or a farmers field was planted. The same goes for placed rocks, which could be used to block an area of a map until the rocks are removed by a stonemason. Do i misunderstand some thing?

Some of the immovables in 'Miscellaneous' are permanently changing build caps (they can not be removed), e.g. the 'Debris'-ones, which were massively used on the map Dolomites. Same goes for all immovables in 'Standing Stones'.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

Hm. Not sure about this thing currently I did some digging into the c++ Code but I don't understand yet when these Caps are calculated. I always thought they are calculated only once and reflect general properties of a field. In the code it is written that they only need to be recalculated when the Terrain has been altered (e.g. by scenario script) In the c++ Part there is the explicit possibility to calculate with or without taking immovable into account. But I couldn't find out which possibility is used by the lua Hook and for which purpose the different options are used.
Anyhow I think we need to understand these issues to be able to do the things 100% correct for every map. For the moment being I could easily live with the current state of accuracy as it is much better than before and will render 99% of the maps to be playable in territorial mode. Only thing is we need to do something about performance, as I agree 30 secs is to long if they are experienced by the user.

Revision history for this message
Toni Förster (stonerl) wrote :

>From my perspective an immovable shouldn't alter the build caps
>information, cause this behaviour would mean that the check for
>reachability would be compromised as well.

What kaputtnik said, they should do this temporarily otherwise one
could build on an immovable. The easiest would be to assume that would
could just add all fields with immovables...

>regarding the issues of islands I fully recognize and appreciate that
>your code is a major improvement, I just wanted to point out that there
>is still room for improvement.

Thank you. Sorry if sounded salty in my last comment, that wasn't my intention.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

okay did some further investigations.
The C++ function to calculate the nodecaps can be executed with or without taking immovable into account (you need to provide the bool "consider_Mobs" which defaults to true)

As far as I could see the only instance where Mobs are ignored is for calculating if a portspace is allowed on a field.

the lua Hook has_caps is using the default calculation with mobs considered.

From my perspective we need to adjust the lua Hook to use this bool "consider_mobs" parameter to get the max caps of a field instead of the current caps. As for the valuable fields calculation we should consider the possible building options after removing stones and trees.

I might try to implement this but I am not very skilled in C++.

with this information we could use Toni's algorithm to get very close to the optimum.

Only problem is I am short of time

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

Hi
I figured out a solution to check the caps of a field without taking immovables into account.
Basicly I just added a lua hook and a new field property "max_caps".
This solution speeds up the algorithm because we don't have to take immovable fields into account for the accesibility loops.

Question is shall I push another revision to this branch with my changes or shall I push a new branch?

8962. By hessenfarmer

added new property max_caps to field.
addes lua hook to read this property

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4425. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/490531563.
Appveyor build 4213. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1810062_territorial_calculations-4213.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Shifting this to C++ and pre-calculating would work like this:

1. Add a Bool to flag whether the fields have been calculated
2. Increase the map packet number and use it to add a condition to map loading
3. For older maps, calculate, for newer maps, pre-load

We should only save this to maps though once it has been well-tested.

I have a slow machine to test the performance of the current state.

Revision history for this message
GunChleoc (gunchleoc) wrote :

I'm getting a compile error on debug builds:

Building CXX object src/wui/CMakeFiles/wui_common_gamedetails.dir/load_or_save_game.cc.o
In file included from /home/bratzbert/sources/widelands/bug-1810062-territorial-calculations/src/logic/map.h:33:0,
                 from /home/bratzbert/sources/widelands/bug-1810062-territorial-calculations/src/logic/editor_game_base.h:29,
                 from /home/bratzbert/sources/widelands/bug-1810062-territorial-calculations/src/logic/game.h:28,
                 from /home/bratzbert/sources/widelands/bug-1810062-territorial-calculations/src/wui/load_or_save_game.h:25,
                 from /home/bratzbert/sources/widelands/bug-1810062-territorial-calculations/src/wui/load_or_save_game.cc:20:
/home/bratzbert/sources/widelands/bug-1810062-territorial-calculations/src/logic/field.h:261:1: error: static assertion failed: Field is not tightly packed.
 static_assert(sizeof(Field) == sizeof(void*) * 2 + 10, "Field is not tightly packed.");

8963. By Toni Förster

merged trunk

Revision history for this message
Toni Förster (stonerl) wrote :

@hessenfarmer

I can't compile your changes, Travis isn't pleased as well.

/src/logic/field.h:261:1: error: static_assert failed "Field is not tightly packed."
static_assert(sizeof(Field) == sizeof(void*) * 2 + 10, "Field is not tightly packed.");
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

review: Needs Fixing
Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4453. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/491252805.
Appveyor build 4241. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1810062_territorial_calculations-4241.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

I didn't know about this as I use a MSYS2 Environment (like Appveyor) where it compiled and worked.
Unfortunately my C++ knowledge is very limited, so I am not sure how to fix this.

If anyone of you knows what is going on there feel free to correct it. I will try to do so as well tonight.

I believe this assert is to ensure minimal ressources consumption of a field object cause there are so many of them.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

The check that is failing is checking whether a field instance is tightly packed. (whatever that means)

However the reason for Linux failing while appveyor / MSYS2 not failing is that this check is defined different for a WIN32 system as for the other systems. The reason for this is unknown to me.
So I would propose to fix line 261 ff. of field.h and doing the check equally for all systems (i.e. deleting the if loop)

Revision history for this message
GunChleoc (gunchleoc) wrote :

The check under Windows is more strict, so unifying the check will not make the assert error go away. The roper fix is to understand what the problem is - I expect that there is a reason that somebody put that assert there and that there is a bug in the new code.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

the check for WIN32 is an ifndef check so in fact the check for Windows is less strict.

My code adds a new property to the field struct so I believe it is increased in size and therefore the check for all other Systems than Windows fail due to the more restrictive size check.
As these asserts are checking the size of field I can't see the reason why the assert is different for different OS.

As I haven't a Linux machine ready I'll try it the other way round by lowering the windows part as well and see if MSYS2 environment complains as well.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

Ok I tested it and it is a completely logical behaviour:

1. the assert tests the size of a field object to ensure the preprocessor command to pack this struct was successful
2. before my changes this object had 10 bytes + 2 times the standard address width (sizeof (void*))
3. I added a property as uint8 which needs another byte. so this results in 11 bytes + 2 times sizeof(void*)
4. the winows assert allowed for up to 11 bytes and did not complain, the other OS assert was checking the 10 bytes and complained logically.

so the solution (if we want to have this change) is indeed to adapt the assert to the new size of the field struct. I uploaded a new revision with the relevant changes.

8964. By hessenfarmer

adapted assert to new size of struct field

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4461. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/491840111.
Appveyor build 4249. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1810062_territorial_calculations-4249.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Duh, I overlooked the n. Your solution sounds good to me.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

Ok but still this branch needs testing especially with regard to performance.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Definitely. We should target it to Build 21 too.

8965. By Toni Förster

merged trunk

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4468. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/492822657.
Appveyor build 4256. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1810062_territorial_calculations-4256.

Revision history for this message
kaputtnik (franku) wrote :

This branch does not work for me on my poor laptop (Celeron 900 with 3,8Gb Memory). E.g. trying to load map Oasis Triangle will last forever while memory usage increases until no memory is available anymore. I have put a print statement at the beginning and the end of the function get_valuable_fields() and the output was:

Starting get_valuable_fields:
[some minutes gone]
ALSA lib pcm.c:8424:(snd_pcm_recover) underrun occurred
ALSA lib pcm.c:8424:(snd_pcm_recover) underrun occurred
ALSA lib pcm.c:8424:(snd_pcm_recover) underrun occurred
ALSA lib pcm.c:8424:(snd_pcm_recover) underrun occurred
ALSA lib pcm.c:8424:(snd_pcm_recover) underrun occurred
ALSA lib pcm.c:8424:(snd_pcm_recover) underrun occurred
ALSA lib pcm.c:8424:(snd_pcm_recover) underrun occurred
ALSA lib pcm.c:8424:(snd_pcm_recover) underrun occurred
...

I had to kill widelands then... Maybe this is the same issue as mentioned in comment 13 of bug 1651591

It is also annoying that the code is executed after the loading screen has finished. So one is staring at a black screen and nothing happens.

Revision history for this message
kaputtnik (franku) wrote :

Forgot to mention: Tested a release build.

Here the output for map Glacier Lake:

Starting get_valuable_fields:
get_valuable_fields took: 6410ms

6 seconds sitting in front of a black screen ;)

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

confirmed this is killing old machines on oasis triangle.

How about just checking the map for fields with maximum buildcaps small, medium, big, port, and mine. this isn't exakt as well but far less consuming.

8966. By Toni Förster

merge trunk

8967. By Toni Förster

use region() to evaluate valuable fields

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

we are getting better, at least my machine (Core2Duo T7500 6GB) is able to calculate oasis triangle with about 14000 valuable fields. Problem is it still took 20+ seconds.
I don't know the table beahviour in lua exactly, but if we could exclude/remove duplicates it would make sense to simply evaluate each field once and add all fields which could be conquered. I will work in this direction and see how far we could get.

Thanks anyway for implementig the region thing. I tdefinitly improves Glacier lake

Revision history for this message
Toni Förster (stonerl) wrote :

You're welcome. The question though, should we really evaluate big fields? We currently stop after 10 fields, because I think 12 is too far away and adds fields that are only accessible under extreme circumstances.

8968. By Toni Förster

only calculate for medium and big buildings up to 6 fields

Revision history for this message
Toni Förster (stonerl) wrote :

I think I found the sweet-spot. It is a radius of 6 for medium and big buildings. Let me explain: Before hessenfarmer introduced the max_caps we had to calculate up to 12 fields to make sure the immovable was accessible. This isn't the case any more. Here are some maps I tested with different calculations on my system:

Glacier Lake:

 6 fields b/m: 0:07 min - 3598 fields
 6 fields all: 0:06 min - 5196 fields
 8 fields all: 0:08 min - 5464 fields
10 fields all: 0:08 min - 5829 fields

3598 fields is the perfect value all others are still to high.

The Oasis Triangle:

 6 fields b/m: 0:42 min - 10520 fields
 6 fields all: 0:39 min - 11910 fields
 8 fields all: 1:02 min - 13652 fields
10 fields all: 1:26 min - 14208 fields

The Nile:

 6 fields b/m: 0:59 min - 39551 fields
 6 fields all: 1:00 min - 39626 fields
 8 fields all: 1:10 min - 45474 fields
10 fields all: 1:36 min - 47766 fields

Astoria 2R:

 6 fields b/m: 0:15 min - 42535 fields
 6 fields all: 0:15 min - 42400 fields
 8 fields all: -:-- min - 42605 fields
10 fields all: -:-- min - 42610 fields

The first 3 are maps that are to big because they use wrong assets. Astoria on the other hand only suffered from fields with immovables not being counted.

The calculation time for big maps with vast wastelands is still high, but maybe we could move the calculation in a parallel thread as hessenfarmer suggested.

Or we could move the initial calculation to the load screen. With a message like: "Calculating win conditions...", if possible.

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4482. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/494349394.
Appveyor build 4270. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1810062_territorial_calculations-4270.

8969. By Toni Förster

start get_valuable_fields in coroutine, sleep if field wasn't added

Revision history for this message
Toni Förster (stonerl) wrote :

I think I had an eureka moment. get_valuable_fields() is in a coroutine and gets calculated in the background. The sleep time can be adjusted, but I think 50ms is a good value. I tested 25ms which cut the time in half but the game was a little choppy.

8970. By Toni Förster

print ingame time when calculations are finished

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

I had a eureka moment as well. Here is my code which is very fast (just some seconds.)

function get_valuable_fields()
   local fields = {}
   local map = wl.Game().map
   for x=0, map.width-1 do
      for y=0, map.height-1 do
         local f = map:get_field(x,y)
         if f:has_max_caps("small") then
            local radius = f:region(6)
            for idx, fg in ipairs(radius) do
               local index = fg.x * 1000 + fg.y
               fields[index] = fg
            end
         elseif f:has_max_caps("medium") then
            local radius = f:region(8)
            for idx, fg in ipairs(radius) do
               local index = fg.x * 1000 + fg.y
               fields[index] = fg
            end
         elseif f:has_max_caps("big") then
            local radius = f:region(12)
            for idx, fg in ipairs(radius) do
               local index = fg.x * 1000 + fg.y
               fields[index] = fg
            end
         end
      end
   end
   local result = {}
   for idx,f in pairs(fields) do
      table.insert(result, f)
   end
   return result
end

of course we could discuss about the size of the regions. I will go through the lua files of all military buildings to see what might be good values.

8971. By Toni Förster

use hessenfarmers approach

Revision history for this message
Toni Förster (stonerl) wrote :

Awesome, your approach is so much more elegant. I pushed it to the branch.

Revision history for this message
kaputtnik (franku) wrote :

This is working nicely i think :) Some values with my Celeron 900:

Oasis Triangle:
Finished initial calculations after 21 minute(s) and 57 second(s).

The Nile:
Finished initial calculations after 21 minute(s) and 39 second(s).

Together we're strong:
Finished initial calculations after 14 minute(s) and 10 second(s).

Glacier Lake:
Finished initial calculations after 3 minute(s) and 24 second(s).

I have made also a small lan-game test on Glacier Lake until the first status message appears. All fine and working, and looking at the map it seems visually that i had conquered the half maps size.

review: Approve (testing)
Revision history for this message
kaputtnik (franku) wrote :

hm.. i was too slow... my comment was meant for r8970 of this branch...

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

ok after agoing through all military buildings i think small = 6 medium = 8 and big = 10 are good values.

8972. By Toni Förster

set radius to 10 for big buildings

Revision history for this message
Toni Förster (stonerl) wrote :

I'm torn. I love hessenfarmers approach so much, since it is so blazingly fast. But the calculated areas are too large.

I think r8970 is better ATM, since the calculated areas are more suited for Territorial.

Could we somehow marry both approaches?

8973. By Toni Förster

only calculate with max_caps(big/medium) 6 fields

Revision history for this message
Toni Förster (stonerl) wrote :

I think this could be it. Fast calculations with appropriate sizes.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

Next try I have done is starting from the starting field and add all fields that can be conquered.

function get_valuable_fields()
   local fields = {}
   local check = {}
   local map = wl.Game().map
   local sf = map.player_slots[1].starting_field
   -- initilaize with the region around the startign field of P1
   for idx, fg in ipairs(sf:region(9)) do
      local index = fg.x * 1000 + fg.y
      fields[index] = fg
      check[index] = fg
   end
      local loop = true
      while loop do
         loop = false
         local new = {}
         -- checking the check region for buildcaps and add fields that can be conquered
         for idx, f in pairs(check) do
            if f:has_max_caps("big") then
               local radius = f:region(10)
               for idx, fg in ipairs(radius) do
                  local index = fg.x * 1000 + fg.y
                  if fields[index] == nil and check[index] == nil then
                     -- if new fields are discovered add them to the fields table and mark them for checking next loop
                     new[index] = fg
                     fields[index] = fg
                     loop = true
                  end
               end
            elseif f:has_max_caps("medium") then
               local radius = f:region(8)
               for idx, fg in ipairs(radius) do
                  local index = fg.x * 1000 + fg.y
                  if fields[index] == nil and check[index] == nil then
                     new[index] = fg
                     fields[index] = fg
                     loop = true
                  end
               end
            elseif f:has_max_caps("small") then
               local radius = f:region(6)
               for idx, fg in ipairs(radius) do
                  local index = fg.x * 1000 + fg.y
                  if fields[index] == nil and check[index] == nil then
                     new[index] = fg
                     fields[index] = fg
                     loop = true
                  end
               end
            end
         end
         check = new
      end
   local result = {}
   for idx,f in pairs(fields) do
      table.insert(result, f)
   end
   return result
end

Only thing left is to adapt this for seafaring i.e. doing the same thing for all port spaces as well.
Speed is still good I think.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

Sorry. for some reasons the comments didn't get updated on my machine.
My latest chunk of code is delivering all fields that could be reasonably conquered on a map.
with big = 10 and medium = 8 and small = 6 it delivers 4768 on Glacier lake.

So for me this value sounds reasonable.

for Oasis triangle it delivers somewhat 14xxx which sounds reasonable as well.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

Another thing which is supporting my latest trial is that the actual coverage of fields is calculated against the fields originally identified.

so the algorithm is player x owns yy fields of all fields identified as valuable.

so not only the number is relevant but it matters which fields are identifeid as valuable. Therefore it doesn't help if we only have 3xxx fields on glacier lake as we can't still reach a lot of them.

Furthermore not taking the small caps into account would deliver false results on maps where medium sized and big spots are very rare (the dolomites for example)

8974. By Toni Förster

hessenfarmers second attempt

Revision history for this message
Toni Förster (stonerl) wrote :

Awesome! I pushed your second try.

Revision history for this message
Toni Förster (stonerl) wrote :

I'm fiddling around with the radius. Because Glacier is still a little too big and I think we shouldn't go to the extreme limits. What could be suitable values:

big: 8
medium: 6
small: 4

In Oasis this would result in 11655 fields, which is reasonable. 14000 Fields is way to many to get 50% I tested this yesterday.

In Glacier Lake this would be 4114 fields which is also way better. I'd prefer to have a little less then to many fields that are only accessible in edge cases.

But you do awesome work!

Revision history for this message
Toni Förster (stonerl) wrote :

BTW don't have port spaces also the big caps?

Revision history for this message
GunChleoc (gunchleoc) wrote :

Yes, port spaces also have big caps. I think the point was though if you have an island without a starting field and a port space, we need a field to seed calculation for that island.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

@ GunChleoc: exactly!

I will try to implement this tonight. At the moment I am busy watching my favourite football team so sorry for the delay

8975. By hessenfarmer

start with the starting fields of all players and all port spaces

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

ok done. Now we start our algorithm with all starting fields of all players and all port spaces.

the port space scan is a little consuming. (it doubles the scan time i think) but still i think it is ok.

I reduced the radiuses by one for each caps. (9, 7, 5) which is reasonable. I excluded unwalkable fields from being counted as well so we might have a good value now

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

as the underlying bug is really a shortfall on some maps and it is ensured we are still delivering a properly indexed fields table I would think this is worth going into b20, as it is not introducing new functionality.

Revision history for this message
kaputtnik (franku) wrote :

Can you add a print statement like Toni did? Or something like:

print(("Counting valuable fields took: %dms, result is %d."):format(ticks() - start, #result))

For Glacier lake the output on my slow laptop is:

Counting valuable fields took: 803ms, result is 4015.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

I am afk until tonight. So please feel free to do so.

Perhaps you could post the value for Crossing the horizon where I tested the seafaring part.

Astoria2.R would also be interesting as it is really big, so I would expect about 4 seconds there

Revision history for this message
GunChleoc (gunchleoc) wrote :

Codereview done.

I disagree that this is not a new feature, the code changes are more than trivial. Yes, it does fix a bug, but it's an old bug, so I'm not sure that it is critical enough to risk introducing new bugs.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

In the end I would leave it to your decision. However I still would think it to be worth having it.
The glacier map bug is an old one but the second bug reported by Toni isn't. As the behaviour detected by Toni (immovables affecting the field count) is affecting a lot of maps where the values of "fields to be conquered" are mostly wrong (e.g crater) it will give some great benefit because the effect of the bug is bigger than only the glacier lake / oasis triangle effect.

The changes aren't that big as they appear in my opinion.
the c++ change is barely more than a copy of an existing property of a field (that's true because it is the way I have managed to do so - my knowledge/experience of c++ is very limited).
and the lua change is definitely only in one function which delivers its output formatted in the same way as its predecessor.
so the risk in lua is to have wrong values of "fields to conquer" again for different maps currently not tested. This is no change to the previous situation where the values were wrong for the maps tested.
the risk in c++ is to have a failing method/interface this risk is mitigated by it being a full copy of an existing method/interface.

So if you value risk against benefit you might rethink about having it in b20. But still your decision, I just wanted to made my point clear.

I have answered your review below.

Revision history for this message
GunChleoc (gunchleoc) :
Revision history for this message
Toni Förster (stonerl) wrote :

I'd also like to see this fix make it into build20, since the second bug renders this game mode almost useless. Hessenfarmer as eloquently described the benefit, but in the end, it is up to you GunChleoc.

Revision history for this message
kaputtnik (franku) wrote :

Sorry, my slow laptop is only available at weekends. I can test with an Athlon 4300 Quad core, but i think this is not useful :-)

I'd vote targeting this to build21. GunChleoc wants to add a new c++ function for the port spaces, so the new code will get more complex. But build 21 should be released much faster than build20 ;)

8976. By GunChleoc

Replaced LuaField::get___hash with the coordinates' hash, which is much faster. Needs testing with Atlanteans 1 and Trident of FIre.

8977. By GunChleoc

Add function to list port spaces in Lua.

Revision history for this message
GunChleoc (gunchleoc) wrote :

I am done with my changes.

LuaField already had a __hash function, but I replaced it with a more efficient one. This means that we now need to test the Atlantean scenario and Trident of Fire on multiplayer.

The port spaces can be accessed like this:

   for index, value in ipairs(map.port_spaces) do
      print(index)
      print("x: " .. value["x"])
      print("y: " .. value["y"])
      for key, val in pairs(value) do
         print(key .. " " .. val)
      end
   end

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4493. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/495033189.
Appveyor build 4280. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1810062_territorial_calculations-4280.

8978. By hessenfarmer

adressed code review
used new lua hooks

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

Just did a quick test of the territorial calculation.

it works as before on "Glacier Lakes" (4015) and "crossing the horizon" (8072 if I remember this correctly).
crossing the horizon still takes some time but this should be acceptable from my side.

I tried to add some logging but this doesn't work in my windows installation so I couldn't test it and therefore removed it.

Does anybody know how to log something from lua in a windows build?

Regression testing for atlanteans 1 and trident of fire still necessary (I didn't know at all about the scripting in trident of fire, perhaps one could add some hint to the map description and some tesxt to scripting that explains what will go on)

Revision history for this message
kaputtnik (franku) wrote :

Doesn't lua print statements work on windows? At least some output in the console (cmd window)?

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

unfortunately they do not work nor in stdout.txt nor in the cmd window if started from there.
So I really would love to have some loggting possibility to stdout.txt for debugging purposes.

Revision history for this message
GunChleoc (gunchleoc) wrote :

1 remaining comment.

I am not planning any changes to the port spaces that would affect this function, so I'm not on the fence on whether to merge this into Build 20.

Revision history for this message
kaputtnik (franku) wrote :

On my usual machine:

Astoria2.r:
Counting valuable fields took: 29533ms, result is 48422.

Crossing the horizon:
Counting valuable fields took: 2853ms, result is 8072.

Wide World (Big map):
Counting valuable fields took: 5648ms, result is 21141.

Maybe the calculation should be done in a coroutine again?

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

@ Gun: I tried to reply your comment, although I am not sure whether I understood it correctly.

@ kaputtnik: Yeah the values of Astoria might imply we should do it in a coroutine.

I just did some testing with trident of fire and it didn't crash although I hardly managed to conquer some land and did not see any map editing. will continue to do so.

Revision history for this message
Toni Förster (stonerl) wrote :

I compiled a list of 65 maps with the calculation times and the calculated fields.

https://fosuta.org/pics/Territorial_Times_r8978.ods

Revision history for this message
GunChleoc (gunchleoc) :
Revision history for this message
hessenfarmer (stephan-lutz) wrote :

kaputtnik, Toni:
Could you please try Revision 8975 to Computer the values of Computing time

Revision history for this message
Toni Förster (stonerl) wrote :

@kaputtnik I'm at it right now.

Revision history for this message
Toni Förster (stonerl) wrote :

Done, can be found here:

https://fosuta.org/pics/Territorial_Times_r8975_r8978.ods

On average r8975 is slower. Furthermore, on some maps it calculates fewer fields. I marked them yellow.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

@Toni: Thanks a lot for providing the tables. So we still have an issue on large maps. Sounds like we would need to run in a coroutine, although we should try to improve the algorithm any further

One interesting question for me is what have been the values in the current trunk without these changes? Although I hardly dare asking for another 2 columns in the table ;-)

BTW: What Machine are you computing these values on. and do you use a debug or a release build?

I have no explanation yet for the different values calculated.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

@GunChleoc: i checked your code suggestion from my perspective this won't work. see the comment below.

The mechanism is that we only check a certain area in each loop. Beginning with the initial seed we discover new fields to be checked. but we can't add them to check directly cause we will recheck them in the next loop. so we add them to a new table and make them the area to check in the next run.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

OFF Topic: It seems the latest appveyor build of this branch seems to be stuck

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

Update:
I continued to playtest trident of fire and I saw a volcano eruption and saw it vanish again. So I assume the script still works with the new hash value

8979. By kaputtnik

print elapsed time and number of counted fields

Revision history for this message
Toni Förster (stonerl) wrote :

@hessenfarmer You're welcome. :D

https://fosuta.org/pics/Territorial_Times_trunk_r8975_r8978.ods

BTW. We shouldn't let us fool by the numbers, because they say nothing about the "quality" of the calculated fields. The fields calculated with r8978 are accurate while the ones in trunk are not.

I used the Release-Build on a Late 2013 MacBook Pro with a 2,4Ghz Intel Core i5 (I5-4258U)

8980. By Toni Förster

merged trunk

Revision history for this message
Toni Förster (stonerl) wrote :

I guess the difference in size for some maps between 8975 and 8978 is the way port spaces are evaluated. That is the main difference between the 2 revisions.

8981. By GunChleoc

Use repeat... until loop.

8982. By GunChleoc

Use print_loading_message.

8983. By GunChleoc

#table will become 0 if (0, 0) are the only new coordinates to be ckecked (hash = 0), so use a boolean again.

8984. By GunChleoc

check is smaller than fields, so compare that first

Revision history for this message
GunChleoc (gunchleoc) wrote :

Forget my last code tweak comment, I needed to look at this with code highlighting.

I have done some small tweaks to the code.

We should definitely stick this in a coroutine, because my screen will go black for a bit on Wide World, and my machine is fast.

8985. By GunChleoc

Delete dead variable

Revision history for this message
Toni Förster (stonerl) wrote :

I think a found a good tweak. The main problem is the second for loop:

for idx, fg in ipairs(radius) do

Here we iterate through fields too many times. On a small map some million times.

I got Astoria 2.R down from 58 seconds to 23seconds. On some maps the calculated area is a little smaller(in Elven Forest 5 fields) but it is much faster. The idea is that with big and medium caps we can assume that the inner 7 or 5 fields are always accessible, so we don't need to iterate to through them again and again.

I made my comments in the code.

8986. By Toni Förster

set inner radius for big and medium caps

Revision history for this message
Toni Förster (stonerl) wrote :

I pushed my tweak as revision 8986. I'm currently do some testing with all the maps and it looks promising. Will post an updated list later on.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

I am on a business trip until Friday so I can't do much other than check this side.
@Toni: In principle you are right in perhaps98% of the maps but there are cornercases where you might miss a special building spot which is needed to reach a somewhat hidden valley or a big mountain. So I believe as we need to put it in a coroutine anyway (23 secs is still too long) we could calculate the exact value. In any case many thanks for the values so as trunk is the benchmark for time we would struggle to achieve this.
@all: I just came to an idea how we could avoid the effect that Toni described and which is truly consuming. I can't test it until Friday though.

Revision history for this message
Toni Förster (stonerl) wrote :

New calculations are up:

https://fosuta.org/pics/Territorial_Times_v4.ods

We are at least 50% faster in general. The fields are almost the same. Green Cells have 20 or less fields difference. Yellow 50. And red more than 50 (but these are only two maps).

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4501. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/496123226.
Appveyor build 4288. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1810062_territorial_calculations-4288.

8987. By GunChleoc

Reimplement function in C++ without hollow region

8988. By GunChleoc

Use hollow region. Removed Lua code because speed increase is ca 50% from shifting it to C++.

Revision history for this message
GunChleoc (gunchleoc) wrote :

I think slightly underreporting is less critical than overreporting. Overreporting means that the game can't be won, underreporting means that it will be won somewhat faster.

I have now shifted the calculation to C++, because it is a lot faster. It also saves us from having to copy over the data structure at the end.

8989. By GunChleoc

Got rid of code duplication

Revision history for this message
GunChleoc (gunchleoc) wrote :

I am now done - I also found and fixed some code duplication. Back to you guys. We should:

1. Find the balance between speed and accuracy.
   You can play with the inner_radius variable for that.
2. Stick the calculation in a coroutine

Revision history for this message
Toni Förster (stonerl) wrote :

@GunChleoc

Shifting this to C++ isn't 50% faster. It is ludicrously fast. We are now as fast as trunk, sometimes even faster. I'm going to test with an inner_radius of 1 for all caps and the 1/5/7 as it is in revision 8989 and see what the difference is in time and fields.

I'm convinced we don't need to move this into a coroutine any more.

Just one question regarding the code (see inline).

Revision history for this message
Toni Förster (stonerl) :
Revision history for this message
kaputtnik (franku) wrote :

> I'm convinced we don't need to move this into a coroutine any more.

I don't think so. On my usual machine:

'Trident of Fire':
Found 15124 valuable fields
Calculating valuable fields took 2094ms

'Astoria2.r':
Found 42605 valuable fields
Calculating valuable fields took 8847ms

We have to think about slow machines too.

Sorry that i couldn't find time to provide more values.

Revision history for this message
Toni Förster (stonerl) wrote :

New calculations are up:

https://fosuta.org/pics/Territorial_Times_v5.ods

I just added revision 8989. I tried to set the inner_radius variable to 1/1/1 just for testing but the result was, that time went up, but we didn't gain many fields. So it wasn't worth it.

@kaputtnik please heave a look at the table. We are faster than trunk for the majority of the maps. 18 are less than 100ms slower. 3 less than 200. 1 is 269 and another one 1787.

So we are better than before IMHO.

Revision history for this message
Toni Förster (stonerl) wrote :

From my understanding we can't put this in a coroutine, because:

wl.Game().map.valuable_fields

blocks the lua script, as long as it is calculating, and these
calculations eat up all available cpu.

With this

run(function()
    fields = wl.Game().map.valuable_fields
end)

it runs in a coroutine, but the game stalls until the calculations are
done.

It isn't a real coroutine either, because we need to call

coroutine.yield()

from within the coroutine to pause it. But we can't since the c++ part
blocks the script.

Revision history for this message
Toni Förster (stonerl) wrote :

@kaputtnik I think you ran the debug build of the game.

On my system:

'Astoria2.r':

Release: 416ms
Debug: 11214ms

'Trident of Fire':

Release: 131ms
Debug: 2778ms

Revision history for this message
Toni Förster (stonerl) :
review: Approve (testing)
Revision history for this message
kaputtnik (franku) wrote :

Yes, it was a debug build...

8990. By GunChleoc

Add test for max_caps

8991. By GunChleoc

Add minimal test for valuable_fields

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

Hi,
As I am Bacck on my dev laptop I did some tests. first I found the reason for rev 8989 having less fields as rev 8978. In the c++ version non walkable fields are not added for port spaces and starting fields. These fields form the delta. especially in maps with a lot of adjacent port spaces (e.g. trident of fire) this is a better calculation as you would only conquer one port and not necessarily the others. By this we have implemented a general rule, that water isn't counting at all which is more congruent to the user. so I am very pleased with this change.

Furthermore I changed the lua version to the values in the c++ branch and they delivered the same results.
I modified the starting values and regions a bit. And still got the same results for some test maps. I believe this should be true for all maps.

The reason is that every building is blocking at least a radius of 1 for other buildings (only flags can be built there) so I think we could savely set the inner radius to 2 for every building which saves a lot of calculations. In lua f.e. Astoria 2R is down to around 10 seconds (handstopped with my watch). So I think we should try these values (using hollowregion with an inner radius of 2 for all fields including starting and port fields) before merging.

I really would love to see Tonis Table for these values.

I will playtest atlanteans 1 tonight just be sure we really do not have any issue with the hash code. If this is fine and the values for the changed regions are still good for the results I think we are finished.

Will report back later.

@ all and especially Toni: many many thanks for your contribution to fix this major bug in our winconditions.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

Another step forward did a full playtest of the first atlantean mission. No anomalies. Water was rising as intended.

So from my side we are ready. but we should try the inner radius of 2 before merging.

8992. By hessenfarmer

change inner radius to 2

Revision history for this message
Toni Förster (stonerl) wrote :

New calculations are up:

https://fosuta.org/pics/Territorial_Times_v6.ods

Setting the inner radius to 2 doesn't bring allot to the table, but it doesn't hurt either. So I leave this up to you guys.

@hessenfarmer thanks allot for your kind words. :-)

8993. By GunChleoc

Use hollow regions in initialization

Revision history for this message
GunChleoc (gunchleoc) wrote :

I am using HollowRegion for the init too now.

When loading the Nile, I get a black screen a lot longer than if I am starting a game in trunk.

I'll check how complicated it will be to add a loading screen here.

8994. By GunChleoc

New optional "init" coroutine for win conditions

8995. By GunChleoc

Calculate fields for Wood Gnome in C++

Revision history for this message
GunChleoc (gunchleoc) wrote :

I have now created a separate, optional initialization coroutine for the winconditions, so it can have a loading screen.

Also some refactoring in Artifacts and Wood Gnome.

This can go in after another round of testing.

review: Approve
8996. By GunChleoc

Add scoped timer

8997. By GunChleoc

Merged trunk.

Revision history for this message
Toni Förster (stonerl) :
8998. By GunChleoc

Fix typo & codecheck.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

I did some testing with the latest version.

Regarding the values of calculating the valuable fields ist is only slightly better in a release build while I suspect it much better in development build (haven't tested this yet) However I would keep this hollowregion values anyway as it is a real representation of what can be reasonably conquered.

Unfortunately Gun's very appreciated change to have this in game initialisation unmasked a new bug which has been there all the time and was not apparent. After initialisation the game is saved. With a big map (lots of fields to be conquered) this save is stalling while trying to write scripting data.
The same behaviour arises if we try to save such a big map in territory mode in trunk. so it is not a bug introduced by this branch. So shal we have a differerent branch or fix it in this one as it is somewhat realted to territorial.
My guess about the reason is that there is some overflow in the map saving code.

review: Needs Fixing
Revision history for this message
GunChleoc (gunchleoc) wrote :

Which map is causing the problem?

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

Magic Mountain at least. I'll test it with another big map as well.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

Europe1.1 has the same behaviour

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

Lost islands works

Revision history for this message
Toni Förster (stonerl) wrote :

The "Writing Scripting Data" part is executed several times and takes some minutes to execute. On my system roughly 3 times 5minutes.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

Big Eden fails. so as Big eden and Lost Islands have the same map size but differ in fields (52k to 118k) it is definitly dependent on the size of fields.

Revision history for this message
Toni Förster (stonerl) wrote :

@hessenfarmer what do you mean by it fails? The games does load eventually. The saving part just takes ages and depending on your system can cause very big maps to take 10-20 minutes to load.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

southfall islands (102k fields) fails as well.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

@ Toni: the game doesn't react anymore no activity can be seen on the HDD

Maybe it is a systemissue I am on win 10 64bit.

However Lost islands has a lot of fields as well (63k in trunk) and it just takes some seconds to save. While with more fields around 100K it stalls completely.

But I will give it a chance and wait for some minutes.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

ok I have to correct myself it doesn't fail it just takes very long. (95 sec to save the data for magic Mountain once).

So I will invalidate the bug just created.

However if we keep this branch in the current version we will have a loading time of 3 - 5 minutes while having a black screen for max 4 seconds before. perhaps we can shift the loading screen to the end so we will save the game without the fields in the beginning and only have the issue while saving in game.

Revision history for this message
GunChleoc (gunchleoc) wrote :

I have testes this with an ASAN debug build any everything is fine. What happens here is that big maps take a long time to calculate. Also, when doing the initial save in trunk, the fields weren't part of the scripting packet yet. Now they are, so saving takes longer.

One thing we should still optimize is that the autosave interval gets triggered before the game has finished loading, so I have to wait for it to saveload twice. I'll take a look at that.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

Waiting for the saveload is the way to go. Calcualtion takes 4 seconds on my old machine for magic mountain while saving takes 90 seconds. So we should initialize the fields after the initial save.

Revision history for this message
kaputtnik (franku) wrote :

Saving Magic Mountain on my old Laptop takes ~18sec with a release build (r8998; Linux).

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

I managed to load magic mountain. I took very long because of the multiple saves. I attach this log to the (now invalidated) bug.

8999. By GunChleoc

Do not postload again in the replay writer

Revision history for this message
GunChleoc (gunchleoc) wrote :

I managed to get rid of 1 of the saves. We are creating an extra temp file with a map save in EditorGameBase::postload() (thank you, Windows), and the postload was being called by the replay writer, which was completely superfluous.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

I tested the new version now. It is better of course as we have one writing cycyle less.
However it lasts still about 3:40 minutes from clicking start until the game finally starts for maps with a lot of fields.
As this was introduced to avoid a 4 (maybe 6) second black screen on big maps after the game started I am pretty unsure whether this is worth the pain. For me I'd rather accept a 6 second blackscreen and a game start after 40 seconds than having to wait 3:40 minutes. Perhaps we could display a message box from lua instead having this done in the game initialisation.
Of course we will suffer the same thing when saving a game but this could be circumvented by increasing the autosave interval.
Anyhow getting rid of the superfluos save is good in any way.

So what are your thoughts about this?

Another question I am interested in is whether this is a windows only behaviour and if so why?

Revision history for this message
Toni Förster (stonerl) wrote :

> Another question I am interested in is whether
> this is a windows only behaviour and if so why?

It happens on macOS as well. Don't know about Linux though.

> this could be circumvented by increasing the autosave interval.

But that does not solve the underlying issue.

> So what are your thoughts about this?

The question I't like to get answered is; why does it take
so long to write the save. Clearly here we have the CPU
bottlenecking the process. But why?

During Gameplay this is a problem as well, since the save might
take a minute or more depending on the CPU. I'd like to have
it the way it is in 8999 and investigate why it currently takes
so much time to save.

How does that affect network/internet games?

Here are some numbers for build 8999:

Lost Island has 51544 fields, with saving times:

Writing Scripting Data ... took 4944ms
Writing Scripting Data ... took 5062ms

Europe has 142137 fields, with saving times:

Writing Scripting Data ... took 38358ms
Writing Scripting Data ... took 41443ms

So Europe has 142% (2.7 times) more fields to save.
But the increase in time is 797% (8 times).

Revision history for this message
GunChleoc (gunchleoc) wrote :

That saving takes longer has nothing to do with the OS, I get it on Linux too.

I don't see how we can easily improve speed on writing the scripting packet, since that's mostly done by the eris backend. We do have substring calls in there though that might be made faster by using char pointers instead - no idea how much performance that would bring though.

We could look into tweaking more areas like I did for the statistics, which gave us a huge speed increase for the saveloading. The question is, do we need to do that right now - the problem is not new.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

I would vote for reverting to rev 8993 and deleting the superfluous write cycle.

Probably we should keep the woodgnome in c++ part as well.

So we would have a short black screen on very large maps but probably not longer than 4 secs.

That would be the best we can get for now, while fixing the wrong calculations.

we should improve saving and loading then for b21 and after this has been improved we should shift this again to the loading screen.

Revision history for this message
Toni Förster (stonerl) wrote :

Is it possible to get the maps name in lua?

like wl.Game().mape.name

Otherwise, since we know that this affects only some
larger maps why not do this for the time being:

   init = function()
      -- Get all valuable fields of the map
      if wl.Game().map.width <= 256 or (wl.Game().map.height <= 256) then
         fields = wl.Game().map.conquerable_fields
      end
   end,

      if wl.Game().map.width > 256 or (wl.Game().map.height > 256) then
         fields = wl.Game().map.conquerable_fields
      end

9000. By Toni Förster

if map is larger than 256x256 calculate the fields after the map has been loaded

Revision history for this message
Toni Förster (stonerl) wrote :

Can you test r9000?

9001. By GunChleoc

Run win condition scripts from InteractiveGameBase.

Revision history for this message
GunChleoc (gunchleoc) wrote :

-1 for hard-coding this to the maps' names. We cannot hard-code future maps uploaded to the website, and the list can get quite long.

I have found a way to shift the calculation past the initial saveloading cycle. We still get a black screen, but it has a message on it, so the player will know that something's happening.

With that change, I don't think that we will need the fix from r9000 any more?

9002. By Toni Förster

revert changes from r9000

Revision history for this message
Toni Förster (stonerl) wrote :

@GunChleoc a big thumbs up for your changes. I reverted the changes from r9000.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

@GunChleoc: You are my hero. This is working better than expected cause we don't have a black screen. We just have a little string in the lower left corner reading "initializing game".

So from my side I think we have got a very fine result now.

Thanks to everybody.

I think we are ready to merge, if CI is green.

review: Approve
Revision history for this message
Toni Förster (stonerl) wrote :

We need to merge trunk first. There seems to be a conflict.

Revision history for this message
Toni Förster (stonerl) wrote :

Just one question. In the general statistics for land differ from the actual valuable owned land, because it counts unwalkable and water. Should we leave it like it is or use the method we use for territorial?

Revision history for this message
Toni Förster (stonerl) wrote :

Just one question. In the general statistics for land differ from the actual valuable owned land, because it counts unwalkable (and ¿water?). Should we leave it like it is or use the method we use for territorials?

9003. By Toni Förster

merged trunk

9004. By GunChleoc

Performance tweaks

9005. By GunChleoc

Commenting and cleanup.

Revision history for this message
GunChleoc (gunchleoc) wrote :

I noticed that The Nile was a lot slower than Wide World, although The Nile is smaller. So, I have tweaked the algorithm further for performance.

The Nile before: 6552ms
The Nile after: 2716ms

General statistics is a good point. If we want to change those, we will have to do this field calculation in all games in order to be consistent. Alternatively, we could add a statistics hook for the win condition, just like we have for Wood Gnome and Collectors.

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4523. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/497984287.
Appveyor build 4310. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1810062_territorial_calculations-4310.

9006. By GunChleoc

Caps comparison is cheaper than container check.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

Code looks very good. The effect is the bigger the more port spaces are available. As we had walked them probably twice before now we walk them only if they weren' touched. by the starting positions walk.

Thanks a lot for doing a great job.

If we want to count only walkable fields in the statistics this might be simply done by adding a maxcaps walkable check to line

https://bazaar.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/view/head:/src/logic/game.cc#L973

However in my opinion the stats should stay as they are, as I am trying sometimes to conquer the complete map and compare the value against the result of mapwidth * mapheight.

Only thing we could probably do is adding the term valuable to the territorial status messages to clearly distinguish that not all field are taken into account but only valuable fields.

Revision history for this message
Toni Förster (stonerl) wrote :

Would it somehow be feasible to have the stats win_condition dependent? So, only for Territorial have the max_caps check and for all others have it as it is now?

Revision history for this message
GunChleoc (gunchleoc) wrote :

Theoretically yes, but I am against this. The same button should always do the same thing for the user.

I'll add a statistics hook just like for Wood Gnome and Collectors.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Adding a statistics hook is non-trivial, so I have created a bug for it:

https://bugs.launchpad.net/widelands/+bug/1817550

In the meantime, I expect that most players won't even notice the difference.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

Ok from my side.
So shall we have it then?

Revision history for this message
GunChleoc (gunchleoc) wrote :

Yes, let's have it.

@bunnybot merge

9007. By GunChleoc

clang-format

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

Thanks again for the great work everybody. However travis si complaining about a missing newline in autogenerated rst files.

It might be the case we need to insert an empty line at this line
https://bazaar.launchpad.net/~widelands-dev/widelands/trunk/view/head:/src/scripting/lua_map.cc#L6612

Revision history for this message
GunChleoc (gunchleoc) wrote :

Thanks, should be fixed ow.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'data/scripting/win_conditions/artifacts.lua'
2--- data/scripting/win_conditions/artifacts.lua 2018-04-15 09:11:29 +0000
3+++ data/scripting/win_conditions/artifacts.lua 2019-02-26 05:56:29 +0000
4@@ -15,10 +15,27 @@
5 local wc_descname = _("Artifacts")
6 local wc_version = 1
7 local wc_desc = _ "Search for ancient artifacts. Once all of them are found, the team who owns most of them will win the game."
8+
9+-- Table of all artifacts to conquer
10+local artifact_fields = {}
11+
12 return {
13 name = wc_name,
14 description = wc_desc,
15 map_tags = { "artifacts" }, -- Map tags needed so that this win condition will be available only for suitable maps
16+ init = function()
17+ -- Find all artifacts
18+ local map = wl.Game().map
19+ for x=0, map.width-1 do
20+ for y=0, map.height-1 do
21+ local field = map:get_field(x,y)
22+ if field.immovable and field.immovable:has_attribute("artifact") then
23+ -- This assumes that the immovable has size small or medium, i.e. only occupies one field
24+ table.insert(artifact_fields, map:get_field(x,y))
25+ end
26+ end
27+ end
28+ end,
29 func = function()
30 set_textdomain("win_conditions")
31 -- set the objective with the game type for all players
32@@ -33,22 +50,6 @@
33 end
34 end
35
36- local artifact_fields = {}
37- local map = wl.Game().map
38-
39- local i = 1
40- -- find all artifacts
41- for x=0, map.width-1 do
42- for y=0, map.height-1 do
43- local field = map:get_field(x,y)
44- if field.immovable and field.immovable:has_attribute("artifact") then
45- -- this assumes that the immovable has size small or medium, i.e. only occupies one field
46- artifact_fields[i] = map:get_field(x,y)
47- i = i + 1
48- end
49- end
50- end
51-
52 local plrs = wl.Game().players
53 if #artifact_fields == 0 then
54 for idx, plr in ipairs(plrs) do
55
56=== modified file 'data/scripting/win_conditions/territorial_functions.lua'
57--- data/scripting/win_conditions/territorial_functions.lua 2019-02-12 17:30:21 +0000
58+++ data/scripting/win_conditions/territorial_functions.lua 2019-02-26 05:56:29 +0000
59@@ -14,27 +14,6 @@
60 local wc_had_territory = _"%1$s had %2$3.0f%% of the land (%3$i of %4$i)."
61
62 -- RST
63--- .. function:: get_buildable_fields()
64---
65--- Collects all fields that are buildable
66---
67--- :returns: a table with the map's buildable fields
68---
69-function get_buildable_fields()
70- local fields = {}
71- local map = wl.Game().map
72- for x=0, map.width-1 do
73- for y=0, map.height-1 do
74- local f = map:get_field(x,y)
75- if f.buildable then
76- table.insert(fields, f)
77- end
78- end
79- end
80- return fields
81-end
82-
83--- RST
84 -- .. function:: count_owned_fields_for_all_players(fields, players)
85 --
86 -- Counts all owned fields for each player.
87@@ -67,12 +46,10 @@
88 return owned_fields
89 end
90
91-
92 -- Used by calculate_territory_points keep track of when the winner changes
93 local winning_players = {}
94 local winning_teams = {}
95
96-
97 -- RST
98 -- .. data:: territory_points
99 --
100
101=== modified file 'data/scripting/win_conditions/territorial_lord.lua'
102--- data/scripting/win_conditions/territorial_lord.lua 2019-01-26 17:38:18 +0000
103+++ data/scripting/win_conditions/territorial_lord.lua 2019-02-26 05:56:29 +0000
104@@ -22,9 +22,17 @@
105 "area. The winner will be the player or the team that is able to keep " ..
106 "that area for at least 20 minutes."
107 )
108+
109+-- Table of fields that are worth conquering
110+local fields = {}
111+
112 return {
113 name = wc_name,
114 description = wc_desc,
115+ init = function()
116+ -- Get all valuable fields of the map
117+ fields = wl.Game().map.conquerable_fields
118+ end,
119 func = function()
120 local plrs = wl.Game().players
121
122@@ -36,9 +44,6 @@
123 -- time in secs, if == 0 -> victory
124 territory_points.remaining_time = time_to_keep_territory
125
126- -- Get all valueable fields of the map
127- local fields = get_buildable_fields()
128-
129 local function _send_state()
130 set_textdomain("win_conditions")
131
132
133=== modified file 'data/scripting/win_conditions/territorial_time.lua'
134--- data/scripting/win_conditions/territorial_time.lua 2019-01-26 17:38:18 +0000
135+++ data/scripting/win_conditions/territorial_time.lua 2019-02-26 05:56:29 +0000
136@@ -27,19 +27,22 @@
137 "after 4 hours, whichever comes first."
138 )
139
140+-- Table of fields that are worth conquering
141+local fields = {}
142
143 return {
144 name = wc_name,
145 description = wc_desc,
146+ init = function()
147+ -- Get all valuable fields of the map
148+ fields = wl.Game().map.conquerable_fields
149+ end,
150 func = function()
151 local plrs = wl.Game().players
152
153 -- set the objective with the game type for all players
154 broadcast_objective("win_condition", wc_descname, wc_desc)
155
156- -- Get all valueable fields of the map
157- local fields = get_buildable_fields()
158-
159 -- variables to track the maximum 4 hours of gametime
160 local remaining_max_time = 4 * 60 * 60 -- 4 hours
161
162
163=== modified file 'data/scripting/win_conditions/wood_gnome.lua'
164--- data/scripting/win_conditions/wood_gnome.lua 2019-02-09 07:51:55 +0000
165+++ data/scripting/win_conditions/wood_gnome.lua 2019-02-26 05:56:29 +0000
166@@ -20,9 +20,18 @@
167 [[your territory than any other player. The game will end after 4 hours of ]] ..
168 [[playing. The one with the most trees at that point will win the game.]])
169 local wc_trees_owned = _"Trees owned"
170+
171+
172+-- Table of terrestrial fields
173+local fields = {}
174+
175 return {
176 name = wc_name,
177 description = wc_desc,
178+ init = function()
179+ -- Get all terrestrial fields of the map
180+ fields = wl.Game().map.terrestrial_fields
181+ end,
182 func = function()
183 local plrs = wl.Game().players
184 local game = wl.Game()
185@@ -33,21 +42,6 @@
186 -- set the objective with the game type for all players
187 broadcast_objective("win_condition", wc_descname, wc_desc)
188
189- -- Get all valueable fields of the map
190- local fields = {}
191- local map = wl.Game().map
192- for x=0,map.width-1 do
193- for y=0,map.height-1 do
194- local f = map:get_field(x,y)
195- if f then
196- -- add this field to the list as long as it has not movecaps swim
197- if not f:has_caps("swimmable") then
198- fields[#fields+1] = f
199- end
200- end
201- end
202- end
203-
204 -- The function to calculate the current points.
205 local _last_time_calculated = -100000
206 local _plrpoints = {}
207
208=== modified file 'src/logic/field.h'
209--- src/logic/field.h 2019-02-23 11:00:49 +0000
210+++ src/logic/field.h 2019-02-26 05:56:29 +0000
211@@ -92,6 +92,9 @@
212 NodeCaps nodecaps() const {
213 return static_cast<NodeCaps>(caps);
214 }
215+ NodeCaps maxcaps() const {
216+ return static_cast<NodeCaps>(max_caps);
217+ }
218 uint16_t get_caps() const {
219 return caps;
220 }
221@@ -237,6 +240,7 @@
222 BaseImmovable* immovable = nullptr;
223
224 uint8_t caps = 0U;
225+ uint8_t max_caps = 0U;
226 uint8_t roads = 0U;
227
228 Height height = 0U;
229@@ -254,9 +258,9 @@
230
231 // Check that Field is tightly packed.
232 #ifndef WIN32
233-static_assert(sizeof(Field) == sizeof(void*) * 2 + 10, "Field is not tightly packed.");
234+static_assert(sizeof(Field) == sizeof(void*) * 2 + 11, "Field is not tightly packed.");
235 #else
236-static_assert(sizeof(Field) <= sizeof(void*) * 2 + 11, "Field is not tightly packed.");
237+static_assert(sizeof(Field) <= sizeof(void*) * 2 + 12, "Field is not tightly packed.");
238 #endif
239 } // namespace Widelands
240
241
242=== modified file 'src/logic/game.cc'
243--- src/logic/game.cc 2019-02-23 11:00:49 +0000
244+++ src/logic/game.cc 2019-02-26 05:56:29 +0000
245@@ -314,13 +314,29 @@
246
247 // Check for win_conditions
248 if (!settings.scenario) {
249- std::unique_ptr<LuaTable> table(lua().run_script(settings.win_condition_script));
250+ win_condition_script_ = settings.win_condition_script;
251+ std::unique_ptr<LuaTable> table(lua().run_script(win_condition_script_));
252 table->do_not_warn_about_unaccessed_keys();
253 win_condition_displayname_ = table->get_string("name");
254+ // We run the actual win condition from InteractiveGameBase::start() to prevent a pure black
255+ // screen while the game is being started - we can display a message there.
256+ } else {
257+ win_condition_displayname_ = "Scenario";
258+ }
259+}
260+
261+void Game::run_win_condition() {
262+ if (!win_condition_script_.empty()) {
263+ std::unique_ptr<LuaTable> table(lua().run_script(win_condition_script_));
264+ table->do_not_warn_about_unaccessed_keys();
265+ // Run separate initialization function if it is there.
266+ if (table->has_key<std::string>("init")) {
267+ std::unique_ptr<LuaCoroutine> cr = table->get_coroutine("init");
268+ cr->resume();
269+ }
270 std::unique_ptr<LuaCoroutine> cr = table->get_coroutine("func");
271 enqueue_command(new CmdLuaCoroutine(get_gametime() + 100, std::move(cr)));
272- } else {
273- win_condition_displayname_ = "Scenario";
274+ win_condition_script_ = "";
275 }
276 }
277
278
279=== modified file 'src/logic/game.h'
280--- src/logic/game.h 2019-02-23 11:00:49 +0000
281+++ src/logic/game.h 2019-02-26 05:56:29 +0000
282@@ -176,6 +176,10 @@
283 void save_syncstream(bool save);
284 void init_newgame(UI::ProgressWindow* loader_ui, const GameSettings&);
285 void init_savegame(UI::ProgressWindow* loader_ui, const GameSettings&);
286+
287+ /// Run the win condition that is defined by win_condition_script_
288+ void run_win_condition();
289+
290 enum StartGameType { NewSPScenario, NewNonScenario, Loaded, NewMPScenario };
291
292 bool run(UI::ProgressWindow* loader_ui,
293@@ -403,6 +407,7 @@
294
295 /// For save games and statistics generation
296 std::string win_condition_displayname_;
297+ std::string win_condition_script_;
298 bool replay_;
299 };
300
301
302=== modified file 'src/logic/map.cc'
303--- src/logic/map.cc 2019-02-23 11:00:49 +0000
304+++ src/logic/map.cc 2019-02-26 05:56:29 +0000
305@@ -28,6 +28,7 @@
306
307 #include "base/log.h"
308 #include "base/macros.h"
309+#include "base/scoped_timer.h"
310 #include "base/wexception.h"
311 #include "build_info.h"
312 #include "economy/flag.h"
313@@ -43,6 +44,7 @@
314 #include "logic/map_objects/world/terrain_description.h"
315 #include "logic/map_objects/world/world.h"
316 #include "logic/mapfringeregion.h"
317+#include "logic/maphollowregion.h"
318 #include "logic/objective.h"
319 #include "logic/pathfield.h"
320 #include "logic/player.h"
321@@ -255,6 +257,109 @@
322 }
323 }
324
325+std::set<FCoords> Map::calculate_all_conquerable_fields() const {
326+ std::set<FCoords> result;
327+
328+ std::set<FCoords> coords_to_check;
329+
330+ ScopedTimer timer("Calculating valuable fields took %ums");
331+
332+ // If we don't have the given coordinates yet, walk the map and collect conquerable fields,
333+ // initialized with the given radius around the coordinates
334+ const auto walk_starting_coords = [this, &result, &coords_to_check](
335+ const Coords& coords, int radius) {
336+ FCoords fcoords = get_fcoords(coords);
337+
338+ // We already have these coordinates
339+ if (result.count(fcoords) == 1) {
340+ return;
341+ }
342+
343+ // Add starting field
344+ result.insert(fcoords);
345+
346+ // Add outer land coordinates around the starting field for the given radius
347+ std::unique_ptr<Widelands::HollowArea<>> hollow_area(
348+ new Widelands::HollowArea<>(Widelands::Area<>(fcoords, radius), 2));
349+ std::unique_ptr<Widelands::MapHollowRegion<>> map_region(
350+ new Widelands::MapHollowRegion<>(*this, *hollow_area));
351+ do {
352+ coords_to_check.insert(get_fcoords(map_region->location()));
353+ } while (map_region->advance(*this));
354+
355+ // Walk the map
356+ while (!coords_to_check.empty()) {
357+ // Get some coordinates to check
358+ const auto coords_it = coords_to_check.begin();
359+ fcoords = *coords_it;
360+
361+ // Get region according to buildcaps
362+ radius = 0;
363+ int inner_radius = 2;
364+ if ((fcoords.field->maxcaps() & BUILDCAPS_BIG) == BUILDCAPS_BIG) {
365+ radius = 9;
366+ inner_radius = 7;
367+ } else if (fcoords.field->maxcaps() & BUILDCAPS_MEDIUM) {
368+ radius = 7;
369+ inner_radius = 5;
370+ } else if (fcoords.field->maxcaps() & BUILDCAPS_SMALL) {
371+ radius = 5;
372+ }
373+
374+ // Check region and add walkable fields
375+ if (radius > 0) {
376+ hollow_area.reset(
377+ new Widelands::HollowArea<>(Widelands::Area<>(fcoords, radius), inner_radius));
378+ map_region.reset(new Widelands::MapHollowRegion<>(*this, *hollow_area));
379+ do {
380+ fcoords = get_fcoords(map_region->location());
381+
382+ // We do the caps check first, because the comparison is faster than the container
383+ // check
384+ if ((fcoords.field->maxcaps() & MOVECAPS_WALK) && (result.count(fcoords) == 0)) {
385+ result.insert(fcoords);
386+ coords_to_check.insert(fcoords);
387+ }
388+ } while (map_region->advance(*this));
389+ }
390+
391+ // These coordinates are done. We do not keep track of visited coordinates that didn't make
392+ // the result, because the container insert operations are more expensive than the checks
393+ coords_to_check.erase(coords_it);
394+ }
395+ };
396+
397+ // Walk the map from the starting field of each player
398+ for (const Coords& coords : starting_pos_) {
399+ walk_starting_coords(coords, 9);
400+ }
401+
402+ // Walk the map from port spaces
403+ if (allows_seafaring()) {
404+ for (const Coords& coords : get_port_spaces()) {
405+ walk_starting_coords(coords, 5);
406+ }
407+ }
408+
409+ log("Found %" PRIuS " valuable fields\n", result.size());
410+ return result;
411+}
412+
413+std::set<FCoords> Map::calculate_all_fields_excluding_caps(NodeCaps caps) const {
414+ ScopedTimer timer("Calculating valuable fields took %ums");
415+
416+ std::set<FCoords> result;
417+ for (MapIndex i = 0; i < max_index(); ++i) {
418+ Field& field = fields_[i];
419+ if (!(field.nodecaps() & caps)) {
420+ result.insert(get_fcoords(field));
421+ }
422+ }
423+
424+ log("Found %" PRIuS " valuable fields\n", result.size());
425+ return result;
426+}
427+
428 /*
429 ===============
430 remove your world, remove your data
431@@ -550,9 +655,9 @@
432 }
433
434 NodeCaps Map::get_max_nodecaps(const World& world, const FCoords& fc) const {
435- NodeCaps caps = calc_nodecaps_pass1(world, fc, false);
436- caps = calc_nodecaps_pass2(world, fc, false, caps);
437- return caps;
438+ NodeCaps max_caps = calc_nodecaps_pass1(world, fc, false);
439+ max_caps = calc_nodecaps_pass2(world, fc, false, max_caps);
440+ return static_cast<NodeCaps>(max_caps);
441 }
442
443 /// \returns the immovable at the given coordinate
444@@ -938,6 +1043,7 @@
445 */
446 void Map::recalc_nodecaps_pass1(const World& world, const FCoords& f) {
447 f.field->caps = calc_nodecaps_pass1(world, f, true);
448+ f.field->max_caps = calc_nodecaps_pass1(world, f, false);
449 }
450
451 NodeCaps Map::calc_nodecaps_pass1(const World& world, const FCoords& f, bool consider_mobs) const {
452@@ -1056,6 +1162,8 @@
453 */
454 void Map::recalc_nodecaps_pass2(const World& world, const FCoords& f) {
455 f.field->caps = calc_nodecaps_pass2(world, f, true);
456+ f.field->max_caps =
457+ calc_nodecaps_pass2(world, f, false, static_cast<NodeCaps>(f.field->max_caps));
458 }
459
460 NodeCaps Map::calc_nodecaps_pass2(const World& world,
461
462=== modified file 'src/logic/map.h'
463--- src/logic/map.h 2019-02-23 11:00:49 +0000
464+++ src/logic/map.h 2019-02-26 05:56:29 +0000
465@@ -165,6 +165,12 @@
466 void recalc_whole_map(const World& world);
467 void recalc_for_field_area(const World& world, Area<FCoords>);
468
469+ /// Calculates and returns a list of the fields that could be conquered by a player throughout a
470+ /// game. Useful for territorial win conditions.
471+ std::set<FCoords> calculate_all_conquerable_fields() const;
472+ /// Calculates and returns a list of the fields that do not have the given caps.
473+ std::set<FCoords> calculate_all_fields_excluding_caps(NodeCaps caps) const;
474+
475 /***
476 * Ensures that resources match their adjacent terrains.
477 */
478
479=== modified file 'src/logic/replay.cc'
480--- src/logic/replay.cc 2019-02-23 11:00:49 +0000
481+++ src/logic/replay.cc 2019-02-26 05:56:29 +0000
482@@ -229,7 +229,6 @@
483 GameLoader gl(filename_ + kSavegameExtension, game);
484 gl.load_game();
485 }
486- game.postload();
487 log("Done reloading the game from replay\n");
488
489 game.enqueue_command(new CmdReplaySyncWrite(game.get_gametime() + kSyncInterval));
490
491=== modified file 'src/scripting/lua_map.cc'
492--- src/scripting/lua_map.cc 2019-02-23 11:00:49 +0000
493+++ src/scripting/lua_map.cc 2019-02-26 05:56:29 +0000
494@@ -71,6 +71,39 @@
495
496 namespace {
497
498+// Checks if a field has the desired caps
499+bool check_has_caps(lua_State* L,
500+ const std::string& query,
501+ const FCoords& f,
502+ const NodeCaps& caps,
503+ const Widelands::Map& map) {
504+ if (query == "walkable") {
505+ return caps & MOVECAPS_WALK;
506+ }
507+ if (query == "swimmable") {
508+ return caps & MOVECAPS_SWIM;
509+ }
510+ if (query == "small") {
511+ return caps & BUILDCAPS_SMALL;
512+ }
513+ if (query == "medium") {
514+ return caps & BUILDCAPS_MEDIUM;
515+ }
516+ if (query == "big") {
517+ return (caps & BUILDCAPS_BIG) == BUILDCAPS_BIG;
518+ }
519+ if (query == "port") {
520+ return (caps & BUILDCAPS_PORT) && map.is_port_space(f);
521+ }
522+ if (query == "mine") {
523+ return caps & BUILDCAPS_MINE;
524+ }
525+ if (query == "flag") {
526+ return caps & BUILDCAPS_FLAG;
527+ }
528+ report_error(L, "Unknown caps queried: %s!", query.c_str());
529+}
530+
531 // Pushes a lua table with (name, count) pairs for the given 'ware_amount_container' on the
532 // stack. The 'type' needs to be WARE or WORKER. Returns 1.
533 int wares_or_workers_map_to_lua(lua_State* L,
534@@ -1160,9 +1193,12 @@
535 const PropertyType<LuaMap> LuaMap::Properties[] = {
536 PROP_RO(LuaMap, allows_seafaring),
537 PROP_RO(LuaMap, number_of_port_spaces),
538+ PROP_RO(LuaMap, port_spaces),
539 PROP_RO(LuaMap, width),
540 PROP_RO(LuaMap, height),
541 PROP_RO(LuaMap, player_slots),
542+ PROP_RO(LuaMap, conquerable_fields),
543+ PROP_RO(LuaMap, terrestrial_fields),
544 {nullptr, nullptr, nullptr},
545 };
546
547@@ -1199,6 +1235,32 @@
548 lua_pushuint32(L, get_egbase(L).map().get_port_spaces().size());
549 return 1;
550 }
551+
552+/* RST
553+ .. attribute:: port_spaces
554+
555+ (RO) A list of coordinates for all port spaces on the map.
556+
557+ :returns: A table of port space coordinates,
558+ like this: ``{{x = 0, y = 2}, {x = 54, y = 23}}``.
559+*/
560+int LuaMap::get_port_spaces(lua_State* L) {
561+ lua_newtable(L);
562+ int counter = 0;
563+ for (const Coords& space : get_egbase(L).map().get_port_spaces()) {
564+ lua_pushinteger(L, ++counter);
565+ lua_newtable(L);
566+ lua_pushstring(L, "x");
567+ lua_pushint32(L, space.x);
568+ lua_settable(L, -3);
569+ lua_pushstring(L, "y");
570+ lua_pushint32(L, space.y);
571+ lua_settable(L, -3);
572+ lua_settable(L, -3);
573+ }
574+ return 1;
575+}
576+
577 /* RST
578 .. attribute:: width
579
580@@ -1237,6 +1299,42 @@
581 return 1;
582 }
583
584+/* RST
585+ .. attribute:: conquerable_fields
586+
587+ (RO) Calculates and returns all reachable fields that a player could build on.
588+
589+ **Note:** This function is expensive, so call it seldom.
590+*/
591+int LuaMap::get_conquerable_fields(lua_State* L) {
592+ lua_newtable(L);
593+ int counter = 0;
594+ for (const Widelands::FCoords& fcoords :
595+ get_egbase(L).map().calculate_all_conquerable_fields()) {
596+ lua_pushinteger(L, ++counter);
597+ to_lua<LuaMaps::LuaField>(L, new LuaMaps::LuaField(fcoords));
598+ lua_settable(L, -3);
599+ }
600+ return 1;
601+}
602+
603+/* RST
604+ .. attribute:: terrestrial_fields
605+
606+ (RO) Calculates and returns all fields that are not swimmable.
607+*/
608+int LuaMap::get_terrestrial_fields(lua_State* L) {
609+ lua_newtable(L);
610+ int counter = 0;
611+ for (const Widelands::FCoords& fcoords :
612+ get_egbase(L).map().calculate_all_fields_excluding_caps(MOVECAPS_SWIM)) {
613+ lua_pushinteger(L, ++counter);
614+ to_lua<LuaMaps::LuaField>(L, new LuaMaps::LuaField(fcoords));
615+ lua_settable(L, -3);
616+ }
617+ return 1;
618+}
619+
620 /*
621 ==========================================================
622 LUA METHODS
623@@ -3798,7 +3896,11 @@
624 PROPERTIES
625 ==========================================================
626 */
627-// Hash is used to identify a class in a Set
628+/* RST
629+ .. attribute:: __hash
630+
631+ (RO) The map object's serial. Used to identify a class in a Set.
632+*/
633 int LuaMapObject::get___hash(lua_State* L) {
634 lua_pushuint32(L, get(L, get_egbase(L))->serial());
635 return 1;
636@@ -5993,8 +6095,8 @@
637
638 const char LuaField::className[] = "Field";
639 const MethodType<LuaField> LuaField::Methods[] = {
640- METHOD(LuaField, __eq), METHOD(LuaField, __tostring), METHOD(LuaField, region),
641- METHOD(LuaField, has_caps), {nullptr, nullptr},
642+ METHOD(LuaField, __eq), METHOD(LuaField, __tostring), METHOD(LuaField, region),
643+ METHOD(LuaField, has_caps), METHOD(LuaField, has_max_caps), {nullptr, nullptr},
644 };
645 const PropertyType<LuaField> LuaField::Properties[] = {
646 PROP_RO(LuaField, __hash),
647@@ -6038,10 +6140,13 @@
648 PROPERTIES
649 ==========================================================
650 */
651-// Hash is used to identify a class in a Set
652+/* RST
653+ .. attribute:: __hash
654+
655+ (RO) The hashed coordinates of the field's position. Used to identify a class in a Set.
656+*/
657 int LuaField::get___hash(lua_State* L) {
658- const std::string pushme = (boost::format("%i_%i") % coords_.x % coords_.y).str();
659- lua_pushstring(L, pushme.c_str());
660+ lua_pushuint32(L, coords_.hash());
661 return 1;
662 }
663
664@@ -6458,45 +6563,51 @@
665
666 Returns :const:`true` if the field has this caps associated
667 with it, otherwise returns false.
668+ Note: Immovables will hide the caps. If you want to have the caps
669+ without immovables use has_max_caps instead
670
671 :arg capname: can be either of
672
673- * :const:`small`: Can a small building be build here?
674- * :const:`medium`: Can a medium building be build here?
675- * :const:`big`: Can a big building be build here?
676- * :const:`mine`: Can a mine be build here?
677- * :const:`port`: Can a port be build here?
678- * :const:`flag`: Can a flag be build here?
679+ * :const:`small`: Can a small building be built here?
680+ * :const:`medium`: Can a medium building be built here?
681+ * :const:`big`: Can a big building be built here?
682+ * :const:`mine`: Can a mine be built here?
683+ * :const:`port`: Can a port be built here?
684+ * :const:`flag`: Can a flag be built here?
685 * :const:`walkable`: Is this field passable for walking bobs?
686 * :const:`swimmable`: Is this field passable for swimming bobs?
687 */
688 int LuaField::has_caps(lua_State* L) {
689- FCoords f = fcoords(L);
690- std::string query = luaL_checkstring(L, 2);
691-
692- if (query == "walkable")
693- lua_pushboolean(L, f.field->nodecaps() & MOVECAPS_WALK);
694- else if (query == "swimmable")
695- lua_pushboolean(L, f.field->nodecaps() & MOVECAPS_SWIM);
696- else if (query == "small")
697- lua_pushboolean(L, f.field->nodecaps() & BUILDCAPS_SMALL);
698- else if (query == "medium")
699- lua_pushboolean(L, f.field->nodecaps() & BUILDCAPS_MEDIUM);
700- else if (query == "big")
701- lua_pushboolean(L, (f.field->nodecaps() & BUILDCAPS_BIG) == BUILDCAPS_BIG);
702- else if (query == "port") {
703- lua_pushboolean(
704- L, (f.field->nodecaps() & BUILDCAPS_PORT) && get_egbase(L).map().is_port_space(f));
705- } else if (query == "mine")
706- lua_pushboolean(L, f.field->nodecaps() & BUILDCAPS_MINE);
707- else if (query == "flag")
708- lua_pushboolean(L, f.field->nodecaps() & BUILDCAPS_FLAG);
709- else
710- report_error(L, "Unknown caps queried: %s!", query.c_str());
711-
712- return 1;
713-}
714-
715+ const FCoords& f = fcoords(L);
716+ std::string query = luaL_checkstring(L, 2);
717+ lua_pushboolean(L, check_has_caps(L, query, f, f.field->nodecaps(), get_egbase(L).map()));
718+ return 1;
719+}
720+
721+/* RST
722+ .. method:: has_max_caps(capname)
723+
724+ Returns :const:`true` if the field has this maximum caps (not taking immovables into account)
725+ associated with it, otherwise returns false.
726+
727+ :arg capname: can be either of
728+
729+ * :const:`small`: Can a small building be built here?
730+ * :const:`medium`: Can a medium building be built here?
731+ * :const:`big`: Can a big building be built here?
732+ * :const:`mine`: Can a mine be built here?
733+ * :const:`port`: Can a port be built here?
734+ * :const:`flag`: Can a flag be built here?
735+ * :const:`walkable`: Is this field passable for walking bobs?
736+ * :const:`swimmable`: Is this field passable for swimming bobs?
737+*/
738+int LuaField::has_max_caps(lua_State* L) {
739+ const FCoords& f = fcoords(L);
740+ std::string query = luaL_checkstring(L, 2);
741+ lua_pushboolean(
742+ L, check_has_caps(L, luaL_checkstring(L, 2), f, f.field->maxcaps(), get_egbase(L).map()));
743+ return 1;
744+}
745 /*
746 ==========================================================
747 C METHODS
748
749=== modified file 'src/scripting/lua_map.h'
750--- src/scripting/lua_map.h 2019-02-23 11:00:49 +0000
751+++ src/scripting/lua_map.h 2019-02-26 05:56:29 +0000
752@@ -88,9 +88,12 @@
753 */
754 int get_allows_seafaring(lua_State*);
755 int get_number_of_port_spaces(lua_State*);
756+ int get_port_spaces(lua_State*);
757 int get_width(lua_State*);
758 int get_height(lua_State*);
759 int get_player_slots(lua_State*);
760+ int get_conquerable_fields(lua_State*);
761+ int get_terrestrial_fields(lua_State*);
762
763 /*
764 * Lua methods
765@@ -1424,6 +1427,7 @@
766 int __eq(lua_State* L);
767 int region(lua_State* L);
768 int has_caps(lua_State*);
769+ int has_max_caps(lua_State*);
770
771 /*
772 * C methods
773
774=== modified file 'src/wui/interactive_gamebase.cc'
775--- src/wui/interactive_gamebase.cc 2019-02-23 11:00:49 +0000
776+++ src/wui/interactive_gamebase.cc 2019-02-26 05:56:29 +0000
777@@ -224,6 +224,8 @@
778 }
779
780 void InteractiveGameBase::start() {
781+ game().run_win_condition();
782+
783 // Multiplayer games don't save the view position, so we go to the starting position instead
784 if (is_multiplayer()) {
785 Widelands::PlayerNumber pln = player_number();
786
787=== modified file 'test/maps/lua_testsuite.wmf/scripting/cfield.lua'
788--- test/maps/lua_testsuite.wmf/scripting/cfield.lua 2016-07-10 19:03:33 +0000
789+++ test/maps/lua_testsuite.wmf/scripting/cfield.lua 2019-02-26 05:56:29 +0000
790@@ -8,12 +8,12 @@
791 assert_equal(c.y, 32)
792 end
793
794-function field_tests:test_access_xistobig()
795+function field_tests:test_access_xistoobig()
796 assert_error("x should be too big", function()
797 map:get_field(64, 23)
798 end)
799 end
800-function field_tests:test_access_yistobig()
801+function field_tests:test_access_yistoobig()
802 assert_error("y should be too big", function()
803 map:get_field(25, 80)
804 end)
805@@ -23,10 +23,10 @@
806 map:get_field(64)
807 end)
808 end
809-function field_tests:test_access_xisnegativ()
810+function field_tests:test_access_xisnegative()
811 assert_error("x is negativ", function() map:get_field(-12, 23) end)
812 end
813-function field_tests:test_access_yisnegativ()
814+function field_tests:test_access_yisnegative()
815 assert_error("y is negativ", function() map:get_field(25, -12) end)
816 end
817 function field_tests:test_direct_change_impossible()
818@@ -34,7 +34,12 @@
819 assert_error("c.y should be read only", function() c.y = 12 end)
820 end
821 function field_tests:test_hash()
822- assert_equal("25_40", map:get_field(25,40).__hash)
823+ assert_equal(1638440, map:get_field(25,40).__hash)
824+end
825+function field_tests:test_coordinates()
826+ local field = map:get_field(25,40);
827+ assert_equal(25, field.x)
828+ assert_equal(40, field.y)
829 end
830
831 function field_tests:test_r_neighbour()
832@@ -212,6 +217,15 @@
833 assert_equal(false, f:has_caps("swimmable"))
834 assert_equal(true, f:has_caps("walkable"))
835 assert_equal(false, f:has_caps("port"))
836+
837+ assert_equal(true, f:has_max_caps("small"))
838+ assert_equal(true, f:has_max_caps("medium"))
839+ assert_equal(true, f:has_max_caps("big"))
840+ assert_equal(true, f:has_max_caps("flag"))
841+ assert_equal(false, f:has_max_caps("mine"))
842+ assert_equal(false, f:has_max_caps("swimmable"))
843+ assert_equal(true, f:has_max_caps("walkable"))
844+ assert_equal(false, f:has_max_caps("port"))
845 end
846 function field_caps_tests:test_flag_field_on_land()
847 local f = map:get_field(1,53)
848@@ -223,6 +237,15 @@
849 assert_equal(false, f:has_caps("swimmable"))
850 assert_equal(true, f:has_caps("walkable"))
851 assert_equal(false, f:has_caps("port"))
852+
853+ assert_equal(false, f:has_max_caps("small"))
854+ assert_equal(false, f:has_max_caps("medium"))
855+ assert_equal(false, f:has_max_caps("big"))
856+ assert_equal(true, f:has_max_caps("flag"))
857+ assert_equal(false, f:has_max_caps("mine"))
858+ assert_equal(false, f:has_max_caps("swimmable"))
859+ assert_equal(true, f:has_max_caps("walkable"))
860+ assert_equal(false, f:has_max_caps("port"))
861 end
862 function field_caps_tests:test_mine_field()
863 local f = map:get_field(63,54)
864@@ -234,6 +257,15 @@
865 assert_equal(false, f:has_caps("swimmable"))
866 assert_equal(true, f:has_caps("walkable"))
867 assert_equal(false, f:has_caps("port"))
868+
869+ assert_equal(false, f:has_max_caps("small"))
870+ assert_equal(false, f:has_max_caps("medium"))
871+ assert_equal(false, f:has_max_caps("big"))
872+ assert_equal(true, f:has_max_caps("flag"))
873+ assert_equal(true, f:has_max_caps("mine"))
874+ assert_equal(false, f:has_max_caps("swimmable"))
875+ assert_equal(true, f:has_max_caps("walkable"))
876+ assert_equal(false, f:has_max_caps("port"))
877 end
878 function field_caps_tests:test_field_on_water()
879 local f = map:get_field(7,58)
880@@ -245,6 +277,15 @@
881 assert_equal(true, f:has_caps("swimmable"))
882 assert_equal(false, f:has_caps("walkable"))
883 assert_equal(false, f:has_caps("port"))
884+
885+ assert_equal(false, f:has_max_caps("small"))
886+ assert_equal(false, f:has_max_caps("medium"))
887+ assert_equal(false, f:has_max_caps("big"))
888+ assert_equal(false, f:has_max_caps("flag"))
889+ assert_equal(false, f:has_max_caps("mine"))
890+ assert_equal(true, f:has_max_caps("swimmable"))
891+ assert_equal(false, f:has_max_caps("walkable"))
892+ assert_equal(false, f:has_max_caps("port"))
893 end
894 function field_caps_tests:test_port_field()
895 local f = map:get_field(9,56)
896@@ -256,15 +297,47 @@
897 assert_equal(false, f:has_caps("swimmable"))
898 assert_equal(true, f:has_caps("walkable"))
899 assert_equal(true, f:has_caps("port"))
900+
901+ assert_equal(true, f:has_max_caps("small"))
902+ assert_equal(true, f:has_max_caps("medium"))
903+ assert_equal(true, f:has_max_caps("big"))
904+ assert_equal(true, f:has_max_caps("flag"))
905+ assert_equal(false, f:has_max_caps("mine"))
906+ assert_equal(false, f:has_max_caps("swimmable"))
907+ assert_equal(true, f:has_max_caps("walkable"))
908+ assert_equal(true, f:has_max_caps("port"))
909 end
910
911+function field_caps_tests:test_field_with_immovable()
912+ local field = map:get_field(1,53)
913+ assert_equal(true, field:has_caps("flag"))
914+ assert_equal(true, field:has_max_caps("flag"))
915+
916+ local flag = player1:place_flag(field, true)
917+ assert_equal(false, field:has_caps("flag"))
918+ assert_equal(true, field:has_max_caps("flag"))
919+
920+ flag:remove()
921+ assert_equal(true, field:has_caps("flag"))
922+ assert_equal(true, field:has_max_caps("flag"))
923+ end
924+
925+
926 function field_caps_tests:test_wrong_call()
927 assert_error("Unknown caps", function()
928 map:get_field(7,58):has_caps("blahfasel")
929 end)
930 end
931
932+function field_caps_tests:test_conquerable_fields_does_not_crash()
933+ local fields = map.conquerable_fields
934+ assert_equal(false, fields[1] == nil)
935+end
936
937+function field_caps_tests:test_terrestrial_fields_does_not_crash()
938+ local fields = map.terrestrial_fields
939+ assert_equal(false, fields[1] == nil)
940+end
941
942 -- ===============
943 -- owner/claimers
944
945=== modified file 'test/maps/two_ponds.wmf/scripting/test_seafaring.lua'
946--- test/maps/two_ponds.wmf/scripting/test_seafaring.lua 2018-04-11 06:55:01 +0000
947+++ test/maps/two_ponds.wmf/scripting/test_seafaring.lua 2019-02-26 05:56:29 +0000
948@@ -33,6 +33,14 @@
949 map:recalculate_seafaring()
950 assert_equal(true, map.allows_seafaring)
951
952+ local port_spaces = map.port_spaces
953+ assert_equal(port_spaces[1]["x"], 0)
954+ assert_equal(port_spaces[1]["y"], 2)
955+ assert_equal(port_spaces[2]["x"], 7)
956+ assert_equal(port_spaces[2]["y"], 2)
957+ assert_equal(port_spaces[3]["x"], 12)
958+ assert_equal(port_spaces[3]["y"], 24)
959+
960 -- Remove the port space again
961 assert_equal(true, map:set_port_space(0, 2, false))
962 assert_equal(2, map.number_of_port_spaces)

Subscribers

People subscribed via source and target branches

to status/vote changes: