Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
I have added some comments
Diff comments:
>
> === modified file 'src/logic/map_objects/tribes/ship.cc'
> --- src/logic/map_objects/tribes/ship.cc 2019-05-27 14:28:34 +
> +++ src/logic/map_objects/tribes/ship.cc 2019-09-03 15:47:16 +
> @@ -736,22 +736,215 @@
> }
> }
>
> +bool Ship::has_destination(EditorGameBase& egbase, const PortDock& pd) const
> {
> + for (const auto& pair : destinations_) {
> + if (pair.first.get(egbase) == ) {
> + return true;
> + }
> + }
> + return false;
> +}
> +
> /**
> * Enter a new destination port for the ship.
> * Call this after (un)loading the ship, for proper logging.
> */
> -void Ship::set_destination(PortDock* pd) {
> - destination_ = pd;
> - if (pd) {
> - molog("set_destination / sending to portdock %u (carrying %"
> PRIuS " items)\n", pd->serial(),
> - items_.size());
> - pd->ship_coming(true);
> - } else {
> - molog("set_destination / none\n");
> +void Ship::push_destination(Game& game, PortDock& pd) {
> + for (auto it = destinations_.begin(); it != destinations_.end(); ++it) {
for (const auto& pair : destinations_) {
> + if (it->first.get(game) == ) {
> + // We're already going there
> + return;
> + }
> }
> + destinations_.push_back(std::make_pair(OPtr(), 1));
> + reorder_destinations(game);
> + molog("push_destination(%u): rerouted to portdock %u (carrying %" PRIuS
> " items)\n",
> + pd.serial(), get_current_destination(game)->serial(),
> items_.size());
> + pd.ship_coming(*this, true);
> Notifications::publish(NoteShip(this,
> NoteShip::Action::kDestinationChanged));
> }
>
> +void Ship::clear_destinations(Game& game) {
> + while (!destinations_.empty()) {
> + pop_destination(game, *destinations_.front().first.get(game));
> + }
> +}
> +
> +/**
> + * Remove this destination from the queue
> + */
> +void Ship::pop_destination(Game& game, PortDock& pd) {
> + pd.ship_coming(*this, false);
> + for (auto it = destinations_.begin(); it != destinations_.end(); ++it) {
> + if (it->first.get(game) == ) {
> + destinations_.erase(it);
> + reorder_destinations(game);
> + if (destinations_.empty()) {
> + molog("pop_destination(%u): no destinations and
> %" PRIuS " items left\n",
> + pd.serial(), items_.size());
> + } else {
> + molog("pop_destination(%u): rerouted to
> portdock %u (carrying %" PRIuS " items)\n",
> + pd.serial(),
> get_current_destination(game)->serial(), items_.size());
> + }
> + return;
> + }
> + }
> +}
> +
> +// Recursively find the best ordering for our destinations
> +static inline float prioritised_distance(Path& path, uint32_t priority,
> uint32_t items) {
> + return static_cast(path.get_nsteps() * items) / (priority *
> priority);
> +}
> +using DestinationsQueue = std::vector>;
> +static std::pair shortest_order(Game& game,
> + Fleet& fleet,
> + bool is_on_dock,
> + void* start,
> + const
> DestinationsQueue& remaining_to_visit,
> + const
> std::map shipping_items) {
> + const size_t nr_dests = remaining_to_visit.size();
> + assert(nr_dests > 0);
> + if (nr_dests == 1) {
> + // Recursion break: Only one portdock left
> + Path path;
> + if (is_on_dock) {
> + PortDock* p = static_cast(start);
> + if (p != remaining_to_visit[0].first) {
> + fleet.get_path(*p,
> *remaining_to_visit[0].first, path);
> + }
> + } else {
> + static_cast(start)->calculate_sea_route(game,
> *remaining_to_visit[0].first, );
> + }
> + return std::pair(remaining_to_visit,
> prioritised_distance(
> + path, remaining_to_visit[0].second,
> shipping_items.at(remaining_to_visit[0].first)));
> + }
> +
> + std::pair best_result;
> + best_result.second = std::numeric_limits::max();
> + for (const auto& pair : remaining_to_visit) {
> + DestinationsQueue remaining;
> + for (const auto& p : remaining_to_visit) {
> + if (p.first != pair.first) {
> + remaining.push_back(p);
> + }
> +
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
The detour-baselength assert will definitely not give any further trouble now – I removed it ;) Making a detour really can shorten the ship´s path by a few fields in some cases… :P The ship.get_nritems() assert is also fixed now, though the savegame might remain broken. -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
I now got Assertion failed: (ship.get_nritems() == 0), function ship_arrived, file ../src/economy/portdock.cc, line 383. after ./widelands --loadgame=~/Downloads/ShipCrash5.wgf Somwhat after 1:56... from https://www.magentacloud.de/share/tu4ayusx.k Do frisians have very special wares? -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
got another detour assert: /widelands-repo/new-shipping/src/economy/fleet.cc:890: void Widelands::Fleet::check_push_destination(Widelands::Game&, Widelands::Ship&, const Widelands::PortDock&, Widelands::PortDock&, uint32_t): Zusicherung »detour >= base_length« nicht erfüllt. Abgebrochen (Speicherabzug geschrieben) The assert is triggert close after loading this save game: https://bugs.launchpad.net/widelands/+bug/1827033/+attachment/5285703/+files/wl_autosave_00.wgf Don't know if it is related: Playing a debug build and the game stutters noticeably. The game stops for a short time. Do you want a save game close before this autosave? -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
The "detour >= base_length" assert should finally be fixed. Also changed the double-colonization check as suggested. -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
I had a look at the check for disallowing double colonization. I think it would be sufficient to check whether there's a building there, we don't care if it's a port or a construction site. It might even be safer, because the enemy might also have been faster with building something there. -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
Can you please merge trunk? I am struggling with bug 1839740 ... which is solved in trunk. I get the same crash: /widelands-repo/new-shipping/src/economy/fleet.cc:888: void Widelands::Fleet::check_push_destination(Widelands::Game&, Widelands::Ship&, const Widelands::PortDock&, Widelands::PortDock&, uint32_t): Zusicherung »detour >= base_length« nicht erfüllt. The crash happens after at least one ship arrives a port. I guess its the ship called 'Saturn' in this savgame: https://bugs.launchpad.net/widelands/+bug/1827033/+attachment/5285464/+files/test.wgf Just press 'E' and click on 'Saturn' -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
Review: Needs Fixing OK I have a ShipCrash4.wgf at https://www.magentacloud.de/share/tu4ayusx.k I start this with $ ./widelands --loadgame=~/Downloads/ShipCrash4.wgf and wait about 10 Minutes for Assertion failed: (detour >= base_length), function check_push_destination, file ../src/economy/fleet.cc, line 888. Abort trap: 6 Please consider that I have soldiers that need transport and have special wishes from Laybrinths and Dungeons. Sorrry for all thee detours :-) -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
No wonder it was buggy, I used the wrong variable for calculating the length of the detour :) Please check whether it still happens… -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
Now I was kiled here: Assertion failed: (detour >= base_length), function check_push_destination, file ../src/economy/fleet.cc, line 887. 5 widelands 0x0001102aedf9 Widelands::Fleet::check_push_destination(Widelands::Game&, Widelands::Ship&, Widelands::PortDock const&, Widelands::PortDock&, unsigned int) + 1497 (fleet.cc:887) 6 widelands 0x0001102ad69e Widelands::Fleet::push_next_destinations(Widelands::Game&, Widelands::Ship&, Widelands::PortDock const&) + 3294 (fleet.cc:843) 7 widelands 0x0001102dd544 Widelands::PortDock::ship_arrived(Widelands::Game&, Widelands::Ship&) + 2260 (portdock.cc:378) 8 widelands 0x00010fbb1cbe Widelands::Ship::ship_update_transport(Widelands::Game&, Widelands::Bob::State&) + 1166 (ship.cc:316) 9 widelands 0x00010fbb08d9 Widelands::Ship::ship_update(Widelands::Game&, Widelands::Bob::State&) + 1033 (ship.cc:265) 10 widelands 0x00010f8ea790 Widelands::Bob::do_act(Widelands::Game&) + 704 (bob.cc:196) 11 widelands 0x00010f8ea486 Widelands::Bob::act(Widelands::Game&, unsigned int) + 1030 (bob.cc:181) 12 widelands 0x00010f98d88a Widelands::CmdAct::execute(Widelands::Game&) + 570 (map_object.cc:110) 13 widelands 0x0001103d42f9 Widelands::CmdQueue::run_queue(int, unsigned int&) + 1497 (cmd_queue.cc:124) 14 widelands 0x00010e3c9b4d Widelands::Game::think() + 637 (game.cc:602) 15 widelands 0x00010f3a53c8 InteractiveBase::think() + 488 (interactive_base.cc:516) 16 widelands 0x00010f4ae6bc InteractivePlayer::think() + 572 (interactive_player.cc:232) 17 widelands 0x00010ea040e7 UI::Panel::do_think() + 151 (panel.cc:488) 18 widelands 0x00010ea0230c UI::Panel::do_run() + 2332 (panel.cc:189) ... Will try to reprodcue this ... -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
OK, I can load the Savegame again, lets continue hunting ... -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
The OPtr is just a wrapper for a MapObject. I use OPtr rather than PortDock* to avoid segfaults when the portdock is destroyed. The == comparison compares serials, so it´s 100% safe to use. I found a potential cause for this assert in the way superfluous or invalid destinations are discarded, please check whether you can still reproduce it now… -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
Next one please check: https://www.magentacloud.de/share/tu4ayusx.k
ShioCrash2.wgf
Crashes here:
Assertion failed: (old_index < nr_dests), function reorder_destinations,
file ../src/logic/map_objects/tribes/ship.cc, line 902.
With a Debugger I found...
Via the ReplayWriter game.cleanup_for_load(); is called
This in turn calls
void MapObject::remove(EditorGameBase& egbase)
void PortDock::cleanup(EditorGameBase& egbase) // why doing such a fuzz for a
cleanup?
void Ship::pop_destination(Game& game, PortDock& pd)
void Ship::reorder_destinations(Game& game)
I assume there is some issue with the == Operator from OPtr
optr(pair.first);
if (old_destinations[old_index].first == optr) {
break; // does not happen
}
This code does not really seem robust, but I still must find out what this OPtr
thing is.
Maybe when loading Objects ar kind of cloned by the loading so the code fails?
--
https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/new-shipping.
___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
I believe this is a bug in trunk since I didn´t touch that code, but I´ll fix it here. Once the portspace becomes unavailable, other ships shouldn´t be allowed to start colonizing. No savegame needed, should be easy to reproduce. I´ll also add this case to the testsuite. Still wasn´t able to reproduce any of the UnrealAssert asserts. -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
OK, found an obvious Bug: * Two ships find the same PortSpace * Tell both of them to build a port. 4: Colonization error: unloading wares to constructionsite of atlanteans_port (owner 4) failed. Wares unloaded to the site: 3, max capacity: 3, remaining to unload: 13 No free capacity to unload another ware Assertion failed: (wq.get_max_fill() > cur), function ship_update_idle, file ../src/logic/map_objects/tribes/ship.cc, line 653. The same might happen if you tell a ship to uild a port that is already (partially) equipped from the Land side Do you want a save game for tis? -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
Hello benedikt, I am back and will try more with a debugger attached, any news from your siede? -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
Mhh, it took about 15-30 minutes to trigger the asserts. I will be "on call duty" this week, so I have even less time. I tried one of the new features of trunk : order Soldiers to be sent to the port immediateley once it was build. As of some Towers "across the Sea", I was able to plan buldings next to the Port while it was build. And I connected some Ports being build to some exiting economy. I will try some AI-only setups (Well he AI fails to grasp the evil tricks needed for this Map..) I will give this some 4 hours (on 10x Speed or such). Next I will setup a debugger and give you details from "inside the code". What OS/CPU/Debugger do you use? Maybe we should recruit some tester from the tournmanent? -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
I tried out all four UnrealAssert savegames now and *none* of them triggered an assert fail. Do you have replays that end with assert-crashes? -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
Review: Needs Fixing Debugging can be FUN!, well not for you Benedikt, sorry: 5: Ullr: exploration - breaking for free sea, dir=1 TrainingSite::drop_stalled_soldiers: Kicking somebody out. Assertion failed: (detour >= base_length), function check_push_destination, file ../src/economy/fleet.cc, line 887. Abort trap: 6 Application Specific Information: Assertion failed: (old_index < nr_dests), function reorder_destinations, file ../src/logic/map_objects/tribes/ship.cc, line 902. I added two more Savegames +/- around the asserts from above https://www.magentacloud.de/share/tu4ayusx.k The _may_ be related to shipping soldiers. I "forgot" some Soldiers on the Mainland and after sending them awy I got theses assertions. Maybe adding some coordinates and the exact values to the assert may help? Ill go back to fine tuning my map ... -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
Review: Needs Fixing OK here comes thee next (me conquering the Imperial Isaland East of mine) 5: (126x 23) expedition spot scoring, colony_scan_area == 21 5: Ullr at 2x 23: PORTSPACE found, we valued it: 0 5: Ullr at 2x 23: explore uphold, visited first time 5: Ullr: continue island circumvention, dir=1 Cmd_EnemyFlagAction::execute player(3): flag->owner(2) number=4 Assertion failed: (old_index < nr_dests), function reorder_destinations, file ../src/logic/map_objects/tribes/ship.cc, line 902. Abort trap: 6 Loos like this logic needs more Braintwisting :-) But now I really _will_ stop... good night -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
Look like a) The attached game on partially reproduces the Issue (just slow as hell) b) A Millitarysite near a Port was under heavy Attack, you ca find two barbarian Inhabitat Atlanter Buidlign, ooks I lost them just during the crash. I set Autosae to Minutes ant will continue ... later. -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
Ouch. This means that you have an item in your portdock that doesn´t know where it wants to go. Happens all the time in trunk, shouldn´t happen anymore in this branch. Did you (or some AI) destroy a port shortly before you got the error? Or connected some previously unconnected ports on land? Or saved and reloaded? -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
OK, now its getting interesting. My "Unreal Water" Map got totally slow and stopped with 1: attacking site at 56x 40, score 5, with 2 soldiers, attacking 2 times, after 102 seconds Cmd_EnemyFlagAction::execute player(1): flag->owner(3) number=2 Cmd_EnemyFlagAction::execute player(3): flag->owner(2) number=3 TI(55348): destination disappeared or economy mismatch -> fail Assertion failed: (dest), function load_wares, file ../src/economy/portdock.cc, line 395. Abort trap: 6 Lets see, if I can get some savegame ... Check here; https://www.magentacloud.de/share/tu4ayusx.k UnrealAssert.wgf -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
Just started the Debugger and wondered where this this OPtr thing comes from. But whatever you did looks like you fixed it. Will go on laying my Map now until I got whatever was possible. AND thanks! -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
Another guess pushed, this one may fix it. (untested, can´t compile atm.) Please test… -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
I added the complete output to: https://www.magentacloud.de/share/tu4ayusx.k#$/ Crash.txt Will try a debugger now -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
I added some debug output, please try again… -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
Sorry still got it SaveHandler::save_game() took 2536ms Reloading the game from replay = ==99248==ERROR: AddressSanitizer: heap-use-after-free on address 0x603000c71568 at pc 0x0001102296de bp 0x7ffee2224ef0 sp 0x7ffee2224ee8 #2 0x110214b23 in Widelands::PortDock::cleanup(Widelands::EditorGameBase&) portdock.cc:200 #3 0x10f8c9822 in Widelands::MapObject::remove(Widelands::EditorGameBase&) map_object.cc:465 #4 0x10fc7d67c in Widelands::Warehouse::cleanup(Widelands::EditorGameBase&) warehouse.cc:638 #5 0x10f8c9822 in Widelands::MapObject::remove(Widelands::EditorGameBase&) map_object.cc:465 #6 0x10f8c9436 in Widelands::ObjectManager::cleanup(Widelands::EditorGameBase&) map_object.cc:167 #7 0x10db4f8f4 in Widelands::EditorGameBase::cleanup_objects() editor_game_base.h:170 #8 0x10e2d1b08 in Widelands::EditorGameBase::cleanup_for_load() editor_game_base.cc:510 #9 0x10e304d5f in Widelands::Game::cleanup_for_load() game.cc:615 #10 0x10e3eee66 in Widelands::ReplayWriter::ReplayWriter(Widelands::Game&, std::__1::basic_string, std::__1::alloca freed by thread T0 here: ... std::__1::__tree_node, void*>*, long>) set:647 #6 0x11021740f in Widelands::PortDock::ship_coming(Widelands::Ship&, bool) portdock.cc:336 #7 0x10fafac3b in Widelands::Ship::pop_destination(Widelands::Game&, Widelands::PortDock&) ship.cc:777 No idea what this replay writer does, no time for debugging right now -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
I have pushed a blind guess what might be causing this, please test whether this fixes it -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
Mhh, I have: Check for working CXX compiler: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ -- works Apple LLVM version 10.0.1 (clang-1001.0.46.4) And I get still get this crash at bzr9140[new-shipping] directy when loading that file. We had such subtule differences between compilers before. ==1286==ERROR: AddressSanitizer: heap-use-after-free on address 0x604001876498 at pc 0x00010f0ed78e bp 0x7ffee3233ef0 sp 0x7ffee3233ee8 #2 0x10f0d88cc in Widelands::PortDock::cleanup(Widelands::EditorGameBase&) portdock.cc:200 Hmm, trying a Debugger, next. -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
Whatever you did, looks like it fixed the regression tests, too. I have about no time this weeek. But this deserves some playtesting, mmh. -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
Yes, with ASan. It complains about the usual memory leak when closing Widelands, nothing else. Travis is happy now, regression tests are working there. -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
Can´t reproduce. I downloaded your savegame, loaded it, let it run for ten minutes gametime and then closed it, all without problems… -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
Loading the Savegame Unreal.wgf from https://www.magentacloud.de/share/tu4ayusx.k provokes this ASAN error in ==1418==ERROR: AddressSanitizer: heap-use-after-free on address 0x6040025b5898 at pc 0x00010b77478e ... #2 0x10b75f8cc in Widelands::PortDock::cleanup(Widelands::EditorGameBase&) portdock.cc:200 please confirm you can reproduce it and can fix the regression tests. new code looks fine. -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
Ah ok :) you meant, don´t use realtime as a limit for calculation steps. OK, if playtesting shows that such a limit is necessary (atm I don´t think it is) the limit will be implemented by max number of destinations or perhaps max total route length. -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
> be carefull not to use (local) timings ...
For some rendering code you might use an approach
long ts = current_time() + kMaxRenderingMillis;
do {
refine_textures(...)
} while (ts < current_time());
With widelands we must not do such things, got the idea?
--
https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/new-shipping.
___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
Thanks for reviewing :)
Implemented or replied to your comments.
I´ve been playing so far on Dust In The Wind where I didn´t notice performance
problems (owning ~ half the islands). A big map with many many ports would be
great for testing though.
This is sort of like the TSP with additional rules like "dynamic route lengths"
(priority weights, esp. numbers of items). But since ships that already have
destinations are highly penalized, they will be sifted out quickly and only
ships with few destinations will be chosen; so although the recursion call is
exponential, the recursion depth should likely always be shallow. (If you have
just one ship for a hundred ports, new requests will be ignored for a while
rather than assigned in a suboptimal way if your ship already has lots of
destinations.)
> * be carefull not to use (local) timings as this depends on the CPU and may
> rsult in desyncs
I don´t understand, what do you mean by local timings?
Diff comments:
> === modified file 'src/economy/fleet.cc'
> --- src/economy/fleet.cc 2019-04-24 15:11:23 +
> +++ src/economy/fleet.cc 2019-08-10 17:23:31 +
> @@ -744,107 +785,93 @@
> }
>
> if (closest_port) {
> - s->set_destination(closest_port);
> + s->push_destination(game, *closest_port);
> s->send_signal(game, "wakeup");
> }
> }
> }
>
> /**
> - * For the given three consecutive ports, decide if their path is favourable
> or not.
> - * \return true if the path from start to finish >= the path from middle to
> finish
> - */
> -bool Fleet::is_path_favourable(const PortDock& start,
> - const PortDock& middle,
> - const PortDock& finish) {
> - if ( != ) {
> - Path path_start_to_finish;
> - Path path_middle_to_finish;
> -#ifndef NDEBUG
> - assert(get_path(start, finish, path_start_to_finish));
> -#else
> - get_path(start, finish, path_start_to_finish);
> -#endif
> - if (get_path(middle, finish, path_middle_to_finish)) {
> - if (path_middle_to_finish.get_nsteps() >
> path_start_to_finish.get_nsteps()) {
> - return false;
> - }
> - }
> - }
> - return true; // default
> -}
> -
> -/**
> - * For the given ship, go through all ports of this fleet
> - * and find the one with the best score.
> - * \return that port
> - */
> -PortDock* Fleet::find_next_dest(Game& game, const Ship& ship, const
> PortDock& from_port) {
> - PortDock* best_port = nullptr;
> - float best_score = 0.0f;
> -
> - for (PortDock* p : ports_) {
> - if (p == _port) {
> - continue; // same port
> - }
> -
> - float score = 0.0f;
> - WareInstance* ware;
> - Worker* worker;
> -
> - // Score for wares/workers onboard that ship for that port
> - for (const ShippingItem& si : ship.items_) {
> - if (si.get_destination(game) == p) {
> - si.get(game, , );
> - if (ware) {
> - score += 1; // TODO(ypopezios):
> increase by ware's importance
> - } else {// worker
> - score += 4;
> - }
> - }
> - }
> -
> - // Score for wares/workers waiting at that port
> - for (const ShippingItem& si : from_port.waiting_) {
> - if (si.get_destination(game) == p) {
> - si.get(game, , );
> - if (ware) {
> - score += 1; // TODO(ypopezios):
> increase by ware's importance
> - } else {// worker
> - score += 4;
> - }
> - }
> - }
> -
> - if (score == 0.0f && p->get_need_ship() == 0) {
> - continue; // empty ship to empty port
> - }
> -
> - // Here we get distance ship->port
> - uint32_t route_length = kRouteNotCalculated;
> -
> - // Get precalculated distance if the ship is at a port
> - {
> - Path precalculated_path;
> - if (get_path(from_port, *p, precalculated_path)) { //
> try to use precalculated path
> - route_length = precalculated_path.get_nsteps();
> - }
> - }
> -
> - // Get distance for when the ship is not at a port (should not
> happen frequently)
> - if (route_length == kRouteNotCalculated) {
> - route_length =
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
Review: Needs Fixing regression tests We have a lot of regressiontests about seafaring (for a reason :-) ./regression_test.py -b ./widelands FAILs test/maps/ship_transportation.wmf/scripting/test_rip_ports_with_worker_in_transit.lua ... test/maps/ship_transportation.wmf/scripting/test_rip_first_port_with_worker_in_portdock.lua ... test/maps/ship_transportation.wmf/scripting/test_rip_first_port_with_ware_in_portdock.lua ... test/maps/ship_transportation.wmf/scripting/test_rip_second_port_with_ware_in_portdock.lua ... test/maps/ship_transportation.wmf/scripting/test_rip_farm_with_ware_and_worker_in_transit.lua ... test/maps/ship_transportation.wmf/scripting/test_rip_ports_with_ware_in_transit.lua ... test/maps/ship_transportation.wmf/scripting/test_rip_second_port_with_worker_in_portdock.lua ... One hard crash from ASAN with: ==14492==ERROR: AddressSanitizer: heap-use-after-free on address 0x6040020e6798 at pc 0x000107e3088e bp 0x7ffeea4f2790 sp 0x7ffeea4f2788 READ of size 8 at 0x6040020e6798 thread T0 #0 0x107e3088d in std::__1::__tree_end_node*>* std::__1::__tree_next_iter*>*, std::__1::__tree_node_base*>(std::__1::__tree_node_base*) __tree:176 #1 0x107e1c53a in std::__1::__tree_const_iterator*, long>::operator++() __tree:920 #2 0x107e1bc2c in Widelands::PortDock::cleanup(Widelands::EditorGameBase&) portdock.cc:200 #3 0x1074fcef2 in Widelands::MapObject::remove(Widelands::EditorGameBase&) map_object.cc:465 #4 0x1078b2d7c in Widelands::Warehouse::cleanup(Widelands::EditorGameBase&) warehouse.cc:638 ... previously allocated by thread T0 here: ... #4 0x107e3340f in std::__1::unique_ptr, std::__1::__tree_node_destructor > > > std::__1::__tree, std::__1::allocator >::__construct_node(Widelands::Ship*&&) __tree:2201 #5 0x107e32bfc in std::__1::pair*, long>, bool> std::__1::__tree, std::__1::allocator >::__emplace_unique_key_args(Widelands::Ship* const&, Widelands::Ship*&&) __tree:2147 #6 0x107e327ab in std::__1::__tree, std::__1::allocator >::__insert_unique(Widelands::Ship*&&) __tree:1276 #7 0x107e1eb3f in std::__1::set, std::__1::allocator >::insert(Widelands::Ship*&&) set:635 #8 0x107e1e3af in Widelands::PortDock::ship_coming(Widelands::Ship&, bool) portdock.cc:330 #9 0x10772d84b in Widelands::Ship::push_destination(Widelands::Game&, Widelands::PortDock&) ship.cc:763 #10 0x107df0509 in Widelands::Fleet::push_next_destinations(Widelands::Game&, Widelands::Ship&, Widelands::PortDock const&) fleet.cc:872 #11 0x107e1fa50 in Widelands::PortDock::ship_arrived(Widelands::Game&, Widelands::Ship&) portdock.cc:374 #12 0x10772239d in Widelands::Ship::ship_update_transport(Widelands::Game&, Widelands::Bob::State&) ship.cc:315 #13 0x107720fb8 in Widelands::Ship::ship_update(Widelands::Game&, Widelands::Bob::State&) ship.cc:265 #14 0x107458bff in Widelands::Bob::do_act(Widelands::Game&) bob.cc:194 Caused by FAIL: test/maps/ship_transportation.wmf/scripting/test_rip_portdock_with_worker_and_ware_in_transit.lua Please tell me how I may help you -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
Did a first review, the code is ok, but some more comments
would help me to grasp me the semantics of some stuctures.
and please refactor out some of the bigger loops.
(https://de.wikipedia.org/wiki/Refactoring)
Please check my inline comments.
Do you have an idea how big the perfomance impact may be for a Map with a lot
of Ports.
e.g. two AI Players on the Nile or my new Map
https://www.widelands.org/maps/unreal-waterflow/
You are solving a kind of
https://de.wikipedia.org/wiki/Problem_des_Handlungsreisenden
or https://en.wikipedia.org/wiki/Travelling_salesman_problem
this has potential for exponential (or NP-Hard) complexity. So maybe we should
put in some limits so that we dod not stop the game dunring these calculations.
e.g.
* limit the number of ports on a route to 4/8/16?
* stop some calcualtions after a giiven mount of steps?
* be carefull not to use (local) timings as this depends on the CPU and may
rsult in desyncs
Next I will run the regression tests.
Maybe its worth to design a big Map with a lot of Ports (128) to test this?
Diff comments:
> === modified file 'src/economy/fleet.cc'
> --- src/economy/fleet.cc 2019-04-24 15:11:23 +
> +++ src/economy/fleet.cc 2019-08-10 17:23:31 +
> @@ -699,6 +700,46 @@
> if (route_length == kRouteNotCalculated) {
> route_length = s->calculate_sea_route(game, *p);
> }
> + if (fallback) {
> + // This ship is employed transporting wares,
> lower its priority drastically
Please make this a function of its own add some more coments
> + const uint32_t real_length = route_length;
> + uint32_t malus = 1;
> + uint32_t index = 0;
As wee need an index anyway a "classic" loop would avoid inconsistencies when
we add continue statements later?
> + PortDock* iterator = nullptr;
> + uint32_t shortest_detour =
> std::numeric_limits::max();
> + uint32_t best_index;
> + for (const auto& pair : s->destinations_) {
> + PortDock* pd = pair.first.get(game);
> + Path path;
> + uint32_t detour;
> + if (iterator == p) {
> + detour = 0;
> + } else if (iterator) {
> + get_path(*iterator, *p, path);
> + detour = path.get_nsteps();
> + } else {
> + s->calculate_sea_route(game,
> *p, );
> + detour = path.get_nsteps();
> + }
> + if (p != pd) {
> + get_path(*p, *pd, path);
> + detour += path.get_nsteps();
> + }
> + if (detour < shortest_detour) {
> + shortest_detour = detour;
> + best_index = index;
> + }
> + malus += pair.second;
> + iterator = pd;
> + ++index;
> + }
> + route_length += shortest_detour * best_index;
> + if (route_length + shortest_detour >
> real_length * malus) {
> + // Unreasonably long detour
> + continue;
> + }
> + route_length *= malus;
> + }
>
> if (route_length < shortest_dist) {
> shortest_dist = route_length;
> @@ -744,107 +785,93 @@
> }
>
> if (closest_port) {
> - s->set_destination(closest_port);
> + s->push_destination(game, *closest_port);
> s->send_signal(game, "wakeup");
> }
> }
> }
>
> /**
> - * For the given three consecutive ports, decide if their path is favourable
> or not.
> - * \return true if the path from start to finish >= the path from middle to
> finish
> - */
> -bool Fleet::is_path_favourable(const PortDock& start,
> - const PortDock& middle,
> - const PortDock& finish) {
> - if ( != ) {
> - Path path_start_to_finish;
> - Path
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
Thanks for picking this up, as I am just teesting some new Mpa with a lot of Ports, I can combine two things here. I dont have that much time the next weeks, though. -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/new-shipping into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
