Thanks, should be fixed ow.
--
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.
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
--
Y
Yes, let's have it.
@bunnybot merge
--
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.
Ok from my side.
So shall we have it then?
--
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.
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.
--
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.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366
Your team
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?
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366
Your team Widelands
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
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:
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
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?
--
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?
--
We need to merge trunk first. There seems to be a conflict.
--
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.
Review: Approve
@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
@GunChleoc a big thumbs up for your changes. I reverted the changes from r9000.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366
Your team Widelands Developers is subscribed to branch
-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
Can you test r9000?
--
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.
___
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
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
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
> 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
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
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 ca
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.
--
I managed to load magic mountain. I took very long because of the multiple
saves. I attach this log to the (now invalidated) bug.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366
Your team Widelands Developers is subscribed to branch
Saving Magic Mountain on my old Laptop takes ~18sec with a release build
(r8998; Linux).
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366
Your team Widelands Developers is subscribed to branch
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.
--
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
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
@ 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
@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.
--
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.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366
Your team Widelands Developers is
The "Writing Scripting Data" part is executed several times and takes some
minutes to execute. On my system roughly 3 times 5minutes.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366
Your team Widelands Developers is subscribed to branch
Lost islands works
--
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.
___
Europe1.1 has the same behaviour
--
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.
Magic Mountain at least. I'll test it with another big map as well.
--
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.
Which map is causing the problem?
--
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.
Review: Needs Fixing
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
Diff comments:
>
> === modified file 'src/logic/map.h'
> --- src/logic/map.h 2018-12-13 07:24:01 +
> +++ src/logic/map.h 2019-02-21 09:14:58 +
> @@ -165,6 +165,9 @@
> void recalc_whole_map(const World& world);
> void recalc_for_field_area(const World& world, Area);
>
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 ca
Review: Approve
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.
--
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.
--
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. :-)
--
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.
--
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
Yes, it was a debug build...
--
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.
Review: Approve testing
--
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.
@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
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366
Your team Widelands
>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
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
> 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
Diff comments:
>
> === modified file 'src/logic/map.cc'
> --- src/logic/map.cc 2019-02-12 17:27:04 +
> +++ src/logic/map.cc 2019-02-21 09:14:58 +
> @@ -255,6 +257,77 @@
> }
> }
>
> +
> +std::set Map::calculate_valuable_fields() const {
> + std::set result;
> +
@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
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
--
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
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:
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).
--
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
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
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
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.
--
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
@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.
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
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
@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
@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
@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.
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 +
> +++
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
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
@ 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
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
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
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
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
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:
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
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
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.
--
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
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
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
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
--
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.
--
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.
--
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
@ 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
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
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.
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
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.
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
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
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
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.
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
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
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
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
1 - 100 of 143 matches
Mail list logo
