Atlanteans campaign keeps counting 0 ships

[GunChleoc]

The last objective in the Atlantean campaign never completes, because the size of the fleet always remains 0.

To reproduce, hack the scenario a bit so you won't have to play it through:

Add the following lines to intro():

    p1:allow_buildings{"shipyard"}
    check_for_ships()

And the following line to check_for_ships():

    print("ships: " .. nships)

Then build some support buildings and a shipyard. After the first ship is completed, you will still get this output: "ships: 0"

I tried simplifying the ship check to this:

    local ships = p1:get_ships()
    while #ships < 3 do
        print("ships: " .. #ships)
        sleep(8234)
    end

But it still fails.

[TiborB (tiborb95)]

Regression tests (especially test_ship_movement_controls.lua) still pass?

your syntax is bad:

    local ships = p1:get_ships() <-this must be re-run within every loop below
    while #ships < 3 do
        print("ships: " .. #ships)
        sleep(8234)
    end

Would following work?

while #p1:get_ships() < 3 do

I am not that skilled with lua to say without testing.

[GunChleoc (gunchleoc)]

Regression tests still pass.

I tried:

    while #p1:get_ships() < 3 do
        print("ships: " .. #p1:get_ships())
        sleep(8234)
    end

Ship count is still 0.

[TiborB (tiborb95)]

plr points to right player, correct:

campaigns/tutorial03_seafaring.wmf/scripting/init.lua:plr = wl.Game().players[1]

[TiborB (tiborb95)]

Is the ship properly on water? because it looks for ships only on fields with MOVECAPS_SWIM

[GunChleoc (gunchleoc)]

Yes, I double-checked the player. There is only 1 player in this scenario anyway. I also rememver seeing the ship bobbing on the waves.

Another difference to the regression tests is that we don't have any ports or portdocks - the ships are built at the end of the scenario to escape the island, so there is no trade that would need a port, and here is no expedition built.

[TiborB (tiborb95)]

get_ships is quite simple, it just scan all water fields, identify ships, and compares the owner(s).

I will investigate this...

[TiborB (tiborb95)]

Portdock shows "no use for ships for this map" - this is trunk 7566. Is this not fixed yet?

[GunChleoc (gunchleoc)]

No, the bug is still open: https://bugs.launchpad.net/widelands/+bug/1507923

[TiborB (tiborb95)]

Well, I had to say it better - it is just not building any ships at all! How can I debug this issue then :)

[GunChleoc (gunchleoc)]

I hacked the Map::allows_seafaring() check for testing by letting it always return true.

[TiborB (tiborb95)]

I cannot reproduce it. Ships starts transporting wares and counting counts....

[GunChleoc (gunchleoc)]

If ships start ransporting wares, you will have built a port? The Atlantean scenario doesn't use ports.

[TiborB (tiborb95)]

Oh, dont we talk about seafaring tutorial? It seems I mixed scenario with tutorial. So please navigate me where is that map(?).
When I open single player - new game and click "scenarios", I dont know which map it should be...

[GunChleoc (gunchleoc)]

Seafaring tutorial was the other bug. This one is for the Atlantean campaign. Single Player -> Campaigns -> Atlanteans.

[TiborB (tiborb95)]

I see only Barbarians campaign there. Was it added only lately? My branch was made about a week ago...

[GunChleoc (gunchleoc)]

That's because you never played the campaigns.

Unzip the attached file and drop it into ~/./widelands/save

[TiborB (tiborb95)]

I am plaing it and I am still not in a point where I would build a port, can it be speeded up?

[TiborB (tiborb95)]

Oh, I was wrong - it does not use my get_ships() function at all, but traverses the map by itself. So this is not bug in my code.

I would still look at it, but I am not willing to struggle for undefined time to get to the one point I am interested in...

If you or somebody will modify scripting in the atl01 map, I would go on...

Or provide me a savefile in point where a bug shows...

[TiborB (tiborb95)]

Whoever will look at it, I suggest starting with this:
for idx,f in ipairs(map:get_field(75,80):region(12)) do

[GunChleoc (gunchleoc)]

I have added instructions in the first post on how to hack the scenario, so you won't have to play the whole scenario each time. We should use get_ships here, the code we have is still from the time before we had that function.

If the shipyard complains about no ships needed on map, just comment out "check_map=seafaring" from the Altantean Shipyard.

I can create a branch with these changes in it, no problem.

[TiborB (tiborb95)]

Ok then, I will look at it, thanks!

[TiborB (tiborb95)]

The problem found! This check:
f.field->nodecaps() & MOVECAPS_SWIM
it is. Ships are simply on fields that does not have this capabilities. I can remove the test from my get_ships(), and scan entire map for ships... No big deal

Unless you have another idea, I will do it...

It seems that capabilities of fields were changed lately, because old algorithm also was based on SWIM capabilities, I think....

[GunChleoc (gunchleoc)]

The whole MOVECAPS thing is quite complicated. The SWIM value might not be set for nodes directly next to the shore or something like that. So, searching the whole map is a viable solution for now, if this function isn't called too often.

Another possible design would be to go through the player's portdocks, grab their fleet objects and then grab the fleets' ships. We would need to ensure that there aren't any duplicates counted though - I expect that portdocks will share fleets to transport wares back and forth.

[TiborB (tiborb95)]

In fact I found the core problem.

Sometimes during my tinkering with ships/fleets I omitted ship_update_idle() function that is responsible for moving a ship from a coast.

If it is run - no issue, ship will end up on water and everything works as supposed.

BTW, I lately created a branch ship_update - so I will incorporate a changes needed to fix this there and also I will create (modify) a regression test, when ship is put on shore to test it is really moved to water.

[TiborB (tiborb95)]

correction: the branch is named fleet_update

[GunChleoc (gunchleoc)]

Glad you found it :)

I have linked up the branch and will test once you are ready with a merge request.

[GunChleoc (gunchleoc)]

Fixed in build19-rc1.