Bugs in new ShipScheduling algorithm

[Klaus Halfmann]

Load the attached savegame (bases on the Imperial Sceanrio) and wait a few seconds.

...
 WidelandsMapLoader::load_map_complete() for 'Neptune’s Revenge' took 3182ms
...
[sync] Reset
ComputerPlayer(2): initializing as type 2
    ... DNA initialized
  2: 0 basic buildings in savegame file.
 2: expedition max duration = 5006 (83 minutes), map area root: 104
Assertion failed: (get_path(start, finish, path_start_to_finish)), function is_path_favourable, file ../src/economy/fleet.cc, line 764.
Abort trap: 6

This was just after I conquered the Babarian HQ I think.

This is bzr9088[trunk] compiled with ASAN, LLVM on OSX.

[Benedikt Straub (nordfriese)]

The function in which the assert fail happens was introduced by the new ship scheduling algorithm…

[kaputtnik (franku)]

I got this assert also with bzr9094 when loading the attached savegame and the second ship arrives at the port.

The reason why i have made this savegame was: Before saving i noticed:

1. the first ship arrives at the port
2. loaded soldiers and stones
3. navigated to the close port on the west
4. removed all wares and soldiers
5. loaded the wares and soldiers again
6. navigated to the third port (also in the west but far away)

I have set the priority of soldiers on the third port, so i was surprised why the ship stops at at the second port un/reloading the wares and soldiers. I thought this is a bug and created this savegame, which shows the assert now.

I did not understand why the ship scheduling branch was merged anyway... it hasn't a bug associated and wasn't tested imho. Hopefully this is not part of build20.

[kaputtnik (franku)]

The backtrace of the assert

[kaputtnik (franku)]

If you are interested in the odd behavior ships navigating to a port unloading wares and immediately load the same wares again, here is the replay. Increase gametime until you are at 2hrs and 29 minutes, scroll the map to the position where you can see two ports and wait until the ships start bringing wares to the unseen port.

[Benedikt Straub (nordfriese)]

I accepted this bug now because I´m also getting it in a savegame now ;) My backtrace is identical to yours, so it seems this happens quite often.

The new shipping algorithm workflow seems really strange to me: As far as I can see, the general idea is that a ship will unload wares/workers if its next destination is far out of the way, and it will pick up items where the next destination either the item´s destination or if the destination is "favourable" regarding the item´s actual destination, whatever that means...
Lots of times, a ShippingItem (that´s a wrapper for a WareInstance) will have no destination set (why?) but the Ware it contains has a portdock as its next destination (how???), in which case the ware is unloaded and promptly re-added (as I described in the merge request).

The author of the new algorithm left a TODO that the existence of ShippingItems without a destination should be prevented, and that´s where a true fix would need to start. I don´t know that code nearly well enough to do that though :(

Fortunately this is not in build 20. I feel safe to compare our great new ship-scheduling with our great new anti-congestion algorithm in some regards...

[GunChleoc (gunchleoc)]

I have triggered the merge. I might look into this for Build 21 or not - we can always rip it out if not.

[GunChleoc (gunchleoc)]

I have now attached the original branch to this bug for easier bookkeeping.

To be fair, the anti-congestion algorithm seems to have been working and our transport bug is caused by something else. The savehandling control flow changed when we fixed the handling of economy IDs, so the bug is probably there somewhere, but I had to rip out the other algorithm anyway so that we could exclude it as a possible culprit.

[Benedikt Straub (nordfriese)]

By the way, I just discovered that the destination portdock of a ShippingItem is not set when loading. This may explain the invalid destinations and consequent bugs. I´m fixing this in the attached redesigning branch.

[GunChleoc (gunchleoc)]

Migrated to https://github.com/widelands/widelands/issues/589