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

2019-01-02 Thread Toni Förster
The proposal to merge lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands has been updated. Description changed to: For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366 -- Your team Widela

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

2019-01-02 Thread Toni Förster
The proposal to merge lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands has been updated. Description changed to: This adds fields covered by trees, stones, artifacts and buildings to calculated territory. For more details, see: https://code.launchpad.net/~wi

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

2019-01-02 Thread Toni Förster
Toni Förster has proposed merging lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands. Commit message: add immovables to the table for territory calculation Requested reviews: Widelands Developers (widelands-dev) Related bugs: Bug #1810062 in widelands: "Calcu

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

2019-01-02 Thread bunnybot
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_181006

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

2019-01-03 Thread bunnybot
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_18100

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

2019-01-05 Thread bunnybot
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_181006

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

2019-02-08 Thread bunnybot
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_181006

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

2019-02-10 Thread bunnybot
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_18100

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

2019-02-11 Thread bunnybot
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_181006

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

2019-02-13 Thread bunnybot
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_18100

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

2019-02-16 Thread bunnybot
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_181006

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

2019-02-18 Thread bunnybot
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_181006

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

2019-02-20 Thread bunnybot
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_181006

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

2019-02-23 Thread GunChleoc
The proposal to merge lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands has been updated. Commit message changed to: Refactor initialization of win conditions: - Separate optional "init" coroutine that is executed on the loading screen - Shift calculations fo

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

2019-02-23 Thread GunChleoc
The proposal to merge lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands has been updated. Commit message changed to: Refactor initialization of win conditions: - Separate optional "init" coroutine that is executed on the loading   screen - Shift calculations fo

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

2019-02-25 Thread bunnybot
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_18100

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

2019-02-25 Thread noreply
The proposal to merge lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366 -- Your te

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-01-03 Thread Toni Förster
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 cov

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-01-03 Thread Toni Förster
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... -- https://code.launchpad.net/~w

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-01-04 Thread Toni Förster
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 sys

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-01-05 Thread kaputtnik
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, i

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-01-05 Thread Toni Förster
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 terr

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-01-05 Thread kaputtnik
> 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 :-) -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territo

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-01-08 Thread hessenfarmer
+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 bu

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-01-08 Thread kaputtnik
> 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

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-01-09 Thread hessenfarmer
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 a

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-01-09 Thread Toni Förster
>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 o

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-01-10 Thread hessenfarmer
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

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-01-10 Thread Toni Förster
>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 Tre

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-01-11 Thread hessenfarmer
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 t

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-01-11 Thread kaputtnik
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

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-01-11 Thread hessenfarmer
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

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-01-11 Thread Toni Förster
>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 ass

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-01-14 Thread hessenfarmer
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 calculatin

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-02 Thread hessenfarmer
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. Qu

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-09 Thread GunChleoc
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 thoug

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-09 Thread GunChleoc
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/

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-10 Thread Toni Förster
Review: Needs Fixing @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."); ^

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-11 Thread hessenfarmer
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

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-11 Thread hessenfarmer
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

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-11 Thread GunChleoc
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. -- https://code.launchpad.net/~

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-11 Thread hessenfarmer
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

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-11 Thread hessenfarmer
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

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-11 Thread GunChleoc
Duh, I overlooked the n. Your solution sounds good to me. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1810062-territorial-calculations. ___

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-12 Thread hessenfarmer
Ok but still this branch needs testing especially with regard to performance. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1810062-territorial-calcul

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-12 Thread GunChleoc
Definitely. We should target it to Build 21 too. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1810062-territorial-calculations.

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-16 Thread kaputtnik
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_f

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-16 Thread kaputtnik
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 ;) -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/3613

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-16 Thread hessenfarmer
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. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculat

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-16 Thread hessenfarmer
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 eva

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-16 Thread Toni Förster
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. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-ca

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-16 Thread Toni Förster
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 c

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-16 Thread Toni Förster
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. -- https://code.launchpad.net/~widelands-d

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-17 Thread hessenfarmer
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

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-17 Thread Toni Förster
Awesome, your approach is so much more elegant. I pushed it to the branch. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1810062-territorial-calculati

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-17 Thread kaputtnik
Review: Approve testing 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 calc

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-17 Thread kaputtnik
hm.. i was too slow... my comment was meant for r8970 of this branch... -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1810062-territorial-calculations

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-17 Thread hessenfarmer
ok after agoing through all military buildings i think small = 6 medium = 8 and big = 10 are good values. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/b

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-17 Thread Toni Förster
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? -- https://code.launchpad.net/~widelands-de

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-17 Thread Toni Förster
I think this could be it. Fast calculations with appropriate sizes. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1810062-territorial-calculations. _

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-17 Thread hessenfarmer
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 sta

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-17 Thread hessenfarmer
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 tri

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-17 Thread hessenfarmer
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 id

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-17 Thread Toni Förster
Awesome! I pushed your second try. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1810062-territorial-calculations. _

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-17 Thread Toni Förster
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%

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-17 Thread Toni Förster
BTW don't have port spaces also the big caps? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1810062-territorial-calculations. ___

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-17 Thread GunChleoc
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. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366 Your t

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-17 Thread hessenfarmer
@ 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 -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366 Your team Widelands Developers is subscribed to

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-17 Thread hessenfarmer
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 unw

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-17 Thread hessenfarmer
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. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territoria

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-17 Thread kaputtnik
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. -- https://code.launchpad.net/~

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-18 Thread hessenfarmer
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 -- https://code.launchpad.net/~widelands-dev/wide

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-18 Thread GunChleoc
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. Diff comments: > === modified file 'data/scripting/win_conditions/territoria

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-18 Thread hessenfarmer
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 o

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-18 Thread GunChleoc
Diff comments: > === modified file 'data/scripting/win_conditions/territorial_functions.lua' > --- data/scripting/win_conditions/territorial_functions.lua 2019-02-12 > 17:30:21 + > +++ data/scripting/win_conditions/territorial_functions.lua 2019-02-17 > 19:27:39 + > @@ -14,24 +14,

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-18 Thread Toni Förster
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. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-18 Thread kaputtnik
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 rele

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-18 Thread GunChleoc
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_space

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-19 Thread hessenfarmer
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

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-19 Thread kaputtnik
Doesn't lua print statements work on windows? At least some output in the console (cmd window)? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1810062

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-19 Thread hessenfarmer
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. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366 Y

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-19 Thread GunChleoc
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. Diff comments: > === modified file 'data/scripting/win_conditions/territorial_functions.lua' > --- data/scripting/win_conditions

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-19 Thread kaputtnik
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 coroutin

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-19 Thread hessenfarmer
@ 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 an

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-19 Thread Toni Förster
I compiled a list of 65 maps with the calculation times and the calculated fields. https://fosuta.org/pics/Territorial_Times_r8978.ods -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366 Your team Widelands Developers is subscribed to branch

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-19 Thread GunChleoc
Diff comments: > === modified file 'data/scripting/win_conditions/territorial_functions.lua' > --- data/scripting/win_conditions/territorial_functions.lua 2019-02-12 > 17:30:21 + > +++ data/scripting/win_conditions/territorial_functions.lua 2019-02-19 > 07:34:52 + > @@ -14,24 +14,

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-19 Thread hessenfarmer
kaputtnik, Toni: Could you please try Revision 8975 to Computer the values of Computing time Diff comments: > === modified file 'data/scripting/win_conditions/territorial_functions.lua' > --- data/scripting/win_conditions/territorial_functions.lua 2019-02-12 > 17:30:21 + > +++ data/scripti

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-19 Thread Toni Förster
@kaputtnik I'm at it right now. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1810062-territorial-calculations. _

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-19 Thread Toni Förster
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. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366 Y

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-19 Thread hessenfarmer
@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 cha

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-19 Thread hessenfarmer
@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 re

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-19 Thread hessenfarmer
OFF Topic: It seems the latest appveyor build of this branch seems to be stuck -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1810062-territorial-calcu

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-20 Thread hessenfarmer
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 -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366 Your team Widelands Developers

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-20 Thread Toni Förster
@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

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-20 Thread Toni Förster
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. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366 Your team Widelands Developers is

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-20 Thread GunChleoc
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. -- https://code.launchpad.net/~widelan

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-20 Thread Toni Förster
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

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-20 Thread Toni Förster
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. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366 Your team Widelands Developers is subscribed to

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-20 Thread hessenfarmer
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

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands

2019-02-20 Thread Toni Förster
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). -- https://code.launchpad.net/~widelan

  1   2   >