Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands
What has been common practice when Widelands started development, is not the favorite approach anymore and doesn't have to remain the same practice forever. Big fails in prediction is not in the nature of programming, but in the nature of old-fashioned development. Collaborative development is not an exception. The industry has moved forward (although not all parts of it) and a project like Widelands would only benefit from reconsidering its paleolithic approach (and this is something that the rest of Widelands developers seemed to realize). No, I don't need to deal with that stuff in our era. It's not my business anymore, but the ideal point in time to reconsider a project's approach is right after a major release, so don't lose that rare-in-Widelands opportunity. There are good chances that you will be wondering why you didn't do it earlier. I have not been in search for a community, I didn't come here begging to belong somewhere. I offered to help, but I got "embraced" with immediate doubts and trash. I tried my best to consider it as a problem of communication or culture. So I gave it a chance and proved that I can contribute (which I did in various fields, including the most demanding one). In the end it was not a problem of communication, but one of realism. Even abandoning the project could be considered as a form of helping it wake up. And I don't believe that I'm a special case. Not many developers would tolerate that environment. Widelands community is suffering from isolationism. Open doors cannot help when minds are closed. This is a well-known pattern in open-source projects. -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/355510 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___ 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/ship_scheduling_2 into lp:widelands
Yes, introducing new developers - especially ones that are as capable as you are - definitely does speed up things. Repeatedly spending a lot of time, energy and frustration on discussions like this one however does not. And yes, development can always take longer that one has projected, which is the nature of things. Also, going into feature freeze is a common practice that you will need to learn how to deal with if you're still interested in working collaboratively on software projects this size, even if they are just for fun. I'm sorry that we didn't manage to communicate better and that you have not been happy here. I wish you all the best and that you will find a community that will suit you better. -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/355510 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___ 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/ship_scheduling_2 into lp:widelands
It is funny how you still mention "this instance" when the scale of things here is whole months. Of course introducing new bugs doesn't speed-up things, but I don't remember you ever bothering with project's speed to begin with. Still introducing developers can definitely speed-up things. Especially those that are familiar with the introduced bugs, and those able for real development instead of merely hacking existing hacks. Which mistake was more serious in the longterm shouldn't be hard to see. The anti-congestion algorithm hardly introduced any bugs, it basically brought on the surface pre-existing ones. In any case, nobody cared for my time here, so I don't care if my contributions will be used or not. I have long moved on. Happy coding. -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/355510 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/355510 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___ 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/ship_scheduling_2 into lp:widelands
Considering all the desyncs and unexpected bugs we were struggling with, do you really think adding a complex new algorithm to the code would have made things any more predictable, or speeded up the release in any way whatsoever? It would have potentially slowed things even further. I'm sorry that's not fast enough for you, but when you work together with other people on a project of this size in our spare time, things don't happen right this instant. Merging the anti-congestion algorithm before the release was a mistake and cost us more time, because we had some transport bugs coming up and needed to find out where it came from. Adding it back in will cost us even more time. -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/355510 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___ 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/ship_scheduling_2 into lp:widelands
You can end a discussion, but you cannot help your case. Quite expectedly, the "few weeks" of the "release coming shortly" turned out to be more than half a year (and still counting). Congratulations for the upcoming release, and I wish it to really happen and not to be "too little too late". I also hope that all those months you had your "fun" and maybe you got a lesson on realism. "Widelands is too mature for the player base", but the players base (and even the developers base), although being a beast of patience, it is not mature enough for a project like Widelands. That used to be an educated analysis, but today is a proven reality. Hopefully the persons who got insulted by the analysis, will not get also insulted by reality itself. -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/355510 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___ 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/ship_scheduling_2 into lp:widelands
@bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/355510 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___ 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/ship_scheduling_2 into lp:widelands
Please stop insulting everybody whenever you don't get your way. Yes, the release cycles are too long. No, we won't solve the problem of managing testing exposure by merging your feature right now this very instant. Changing the release process in the middle of a release is not going to happen, because that's far too stressful for me. I'm only human and this is supposed to be a fun hobby. End of discussion. -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/355510 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___ 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/ship_scheduling_2 into lp:widelands
"Widelands is too mature for the player base to find bugs like that acceptable in a release." Unless you have asked the player-base about the matter, the above statement is arbitrary. If you dare, you can conduct a poll on the issue. Projects much bigger and much more serious than Widelands have no problem releasing code that is not tested in every possible way, exactly to involve the user-base in the testing process (even more so when it comes to complex control flow). The endless loop you fixed after many months, would have been both reported and fixed much earlier if there was a release to expose it to users. Most importantly, the vast majority of modern user-bases has no more problem dealing with minor bugs, as long as they have an easy way to report them and the programmers are eager to fix them in soon-to-follow bug-fixing releases. Users can be as patient as the programmers, especially non-paying users. Of course all that may leave your "German mentality" untouched, and this is not the place for a theoretical discussion. Still motivation is no less important than patience, and Widelands needs developers at least as much as it needs users. Or so I was told, among other non-truths, like that there are more testers than developers. On the bright side, it is good to know the practical value of my contributions, before undertaking something as big as fixing the routing or the AI. -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/355510 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___ 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/ship_scheduling_2 into lp:widelands
Well, I have fully mature branches that have been sitting around for a year that won't make it in because they are still waiting for a code review slot... I could just call them "UI bugs" and insist on having them in Build 20, delaying the release until who knows when, maybe another year? Every changed line of code has the potential for bugs, and as we have seen from the failing test suite, there are edge cases that don't immediately come up during gameplay testing. You are providing some complex control flow in your branches, so the potential for undiscovered new bugs is high. I have just fixed an endless loop that was introduced in February and only reported last week. So, I don't think that we should risk it with the release coming shortly. Widelands is too mature for the player base to find bugs like that acceptable in a release. Also, the release procedure that we are using has been developed by my predecessor over the course of > 15 years, so I'm expecting there to be reasons for it to be like it is. Patience is part of the game, I'm afraid. If you are too demotivated to carry on by having to wait for a few weeks until a feature gets in, here is nothing that we can do about it. I'd be sad to see you leave :'( -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/355510 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___ 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/ship_scheduling_2 into lp:widelands
You keep calling that a feature, while it is mostly a fix for the long-standing poor scheduling, so technically it still applies despite the feature freeze. Furthermore, the freeze was announced on 2018-09-17, while this branch has been approved on 2018-09-14. The late problems with the test suite were just that, problems with the test suite, not with the branch. If you insist to be strict with that, you are free to be, but this will inevitably freeze my motivation as well. -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/355510 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___ 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/ship_scheduling_2 into lp:widelands
No, I did mean Build 21. We are in feature freeze, and feature freeze means it's feature freeze for everybody. Otherwise, we'll never get a release out, because there will always be that one cool feature we'll want to squeeze in. It's very tempting. Having an assignee in TODO comment would only make sense if development was paid in this project, which it isn't. Changing the format now will mean changing a bazillion TODO comments in the code base + changing the codecheck rule. I already cleaned them up once and I'm not eager to do it again ;) -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/355510 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___ 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/ship_scheduling_2 into lp:widelands
Thanks for clarifying the TODO tags. I would expect the creator to be written on the left and the assignee on the right of it. The only way for this branch to slow down the game is if the ports and the ships become hundreds. Even in that case, it should be lighter than the previous code, so I don't think that there is a problem with it. Did you mean Build 20? I don't see why it has to wait for Build 21. If there is indeed a problem with the code, it would be easier for me to address it now, before the code gets off my brain's cache. -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/355510 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___ 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/ship_scheduling_2 into lp:widelands
Review: Approve The TODOs get tagged with the person who created them, so we can get back to them later in case we don't understand the TODO. It doesn't mean that you have to fix it personally ;) Test suite is green now, and I have also done a saveloading test with a savegame from trunk. Running 8 AIs on The Nile on auto_speed slowed down to a crawl before 4 hours gametime, but we are having a problem in trunk (https://bugs.launchpad.net/widelands/+bug/1795976), so it might not be related to this branch. This will make a great feature for Build 21, thank you! :) -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/355510 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands
Continuous integration builds have changed state: Travis build 4094. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/437518591. Appveyor build 3890. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ship_scheduling_2-3890. -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/355510 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___ 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/ship_scheduling_2 into lp:widelands
Concerning the specific TODO, the code for invalid destinations pre-existed my involvement and is suspicious enough. I just moved it earlier in the function, in order to catch more cases. I don't know which are those cases (they don't happen in any part of the code that I touched), only that some of them appear when loading a savegame. I'm not familiar with that part of the codebase, so maybe this TODO is not for me. -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/355510 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands
Continuous integration builds have changed state: Travis build 4079. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/435223747. Appveyor build 3875. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ship_scheduling_2-3875. -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/355510 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___ 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/ship_scheduling_2 into lp:widelands
This is not perfect, but should be enough for this branch. -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/355510 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___ 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/ship_scheduling_2 into lp:widelands
Yes, the simulation should be correct. Calling the sink_ship function is calling the sink_ship function, and it doesn't matter if the caller is a Lua script or a UI button. Both will send off the same player command. If they don't, it's a bug. And yes, we do want Widelands not to get into an undefined state when a user performs an action, even if it's sinking a ship or destroying a port. We allow those user actions, so they need to be supported. I have been thinking about this some more, changes in the test suite should be OK in the following 2 cases: 1. The new algorithm needs a bit more time to get back into a well-defined state than the old algorithm. In this case, add a sleep statement to the test suite. 2. The new algorithm gets back into a well-defined state which is different from the well-defined state that the old algorithm used to have. Change the asserts to reflect the new well-defined state. In all other cases, the new algorithm needs fixing. -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/355510 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___ 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/ship_scheduling_2 into lp:widelands
Are you sure that there is proper simulation of user-actions here? Cause I suspect that there is just a magical removal, without calling e.g. sinking function. Of course the code should cover cases of destruction, but how to do that if it is not properly called? The old code was checking every time every combination of everything, wasting resources. Do we really want to do things that way? -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/355510 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___ 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/ship_scheduling_2 into lp:widelands
This removal of ports and ships simulated actions that can be taken by a user, so yes, the new code needs to be able to deal with that. Users can sink ships and destroy ports. Ports can also be destroyed if the enemy conquers the territory. -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/355510 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___ 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/ship_scheduling_2 into lp:widelands
>From what I understand, the test suite forces the removal of ports and ships >without running clean-up code. That invalidates the state of transports and >breaks the advanced logic. I'm not sure if it is worthy to change the logic, >instead of changing the tests. -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/355510 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___ 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/ship_scheduling_2 into lp:widelands
There are still failures in the test suite. Instructions on how to run it manually: https://wl.widelands.org/wiki/RegressionTests/#test-scenarios -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/355510 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands
ypopezios has proposed merging lp:~widelands-dev/widelands/ship_scheduling_2
into lp:widelands.
Commit message:
Improved ware/worker routing algorithm for ships and portdocks.
Requested reviews:
GunChleoc (gunchleoc)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/355510
See description of the branch:
https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2
--
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/ship_scheduling_2.
=== modified file 'src/economy/fleet.cc'
--- src/economy/fleet.cc 2018-09-05 06:42:21 +
+++ src/economy/fleet.cc 2018-09-21 19:46:09 +
@@ -300,7 +300,7 @@
*
* @return true if successful, or false if the docks are not actually part of the fleet.
*/
-bool Fleet::get_path(PortDock& start, PortDock& end, Path& path) {
+bool Fleet::get_path(const PortDock& start, const PortDock& end, Path& path) {
uint32_t startidx = std::find(ports_.begin(), ports_.end(), &start) - ports_.begin();
uint32_t endidx = std::find(ports_.begin(), ports_.end(), &end) - ports_.begin();
@@ -628,8 +628,7 @@
}
/**
- * Act callback updates ship scheduling. All decisions about where transport ships
- * are supposed to go are made via this function.
+ * Act callback updates ship scheduling of idle ships.
*
* @note Do not call this directly; instead, trigger it via @ref update
*/
@@ -637,230 +636,207 @@
act_pending_ = false;
if (!active()) {
- // If we are here, most likely act() was called by a port with waiting wares or an expedition
- // ready
- // although there are still no ships. We can't handle it now, so we reschedule the act()
- schedule_act(game, 5000); // retry in the next time
+ // If we are here, most likely act() was called by a port with waiting wares or
+ // with an expedition ready, although there are still no ships.
+ // We can't handle it now, so we reschedule the act()
+ schedule_act(game, kFleetInterval); // retry in the next time
act_pending_ = true;
return;
}
molog("Fleet::act\n");
- // we need to calculate what ship is to be send to which port
- // for this we will have temporary data structure with format
- // <,score>
- // where ship and port are not objects but positions in ports_ and ships_
- // this is to allow native hashing
- std::map, uint16_t> scores;
-
- // so we will identify all pairs: idle ship : ports, and score all such
- // pairs. We consider
- // - count of wares onboard, first ware (oldest) is counted as 8 (prioritization)
- // (counting wares for particular port only)
- // - count wares waiting at the port/3
- // - distance between ship and a port (0-10 points, the closer the more points)
- // - is another ship heading there right now?
-
- // at the end we must know if requrests of all ports asking for ship were addressed
- // if any unsatisfied, we must schedule new run of this function
- // when we send a ship there, the port is removed from list
- std::list waiting_ports;
-
- // this is just helper - first member of scores map
- std::pair mapping; // ship number, port number
-
- // first we go over ships - idle ones (=without destination)
- // then over wares on these ships and create first ship-port
- // pairs with score
- for (uint16_t s = 0; s < ships_.size(); s += 1) {
- if (ships_[s]->get_destination(game)) {
- continue;
- }
- if (ships_[s]->get_ship_state() != Ship::ShipStates::kTransport) {
- continue; // in expedition obviously
- }
-
- for (uint16_t i = 0; i < ships_[s]->get_nritems(); i += 1) {
- PortDock* dst = ships_[s]->items_[i].get_destination(game);
- if (!dst) {
-// if wares without destination on ship without destination
-// such ship can be send to any port, and should be sent
-// to some port, so we add 1 point to score for each port
-for (uint16_t p = 0; p < ports_.size(); p += 1) {
- mapping.first = s;
- mapping.second = p;
- scores[mapping] += 1;
-}
-continue;
- }
-
- bool destination_found = false; // Just a functional check
- for (uint16_t p = 0; p < ports_.size(); p += 1) {
-if (ports_[p] == ships_[s]->items_[i].get_destination(game)) {
- mapping.first = s;
- mapping.second = p;
- scores[mapping] += (i == 0) ? 8 : 1;
- destination_found = true;
-}
- }
- if (!destination_found) {
-// Perhaps the throw here is too strong
-// we can still remove it before stable release if it proves too much
-// during my testing this situation never happened
-throw wexception("A ware with destination that does not match any of player's"
- " ports, ship %u, ware's destination: %u",
- ships_[s]->serial(),
- ships_[s]->items_[i].get_destination(game)->serial());
- }
- }
- }
-
- // now opposite aproach - we go over ports to find out those that have wares
- // waiting for ship then find candidate ships to satisfy the requests
- for (uint16_t p
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands has been updated. Status: Needs review => Superseded For more details, see: https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/354019 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands
Refusing to merge, since Travis is not green. Use @bunnybot merge force for merging anyways. Travis build 3977. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/428723984. -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/354019 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___ 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/ship_scheduling_2 into lp:widelands
Review: Needs Fixing One of the tests in our test suite has an endless loop in it: test/maps/ship_transportation.wmf/scripting/test_rip_ports_with_worker_in_transit.lua This already happened with older versions of this branch, so I don't think that it's anything I did. -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/354019 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___ 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/ship_scheduling_2 into lp:widelands
Review: Approve I had actually programmed a really nice endless loop with those iterators... all fixed now. I have also improved the savegame compatibility, so it's only calculated once during game loading. Thanks for this great feature! :) @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/354019 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands has been updated. Commit message changed to: Improved routing algorithm for ships and portdocks. For more details, see: https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/354019 -- Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ship_scheduling_2 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands has been updated. Commit message changed to: Improved ware/worker routing algorithm for ships and portdocks. For more details, see: https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/354019 -- Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ship_scheduling_2 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands
Continuous integration builds have changed state: Travis build 3961. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/428035209. Appveyor build 3759. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ship_scheduling_2-3759. -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/354019 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ship_scheduling_2 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands
Added inline comments concerning the NOCOM comments.
Diff comments:
>
> === modified file 'src/economy/portdock.cc'
> --- src/economy/portdock.cc 2018-04-16 07:03:12 +
> +++ src/economy/portdock.cc 2018-09-04 18:46:42 +
> @@ -281,44 +286,70 @@
> }
> }
>
> -void PortDock::update_shippingitem(Game& game,
> std::vector::iterator it) {
> +void PortDock::update_shippingitem(Game& game,
> std::list::iterator it) {
> it->update_destination(game, *this);
>
> - PortDock* dst = it->get_destination(game);
> + const PortDock* dst = it->get_destination(game);
> assert(dst != this);
>
> // Destination might have vanished or be in another economy altogether.
> if (dst && dst->get_economy() == get_economy()) {
> - set_need_ship(game, true);
> + if (ships_coming_ <= 0) {
> + set_need_ship(game, true);
> + }
> } else {
> it->set_location(game, warehouse_);
> it->end_shipping(game);
> *it = waiting_.back();
> waiting_.pop_back();
> -
> - if (waiting_.empty())
> - set_need_ship(game, false);
> - }
> -}
> -
> -/**
> - * A ship has arrived at the dock. Clear all items designated for this dock,
> - * and load the ship.
> + }
> +}
> +
> +/**
> + * Receive shipping item from unloading ship.
> + * Called by ship code.
> + */
> +void PortDock::shipping_item_arrived(Game& game, ShippingItem& si) {
> + si.set_location(game, warehouse_);
> + si.end_shipping(game);
> +}
> +
> +/**
> + * Receive shipping item from departing ship.
> + * Called by ship code.
> + */
> +void PortDock::shipping_item_returned(Game& game, ShippingItem& si) {
> + si.set_location(game, this);
> + waiting_.push_back(si);
> +}
> +
> +/**
> + * A ship changed destination and is now coming to the dock. Increase
> counter for need_ship.
> + */
> +void PortDock::ship_coming(bool affirmative) {
> + if (affirmative) {
> + ++ships_coming_;
> + } else {
> + // Max used for compatibility with old savegames
> + // NOCOM We should shift the savegame compatibility into
> PortDock::Loader::load it at all possible.
> + // Increment kCurrentPacketVersion and write separate loading
> code for both versions.
> + // Which case is for compatibility, and which case is the
> actual case that we want now?
> + ships_coming_ = std::max(0, ships_coming_ - 1);
The current line is for the compatibility case. The line for the actual case
would be simply "--ships_coming_".
> + }
> +}
> +
> +/**
> + * A ship has arrived at the dock. Set its next destination and load it
> accordingly.
> */
> void PortDock::ship_arrived(Game& game, Ship& ship) {
> - std::vector items_brought_by_ship;
> - ship.withdraw_items(game, *this, items_brought_by_ship);
> -
> - for (ShippingItem& shipping_item : items_brought_by_ship) {
> - shipping_item.set_location(game, warehouse_);
> - shipping_item.end_shipping(game);
> - }
> + ship_coming(false);
>
> if (expedition_ready_) {
> assert(expedition_bootstrap_ != nullptr);
>
> // Only use an empty ship.
> if (ship.get_nritems() < 1) {
> + ship.set_destination(nullptr);
> // Load the ship
> std::vector workers;
> std::vector wares;
> @@ -337,46 +368,71 @@
> // The expedition goods are now on the ship, so from
> now on it is independent from the port
> // and thus we switch the port to normal, so we could
> even start a new expedition,
> cancel_expedition(game);
> - return fleet_->update(game);
> + fleet_->update(game);
> + return;
> }
> }
>
> - if (ship.get_nritems() < ship.descr().get_capacity() &&
> !waiting_.empty()) {
> - uint32_t nrload =
> -std::min(waiting_.size(),
> ship.descr().get_capacity() - ship.get_nritems());
> -
> - while (nrload--) {
> - // Check if the item has still a valid destination
> - if (waiting_.back().get_destination(game)) {
> - // Destination is valid, so we load the item
> onto the ship
> - ship.add_item(game, waiting_.back());
> + // Decide where the arrived ship will go next
> + PortDock* next_port = fleet_->find_next_dest(game, ship, *this);
> + if (!next_port) {
> + ship.set_destination(next_port);
> + return; // no need to load anything
> + }
> +
> + // Unload any wares/workers onboard the departing ship which are not
> favored by next dest
> + ship.unload_unfit_items(game, *this, *next_port);
> +
>
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands
I have pushed a commit with a code review and some small tweaks. Please have a look at the NOCOM comments. Not tested yet. -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/354019 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ship_scheduling_2 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands
Reason for appveyor not working anymore currently is that the glbinding package has been upgraded in the repo of MSYS from 2.1.4 to 3.0.2.1 which seems to have issues with our code now. -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/354019 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ship_scheduling_2 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands
AppVeyor seems broken. Otherwise, this branch is now mature and big enough. Not going to add anything more into it, so as to keep it easier for reviewers. If people test it, it can make it into build 20. -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/354019 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ship_scheduling_2 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands
ypopezios has proposed merging lp:~widelands-dev/widelands/ship_scheduling_2
into lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/354019
See description of the branch:
https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands.
=== modified file 'src/economy/fleet.cc'
--- src/economy/fleet.cc 2018-04-07 16:59:00 +
+++ src/economy/fleet.cc 2018-08-30 06:13:48 +
@@ -628,8 +628,7 @@
}
/**
- * Act callback updates ship scheduling. All decisions about where transport ships
- * are supposed to go are made via this function.
+ * Act callback updates ship scheduling of idle ships.
*
* @note Do not call this directly; instead, trigger it via @ref update
*/
@@ -637,230 +636,178 @@
act_pending_ = false;
if (!active()) {
- // If we are here, most likely act() was called by a port with waiting wares or an expedition
- // ready
- // although there are still no ships. We can't handle it now, so we reschedule the act()
- schedule_act(game, 5000); // retry in the next time
+ // If we are here, most likely act() was called by a port with waiting wares or
+ // with an expedition ready, although there are still no ships.
+ // We can't handle it now, so we reschedule the act()
+ schedule_act(game, kFleetInterval); // retry in the next time
act_pending_ = true;
return;
}
molog("Fleet::act\n");
- // we need to calculate what ship is to be send to which port
- // for this we will have temporary data structure with format
- // <,score>
- // where ship and port are not objects but positions in ports_ and ships_
- // this is to allow native hashing
- std::map, uint16_t> scores;
-
- // so we will identify all pairs: idle ship : ports, and score all such
- // pairs. We consider
- // - count of wares onboard, first ware (oldest) is counted as 8 (prioritization)
- // (counting wares for particular port only)
- // - count wares waiting at the port/3
- // - distance between ship and a port (0-10 points, the closer the more points)
- // - is another ship heading there right now?
-
- // at the end we must know if requrests of all ports asking for ship were addressed
- // if any unsatisfied, we must schedule new run of this function
- // when we send a ship there, the port is removed from list
- std::list waiting_ports;
-
- // this is just helper - first member of scores map
- std::pair mapping; // ship number, port number
-
- // first we go over ships - idle ones (=without destination)
- // then over wares on these ships and create first ship-port
- // pairs with score
- for (uint16_t s = 0; s < ships_.size(); s += 1) {
- if (ships_[s]->get_destination(game)) {
- continue;
- }
- if (ships_[s]->get_ship_state() != Ship::ShipStates::kTransport) {
- continue; // in expedition obviously
- }
-
- for (uint16_t i = 0; i < ships_[s]->get_nritems(); i += 1) {
- PortDock* dst = ships_[s]->items_[i].get_destination(game);
- if (!dst) {
-// if wares without destination on ship without destination
-// such ship can be send to any port, and should be sent
-// to some port, so we add 1 point to score for each port
-for (uint16_t p = 0; p < ports_.size(); p += 1) {
- mapping.first = s;
- mapping.second = p;
- scores[mapping] += 1;
-}
-continue;
- }
-
- bool destination_found = false; // Just a functional check
- for (uint16_t p = 0; p < ports_.size(); p += 1) {
-if (ports_[p] == ships_[s]->items_[i].get_destination(game)) {
- mapping.first = s;
- mapping.second = p;
- scores[mapping] += (i == 0) ? 8 : 1;
- destination_found = true;
-}
- }
- if (!destination_found) {
-// Perhaps the throw here is too strong
-// we can still remove it before stable release if it proves too much
-// during my testing this situation never happened
-throw wexception("A ware with destination that does not match any of player's"
- " ports, ship %u, ware's destination: %u",
- ships_[s]->serial(),
- ships_[s]->items_[i].get_destination(game)->serial());
- }
- }
- }
-
- // now opposite aproach - we go over ports to find out those that have wares
- // waiting for ship then find candidate ships to satisfy the requests
- for (uint16_t p = 0; p < ports_.size(); p += 1) {
- PortDock& pd = *ports_[p];
- if (!pd.get_need_ship()) {
- continue;
- }
-
- // general stategy is "one ship for port is enough", but sometimes
- // amount of ware waiting for ship is too high
- if (count_ships_heading_here(game, &pd) * 25 > pd.count_waiting()) {
- continue;
- }
-
- waiting_ports.push_back(p);
-
- // scoring and entering the pair into scores (or increasing existing
- // score if the pair is already
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands has been updated. Status: Needs review => Superseded For more details, see: https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/352335 -- Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ship_scheduling_2 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands
Continuous integration builds have changed state: Travis build 3855. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/44908. Appveyor build 3653. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ship_scheduling_2-3653. -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/352335 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ship_scheduling_2 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands has been updated. Description changed to: See description of the branch: https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2 Get windows builds and ask for testing - do not review yet For more details, see: https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/352335 -- Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ship_scheduling_2 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands has been updated. Commit message changed to: For more details, see: https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/352335 -- Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ship_scheduling_2 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands
Continuous integration builds have changed state: Travis build 3758. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/412187264. Appveyor build 3558. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ship_scheduling_2-3558. -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/352335 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ship_scheduling_2 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands
Continuous integration builds have changed state: Travis build 3754. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/412020649. Appveyor build 3554. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ship_scheduling_2-3554. -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/352335 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ship_scheduling_2 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands
ypopezios has proposed merging lp:~widelands-dev/widelands/ship_scheduling_2
into lp:widelands.
Commit message:
Get windows builds and ask for testing.
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/352335
See description of the branch:
https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands.
=== modified file 'src/economy/fleet.cc'
--- src/economy/fleet.cc 2018-04-07 16:59:00 +
+++ src/economy/fleet.cc 2018-08-04 06:22:49 +
@@ -637,9 +637,9 @@
act_pending_ = false;
if (!active()) {
- // If we are here, most likely act() was called by a port with waiting wares or an expedition
- // ready
- // although there are still no ships. We can't handle it now, so we reschedule the act()
+ // If we are here, most likely act() was called by a port with waiting wares or
+ // with an expedition ready, although there are still no ships.
+ // We can't handle it now, so we reschedule the act()
schedule_act(game, 5000); // retry in the next time
act_pending_ = true;
return;
@@ -647,212 +647,95 @@
molog("Fleet::act\n");
- // we need to calculate what ship is to be send to which port
- // for this we will have temporary data structure with format
- // <,score>
- // where ship and port are not objects but positions in ports_ and ships_
- // this is to allow native hashing
- std::map, uint16_t> scores;
-
- // so we will identify all pairs: idle ship : ports, and score all such
- // pairs. We consider
- // - count of wares onboard, first ware (oldest) is counted as 8 (prioritization)
- // (counting wares for particular port only)
- // - count wares waiting at the port/3
- // - distance between ship and a port (0-10 points, the closer the more points)
- // - is another ship heading there right now?
-
- // at the end we must know if requrests of all ports asking for ship were addressed
- // if any unsatisfied, we must schedule new run of this function
- // when we send a ship there, the port is removed from list
+ // We need to calculate what ship is to be sent to which port,
+ // so we will score all pairs: idle ship : port.
+ // For the score of each pair, we consider:
+ // - count of wares onboard that ship for that port
+ // - count of wares waiting at that port
+ // - distance between that ship and that port
+
+ // At the end we must know if the request of any port for a ship
+ // is still unsatisfied, to schedule new run of this function.
+ // This list holds all waiting ports and we remove every port where we send a ship.
std::list waiting_ports;
-
- // this is just helper - first member of scores map
- std::pair mapping; // ship number, port number
-
- // first we go over ships - idle ones (=without destination)
- // then over wares on these ships and create first ship-port
- // pairs with score
- for (uint16_t s = 0; s < ships_.size(); s += 1) {
- if (ships_[s]->get_destination(game)) {
- continue;
- }
- if (ships_[s]->get_ship_state() != Ship::ShipStates::kTransport) {
+ for (uint16_t pi = 0; pi < ports_.size(); ++pi) {
+ if (ports_[pi]->get_need_ship()) {
+ waiting_ports.push_back(pi);
+ }
+ }
+
+ for (Ship* s : ships_) {
+ if (s->get_destination(game)) {
+ continue; // has already destination
+ }
+ if (s->get_ship_state() != Ship::ShipStates::kTransport) {
continue; // in expedition obviously
}
-
- for (uint16_t i = 0; i < ships_[s]->get_nritems(); i += 1) {
- PortDock* dst = ships_[s]->items_[i].get_destination(game);
- if (!dst) {
-// if wares without destination on ship without destination
-// such ship can be send to any port, and should be sent
-// to some port, so we add 1 point to score for each port
-for (uint16_t p = 0; p < ports_.size(); p += 1) {
- mapping.first = s;
- mapping.second = p;
- scores[mapping] += 1;
-}
-continue;
- }
-
- bool destination_found = false; // Just a functional check
- for (uint16_t p = 0; p < ports_.size(); p += 1) {
-if (ports_[p] == ships_[s]->items_[i].get_destination(game)) {
- mapping.first = s;
- mapping.second = p;
- scores[mapping] += (i == 0) ? 8 : 1;
- destination_found = true;
-}
- }
- if (!destination_found) {
-// Perhaps the throw here is too strong
-// we can still remove it before stable release if it proves too much
-// during my testing this situation never happened
-throw wexception("A ware with destination that does not match any of player's"
- " ports, ship %u, ware's destination: %u",
- ships_[s]->serial(),
- ships_[s]->items_[i].get_destination(game)->serial());
- }
- }
- }
-
- // now opposite aproach - we go over ports to find out those that have wares
- // waiting for ship th
