[Widelands-dev] [Merge] lp:~widelands-dev/widelands/ferry into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/ferry into lp:widelands has been updated. Status: Needs review => Work in progress For more details, see: https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 5418. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/582382256. Appveyor build 5188. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-5188. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 5408. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/582051006. Appveyor build 5178. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-5178. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Pushed a fix for worker creation. As soon as you now trigger an economy update (e.g. by building a flag), the necessary tools are transported from the HQ to the warehouse and there turned into workers. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Hm, at this point I'm wondering whether it might be easier to allow ferries to carry workers and have them waiting for it at the nearest flag. Then we would not need the separate economies. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
No, the workshop just skips everything because no tools are needed. I´m trying to implement that the tools will be transported from one warehouse to another one so the missing workers can be created in the correct economy… -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Looks like the metal workshop 'hangs', it always shows 'The Economy doesn't need the ware Bradpaddle', although the game run more than 5 Minutes. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 5382. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/580127580. Appveyor build 5152. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-5152. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
When I have 2 economies only liked by a waterway, the metal workshop won't produce the necessary wares - I expect the request is generated by the wrong warehouse. Savegame: https://bugs.launchpad.net/widelands/+bug/1584203/comments/50 -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
And another thing that can be improved in checkstep, but will probably be non-trivial: https://bugs.launchpad.net/widelands/+bug/1584203/comments/48 I'd be OK with doing this in a follow-up branch. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
I have pushed a commit that shuts up clang compiler warnings, so you need to pull before continuing to work. I think that Checkstep should ignore waterways that don't have ferries - I have attached a savegame to illustrate the issue. https://bugs.launchpad.net/widelands/+bug/1584203/comments/47 -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Implemented your suggestion for Ferry and clarified the comment in EconomyDataPacket. Economy::create_requested_workers is called directly from balance() for every economy, so we can just ignore it for ware ecos. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
I have pushed a commit with a comment. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Savegame compatibility hack is up :) Thanks for the review and codestyle. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 5359. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/578495337. Appveyor build 5129. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-5129. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 5359. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/578495337. Appveyor build 5129. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-5129. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
I have pushed another commit with code review. I'm done with this for now. I also did a grep on data and test for lua changes, and it looks like we didn't miss anything. The following 2 bits in the code are a bit complicated and could do with testing: * worker creation over multiple economies * Barracks * ferry paths, especially around coasts For ferry oceans separated very narrowly, how about allowing portage there? This might simplify the CheckStepFerry. https://en.wikipedia.org/wiki/Portage Just an idea, don't feel obligagted to implement this. The following new Lua functions don't have tests yet (we can implement those in a follow-up branch though): * get_waterway_max_length * set_waterway_max_length * get_ferry -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
I have pushed a new commit with a code style review. I'll have to return later to go through the discussion. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
I just had an idea how I might just perhaps be able to write savegame compatibility, give me a few days to try it out... -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
I get some text conflicts when merging trunk. Benedikt, can look into this and make the call for merging? -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
I am also for merging into trunk to get better testing. Personally I dont like splitting the economy into two parallel economies, but if there is a consensus about this... -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
The AI will simply ignore the new feature, it will continue playing exactly as before. There are some AI code parts that should be looked into eventually (I added TODOs there), but they should most likely have no effect whatever as long as it doesn´t build a waterway. This branch was tested quite extensively, IMHO it would be better to merge this now and report any bugs that might still be contained against trunk. (Only downside is that this branch breaks savegame compatibility.) -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Will it be problem if the AI can't handle ferrys? Need this branch more testing? -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
The status regarding AI is that it is not supported and probably not be anytime soon if ever. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Ohh, what a history, but I must confess , I lost track. Can we merge this now (as only map with a falg are affcetd anyway?) -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
What´s the status here? -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 5300. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/569856062. Appveyor build 5073. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-5073. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 5254. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/551923331. Appveyor build 5033. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-5033. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 5251. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/551745114. Appveyor build 5030. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-5030. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 5208. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/548539708. Appveyor build 4987. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-4987. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
I am thinking a lot about this feature, but for AI it is relay difficult. On most maps it will not be applicable and AI should be very cautious to use it at all. It has complex logic for implementation. So the conclusion is that it is not worth the effort to implement in AI. What could be considered to make it more AI-friendly: - to allow transfer of workers on ferries - not to produce more ferries than needed - allow transport of ferries over roads, or at least extend work radius of ferryyard significantly - introduce WATERROAD_CAPS, and allow a waterroad between two such fields, or WALK_MOVECAPS and WATERROAD_CAPS. These WATERROAD_CAPS can be calculated and/or set by map creator (to restrict fields where waterroad can be built) - not to route the wares over unattended waterroads (to avoid transportation blockages) But I know that all of that violates your intentions for this branch, but an improvement in one of few of them would make AI implementation easier. I will still keep thinking about this, because idea is interesting... -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Whether waterways/ferries are enabled can be checked as map.get_waterway_max_length() >= 2 Typically, they are enabled only on maps where they make sense. Waterways have a maximum length that is map-dependent. If two flags can be connected both by a new road and by a new waterway, the AI should build a waterway only if a road connection is much much longer. Building waterways if a road is also possible nearby makes sense in a handful of corner cases but not in the majority of cases. Also, if there are two flags with the same ware economy but different worker economies, the AI should try very hard to connect them with a normal road if somehow possible. > It would be nice if the ferries were per economy Completely impossible, sorry. A ferry can be sent to any waterway that is in the same FerryFleet as itself – and there can be multiple economies adjacent to the same ocean (not to mention that each economy can be adjacent to multiple oceans). Per-economy distribution (or bookkeeping) of ferries is just not possible. > and AI would know in advance if the waterroad will be supplied by a ferry > immediately You could either – Build the waterway; some 10 seconds later query its fleet whether a ferry is coming, and if not, destroy the new waterway and remember not to retry for some time. – Or if you really want to check before you build a waterway: Check out FerryFleet::find_other_fleet, and copy the pathfinding code from there to obtain the ferryfleet that belongs to the ocean you are interested in; then you can query that fleet for unoccupied ferries. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Also - another idea: It would be nice if the ferries were per economy and AI would know in advance if the waterroad will be supplied by a ferry immediately = if there is an available ferry. Without painfully calculating whatever. The logic would be - AI is considering connecting flags A and B - no matter where they are. If ferries are available - it can use pathfinding including waterroads, otherwise it falls back to current one. But even this logic is stupid, because there will be non-critical sections where waterroads will be luxury and waste of precious material - but AI will not be able to distinguish such spots... -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Another question is how can AI know if ferries are necessary. In case of ships - it builds shipyard and ports always - if there are portspaces. With ferries it can check if map has a water - but this is not sufficient check. Most maps have water, but will not need ferries at all. So I see a lot of issues with implementing it in AI. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
AI currently is not able to cope with multiple oceans in the case of ships, just to remind this fact... -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
No, because it can be close to several unconnected oceans, just like a shipyard. You could keep a list of all ferry-reachable fields within the workarea of a ferry yard (*), and then check for a given waterway whether a CheckStepFerry-approved path exists from any node on the waterway to any of these fields. (* this is actually an incorrect approximation in some corner cases but that shouldn´t matter normally) -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
And is there any link between ferryyard and a fleet? Can I query the waterroad if if has a ferryyard to provide it with ferry? -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
To get all ferries in a fleet: Waterway::get_fleet() or Ferry::get_fleet() FerryFleet::count_ferries() Counting stuff per player should not be needed, count per FerryFleet instead. I´ll add some functions to query a waterway whether it has a ferry, and to query the fleet how many waterways are unemployed. Btw, the FerryFleet does not keep a list of all waterways that belong to it. They are remembered only while they need a ferry; when they get one, the ferryfleet forgets that waterway. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Is there any function that returns number of ferries for a player or Fleet? Also I presume there is a way how to query the waterroad if it has a ferry... -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
For ships the single ship is enough to attend all ports, with ferries it is the opposite, you need the same number of ferries as waterroads. It would be convenient if AI can built "virtual roads" - from flag to flag, but if crossing the water this virtual road would break down into 3 roads - 2x actual roads and one waterroad. But this is more coding and bigger complexity -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Launchpad limits the diff preview to the first 5000 lines. The other >9000
lines remain hidden…
A MOVECAPS_FERRY is not possible. For MOVECAPS_WALK and MOVECAPS_SWIM, you
always know that if two adjacent nodes both have this movecap, a bob that also
has that movecap can pass directly from one node to the other. For ferries, it
is possible that a waterway may pass both of two adjacent nodes, but it might
not be allowed to go directly from the one to the other. They might even be in
different oceans. Imagine e.g. situation with two lakes separated by a
1-tile-wide strip of land. If you want to check whether a spot is suited for
waterways at all, always use CheckStepFerry::reachable_dest.
The equivalent of the function you linked is easily possible. Instead of plain
CheckStepRoad, use this:
Widelands::CheckStepAnd cstep;
cstep.add(Widelands::CheckStepFerry(egbase()));
cstep.add(Widelands::CheckStepRoad(player, Widelands::MOVECAPS_SWIM |
Widelands::MOVECAPS_WALK));
Also note that in lines like
if (dynamic_cast(map[reachable_coords].get_immovable())) {
Road should be replaces by RoadBase (the superclass of Road and Waterway).
Also keep in mind that waterways can transport only wares, no workers. The AI
should build one between two flags only if they belong to the same worker
economy, or if their respective worker economies both are "functional" (i.e.
have a warehouse). Just so it doesn´t build a constructionsite on an
unreachable shore – a road is still needed so a builder can come.
You need to build one ferry yard per Ferry Ocean, just as you need one shipyard
per Ship Ocean. Ferries can use every path that is also suited for ships plus
narrow channels where ships can´t pass, so a ferry ocean tends to encompass
more places than a ship ocean. Ferries are implemented similar to ships,
they´ll swim around idly until assigned to a waterway, then they swim to it
autonomously.
And it´s not that much additional management that´s needed (less than e.g. for
ports and ships), it only seems so because the concept is so different ;)
--
https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/ferry.
___
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/ferry into lp:widelands
Can ferries be transferred by ships? If not in some maps more then 1 ferryyard can be needed. I had an impression that waterroads will work the same as normal roads, but now I see that quite a lot of management is needed for them... -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
I cannot see that method in the diff, so it is probably something older, but fine... AI will need to use CheckStepFerry or whatever to get list of all fields from which regular road can lead, I will investigate it... Please look at this code: https://bazaar.launchpad.net/~widelands-dev/widelands/trunk/view/head:/src/ai/defaultai.cc#L3849 It would be convenient if equivalent of this was possible for waterroads, what do you think? I still would like to have MOVECAPS_FERRY capability, so that AI would take random flag, and if it has this capability it would run map.find_reachable_fields() from this point to collect possible destinations for waterroads... These are just few thougts for discussion... -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
There is a method Game::send_player_build_waterway(int32_t, Path&) that works just like for roads. Waterways have a special placement rule: They can go along any edge which is located between two water triangles. They can therefore reach all fields with MOVECAPS_SWIM plus some other fields. Always use a CheckStepFerry when checking ferry or waterway paths. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
AI uses send_player_build_road() to build roads, I tried to find something like send_player_build_waterway() but nothing like this exists, correct? Also, does particular fields have some MOVECAPS, like for roads: f->nodecaps() & MOVECAPS_WALK -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Diff comments: > > === modified file 'src/ai/ai_help_structs.h' > --- src/ai/ai_help_structs.h 2019-05-25 08:20:22 + > +++ src/ai/ai_help_structs.h 2019-05-31 11:03:05 + > @@ -78,6 +78,7 @@ > kUpgradeExtends, > kLogRefiner, > kIronMine, > + // TODO(Nordfriese): Someone should update the AI code to handle > buildings that need waterways enabled The Ferry Yard is enabled only on maps with waterways allowed. It should get some special handling code, like the shipyard. And the barbarian and frisian weaving mill are now enabled if seafaring and/or ferries are allowed (previously if and only if seafaring is allowed). Not sure if the AI automatically adapts to that change (esp. on maps with ferries allowed and seafaring forbidden) or some tweak is needed. > kNeedsSeafaring, > kSupportingProducer, > kNeedsBerry, -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
I added some question in the diff Diff comments: > > === modified file 'src/ai/ai_help_structs.h' > --- src/ai/ai_help_structs.h 2019-05-25 08:20:22 + > +++ src/ai/ai_help_structs.h 2019-05-31 11:03:05 + > @@ -78,6 +78,7 @@ > kUpgradeExtends, > kLogRefiner, > kIronMine, > + // TODO(Nordfriese): Someone should update the AI code to handle > buildings that need waterways enabled Can you explain this? We have some new buildings that needs ferries? > kNeedsSeafaring, > kSupportingProducer, > kNeedsBerry, -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
I'll need to set aside a weekend for this, so it can take some time until I get around to it. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Now, how do we get progress here? Will try to play this again on Cavisson -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 5106. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/539673312. Appveyor build 4887. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-4887. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 5077. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/538358799. Appveyor build 4857. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-4857. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 5075. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/538201201. Appveyor build 4855. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-4855. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
We are merging a lot of branches today, so maybe do the merging tomorrow and ping me so I'll take over and have a look? -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
That assert is in trunk – it´s our famous Fleet::is_path_favourable problem... A reason this is not being merged yet is, GunChleoc wanted to check if we can merge this without breaking savegame compatibility (I think it´s not possible). I agree that apart from that this branch should be merged soon, and any not-yet-found bugs reported then on trunk. Will merge trunk soon to fix the diverging -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Review: Needs Fixing testplay I played Calvission again fo about 4 hours gametime. This was fine for the most parts, I found some of the expected Issued: e.g. Buildings not attachted to roads dont get workers. I found one Issue where ferries are not built. But now I hit an seertion that should be reproducible, please check: https://www.magentacloud.de/share/tu4ayusx.k Unferry.wgf If we really want this feature, we should get it in _now_ and fix such bugs with seperate bugs. Otherwsie this Brnach will diverge more and more. Lets get this decided soon. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 5000. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/533393374. Appveyor build 4781. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-4781. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Thanks for the savegame :) There were three different bugs overlayed here. All of them are fixed now. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Here is the SaveGame: https://www.magentacloud.de/share/tu4ayusx.k Ferries2.wgf (Multiplayer Game) -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
This one function with this one assert yet again... yes, please provide a savegame -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Review: Needs Fixing Played Calvission a #1 with a Barbarian at #2 , Imperial at #3 and Frisisan at #4 Got an assert aftree trying to build two woodcutters: Forcing flag at (70, 164) Message: adding warehouse for player 1 at (69, 163) Forcing flag at (73, 163) Forcing flag at (66, 165) Forcing flag at (65, 169) Forcing flag at (157, 27) Message: adding warehouse for player 2 at (157, 26) Forcing flag at (168, 141) Message: adding warehouse for player 3 at (168, 140) Forcing flag at (56, 45) Message: adding warehouse for player 4 at (56, 44) InternetGaming: Received a client list update with 18 items. InternetGaming: Received a game list update with 1 items. Assertion failed: (wh), function get_next_step, file ../src/economy/transfer.cc, line 186. Abort trap: 6 4 widelands 0x0001049b4f48 Widelands::Transfer::get_next_step(Widelands::PlayerImmovable*, bool&) + 3320 (transfer.cc:186) 5 widelands 0x0001049bc711 Widelands::WareInstance::update(Widelands::Game&) + 1841 (ware_instance.cc:330) 6 widelands 0x0001048f73f4 Widelands::Flag::add_ware(Widelands::EditorGameBase&, Widelands::WareInstance&) + 1524 (flag.cc:462) 7 widelands 0x000102f0cbfd Widelands::Worker::dropoff_update(Widelands::Game&, Widelands::Bob::State&) + 2381 (worker.cc:2123) 8 widelands 0x000102abc710 Widelands::Bob::do_act(Widelands::Game&) + 704 (bob.cc:195) Do you need a savegame? -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 4865. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/527627946. Appveyor build 4646. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-4646. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Fieldaction code looks OK. How about passing wwWORKER/wwWARE to get_roadbase and checking for that there? Then you could just call get_roadbase as in your original attempt in r8874. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 4855. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/527387898. Appveyor build 4636. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-4636. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
The worker already is in an impossible situation, so you need to load an older savegame that doesn´t contain the bug yet. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Loading may savegame I now get: Assertion failed: (wh), function get_next_step, file ../src/economy/transfer.cc, line 188. Do I have to restart that Map now? -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 4849. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/526968452. Appveyor build 4630. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-4630. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
I now get a compile Error at:
../src/economy/transfer.cc:166:72: error: non-pointer operand type
'Widelands::MapObjectType' incompatible with nullptr
if (!curflag.get_roadbase(nextflag, request_->get_type() ==
wwWORKER ? MapObjectType::ROAD : nullptr)) {
^ ~~~
../src/economy/transfer.cc:184:78: error: non-pointer operand type
'Widelands::MapObjectType' incompatible with nullptr
if (!nextflag.get_roadbase(nextnextflag,
request_->get_type() == wwWORKER ? MapObjectType::ROAD : nullptr)) {
--
https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/ferry.
___
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/ferry into lp:widelands
Thanks for the video, bug should be fixed. I had overlooked the possibility that two flags can be connected by a road and a waterway. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
> I noticed some odd Behavior that carriers tried to use a waterway to reach > their destination, but got stuck on the waterway. This way neither wares no carriers where transported in the end. That sounds like a fairly serious bug. Could you upload the savegame to the bugreport linked to this branch please? -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Review: Approve compile, review, testplay The ccode is basiccally OK, I did not read all of it, sorry. I noticed some odd Behavior that carrriees tried to use a wterway to reach theire destination, but got stuck on the waterway. This way neither wares no carriers where transported in the end. I think we cann still merge this one (nice work Benedikt!) and create Bugtickets later. I got a saveagme and some movie showing the Problem. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Plaaying Calvission for a while now: In case a location is reachabel by road and by waterway, but there is no ferry, wares may get stuck waiting at the flag for the waterway, mmh. But well, works as designed. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
The automated builds only warn about some unused variables which I now removed. I don´t see the warning in economy.cc either there or when compiling myself… -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
CLang gives me some comppiler warnings:
src/economy/economy.cc:854:45: warning: loop variable 'r' has type
'const std::pair &'
(aka 'const pair &')
but is initialized with type
'std::__1::__map_iterator,
std::__1::__tree_node, void *> *, long> >::value_type'
(aka 'pair')
resulting in a copy [-Wrange-loop-analysis]
for (const std::pair& r : open_requests) {
^
../src/economy/economy.cc:854:8: note: use non-reference type
'std::pair' (aka 'pair') to keep the copy or type 'const
std::__1::__map_iterator,
std::__1::__tree_node, void *> *, long> >::value_type &'
(aka 'const pair &') to prevent copying
for (const std::pair& r : open_requests)
I think you need more const inside the declaration of r?
/src/wui/economy_options_window.cc:20:
../src/wui/economy_options_window.h:56:25: warning: private field 'type_' is
not used
[-Wunused-private-field] Widelands::WareWorker type_;
You should see them in the automated builds, too
--
https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/ferry.
___
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/ferry into lp:widelands
OK this is a little Monster and in depends on some workarea change? Please direkt me to that branch or was this mereged by now? Im checking this out now. and (try to) do some code review. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 4836. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/526029169. Appveyor build 4617. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-4617. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
New behaviour is implemented and tested. @GunChleoc Could you please re-review the latest changes in economy.cc, especially create_requested_worker(), and CheckStepFerry? -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 4759. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/523086402. Appveyor build 4543. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-4543. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 4752. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/522535128. Appveyor build 4537. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-4537. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 4742. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/522276339. Appveyor build 4527. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-4527. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Sounds like a plan :) -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Autodetection is not possible because there are paths where you can build both. Having one mode for both where you get to choose on finish whether you want a road or waterway is possible but difficult to implement because of different placement rules. +1 for showing the flagaction window instead of entering roadbuilding mode after placing a flag where both can be built if the option is enabled. Will implement this next week. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Or maybe the 2 functions could be the same thing, with Widelands detecting automatically whether it will become a road or a waterway? -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 4720. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/520014396. Appveyor build 4506. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-4506. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Review: Approve testing This is really a nice feature. Played two games (Map Twinkling Waves and The Nile) a lot but found no strange issues anymore. There is one small thing: If you have the option 'Start building road after placing a flag' turned on, and place a flag at a coast, roadbuilding mode is immediately enabled. So if you wanted to start building a waterway, you have to make few more clicks to get into 'build waterway' mode (cancel roadbuilding -> click on flag -> click on build waterway). No idea how to solve this in an easy manner. Just show the flag action window is a flag is placed where also a waterway can be build, regardless of the option 'Start building road after placing a flag'? -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 4696. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/517213969. Appveyor build 4482. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-4482. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ferry 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/ferry into lp:widelands
Can´t reply this in the bug report for some reason…: Let W and E be the end flags of the waterway that has a ferry. – Destroy both waterways. – Build a waterway from W to a new flag F on the tiny island south of the warehouse. It´ll get the ferry that used to service the waterway from W to E. – Build a waterway from F to E. It´ll get a new ferry. – Now rebuild the other waterway – and it´ll also get a new ferry. I´m no closer to finding the source of the problem yet though. I suspect the underlying reason may be related to the pathfinder… -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ferry 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 4682. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/516878831. Appveyor build 4468. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-4468. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ferry 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/ferry into lp:widelands
Uploaded the changes discussed in the bug report. Ferries can now travel along any edge where both adjacent triangles are navigable terrain. I cleaned up the economy code, removed lots of duplicate functionality for wares and workers, and fixed a bug in worker creation. If you have two road networks connected only by waterways, and economy A requests a fisherman but all fishing nets are in economy B, nothing will happen. This is intentional because it mirrors the behaviour when a needed worker requires two tools which are in different warehouses. Newly produced fishing nets will be distributed to warehouses independent of such requests. Micromanaging is needed to get a fishing net from B to A, then it´ll be used to create a fisher as soon as it arrives there. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ferry 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 4653. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/511594641. Appveyor build 4440. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-4440. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ferry 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/ferry into lp:widelands
I did not think of the several oceans problem. I think it would be good if you could fix both the new and the original road code. This branch is huge anyway, so I don't mind having more changes in it. I can look into the saveloading if you want. Just leave the annotations in until everything else has been finalized and I'll have a look if I can make it work. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ferry 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/ferry into lp:widelands
Implemented or replied to your diff comments in revision 8852. I can´t reproduce the ASan complaint you reported anymore, but I fixed several others that surfaced now, except for one on game-end that will follow soon. Regarding the ferry target quantity: IMHO the ferry yards work like shipyards, which also produce until the player stops them manually. A target quantity would be especially difficult if the ferry yard is close two several oceans. Some of the waterway code which you annotated is copied 1:1 from road code. Should I therefore leave it as it is, or change both the new waterway code and the original road code? And saveloading – I don´t think it is really possible to preserve compatibility here, see my comment in MapFlagPacket… -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ferry 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 4638. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/510643337. Appveyor build 4425. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-4425. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ferry 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/ferry into lp:widelands
The overlays look fine now, thank you. I have pushed a commit with a code review. Check the "NOCOM" comments. Some notes that are not in the review commit: - Ferry yard needs an economy check, just like the Ox Farm etc. Otherwise, it will just keep producing en masse. - switch is faster than if/else, so use that for distinguishing ware/worker cases if it keeps the code still readable - And ASan is not happy. I built a ferry yard, then a waterway. Connected a construction site with a separate flag to the other end of the waterway and waited a bit. Only then did I connect the worker economy -> boom TI(2857): destination appears to have become split from current location -> fail = ==32501==ERROR: AddressSanitizer: heap-use-after-free on address 0x61100075fb38 at pc 0x55b7a7a1a6e7 bp 0x7ffcd58678f0 sp 0x7ffcd58678e0 READ of size 8 at 0x61100075fb38 thread T0 #0 0x55b7a7a1a6e6 in __gnu_cxx::__normal_iterator > >::__normal_iterator(Widelands::Ferry** const&) /usr/include/c++/7/bits/stl_iterator.h:780 #1 0x55b7a7a127c5 in std::vector >::begin() /usr/include/c++/7/bits/stl_vector.h:564 #2 0x55b7a7a079d2 in Widelands::Fleet::cancel_ferry_request(Widelands::Game&, Widelands::Waterway*) ../src/economy/fleet.cc:533 #3 0x55b7a7a5b870 in Widelands::Waterway::cleanup(Widelands::EditorGameBase&) ../src/economy/waterway.cc:227 #4 0x55b7a7084ea7 in Widelands::MapObject::remove(Widelands::EditorGameBase&) ../src/logic/map_objects/map_object.cc:434 #5 0x55b7a7081626 in Widelands::ObjectManager::cleanup(Widelands::EditorGameBase&) ../src/logic/map_objects/map_object.cc:162 #6 0x55b7a6ccb2d7 in Widelands::EditorGameBase::cleanup_objects() ../src/logic/editor_game_base.h:170 #7 0x55b7a6f6dd40 in Widelands::Game::run(UI::ProgressWindow*, Widelands::Game::StartGameType, std::__cxx11::basic_string, std::allocator > const&, bool, std::__cxx11::basic_string, std::allocator > const&) ../src/logic/game.cc:554 #8 0x55b7a6c6cef0 in WLApplication::new_game() ../src/wlapplication.cc:1320 #9 0x55b7a6c6b29c in WLApplication::mainmenu_singleplayer() ../src/wlapplication.cc:1177 #10 0x55b7a6c6a3d3 in WLApplication::mainmenu() ../src/wlapplication.cc:1082 #11 0x55b7a6c61699 in WLApplication::run() ../src/wlapplication.cc:454 #12 0x55b7a6c5dbee in main ../src/main.cc:44 #13 0x7f0401bfbb96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96) #14 0x55b7a6c5da69 in _start (/media/cuideigin/Linux/sources/widelands/ferry/widelands+0xd94a69) -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ferry 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/ferry into lp:widelands
Removed the duplicate code and the superfluous animation frames, downscaled the remaining images. The barbarian ferry yard now uses reed instead of cloth. The textures for built waterways are now better visible. I agree about the range indicators. That will be easy to implement, but I´ll wait with that until the workareas branch is merged. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ferry 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/ferry into lp:widelands
Barbarian Ferry Yard needs cloth to build, but there is no Weaving Mill,
because that building needs seafaring.
It would be nice to have some indicators for the range when building a new
waterway - I think we best wait with that until after we have the new work
areas, because there could be some common code there. It would also be nice to
have visual indicators for established waterways as an overlay.
Can you please remove the ferry animations except for the first frame each?
They cause the diff to be truncated, because there are so many of them. Since
they are just placeholders, 1 frame should be good enough for visual quality.
Code review of the bits that I can see is done.
Diff comments:
>
> === modified file 'data/scripting/mapobjects.lua'
> --- data/scripting/mapobjects.lua 2017-02-08 18:48:32 +
> +++ data/scripting/mapobjects.lua 2019-03-19 09:56:51 +
> @@ -31,13 +31,30 @@
> --:arg basename: The basename of the animation files. The filenames of
> the animation files need to have the format
> ``_(e|ne|se|sw|w|nw)_\d+.png``
> --:arg hotspot: The hotspot coordinates for blitting, e.g. ``{ 2, 20 }``.
> --:arg fps: Frames per second. Only use this if the animation has more
> than 1 frame, and if you need to deviate from the default frame rate.
> -function add_walking_animations(table, animationname, dirname, basename,
> hotspot, fps)
> - if (fps ~= nil) then
> - for idx, dir in ipairs{ "ne", "e", "se", "sw", "w", "nw" } do
> - table[animationname .. "_" .. dir] = {
> -pictures = path.list_files(dirname .. basename .. "_" .. dir ..
> "_??.png"),
> -hotspot = hotspot,
> -fps = fps,
> +function add_walking_animations(table, animationname, dirname, basename,
> hotspot, fps, scale)
> + if (fps ~= nil and scale ~= nil) then
> + for idx, dir in ipairs{ "ne", "e", "se", "sw", "w", "nw" } do
> + table[animationname .. "_" .. dir] = {
> +pictures = path.list_files(dirname .. basename .. "_" .. dir ..
> "_??.png"),
> +hotspot = hotspot,
> +fps = fps,
> +scale = scale,
> + }
> + end
> + elseif (fps ~= nil) then
> + for idx, dir in ipairs{ "ne", "e", "se", "sw", "w", "nw" } do
> + table[animationname .. "_" .. dir] = {
> +pictures = path.list_files(dirname .. basename .. "_" .. dir ..
> "_??.png"),
> +hotspot = hotspot,
> +fps = fps,
> + }
> + end
> + elseif (scale ~= nil) then
scale is deprecated, because it will be replaced by mipmaps.
> + for idx, dir in ipairs{ "ne", "e", "se", "sw", "w", "nw" } do
> + table[animationname .. "_" .. dir] = {
> +pictures = path.list_files(dirname .. basename .. "_" .. dir ..
> "_??.png"),
> +hotspot = hotspot,
> +scale = scale,
> }
>end
> else
>
> === modified file 'data/tribes/atlanteans.lua'
> --- data/tribes/atlanteans.lua2018-07-29 13:49:09 +
> +++ data/tribes/atlanteans.lua2019-03-19 09:56:51 +
> @@ -20,7 +20,9 @@
> --
> --**animations**: Global animations. Contains subtables for ``frontier``
> and ``flag``. Each animation needs the parameters ``pictures`` (table of
> filenames) and ``hotspot`` (2 integer coordinates), and may also define
> ``fps`` (integer frames per second).
> --
> ---**roads**: The file paths for the tribes' road textures in 2 subtables
> ``busy`` and ``normal``
> +--**roads**: The file paths for the tribe's road textures in 3 subtables
> ``busy``, ``normal`` and ``waterway``.
> +--
> +--**resource_indicators**: The names for the resource indicators. This
> table contains a subtable for each resource name plus a subtable named "" for
> no resources. Each subtable is an array, in which the index of each entry is
> the highest amount of resources the indicator may indicate.
Duplicate - remove
> --
> --**resource_indicators**: The names for the resource indicators. This
> table contains a subtable for each resource name plus a subtable named "" for
> no resources. Each subtable is an array, in which the index of each entry is
> the highest amount of resources the indicator may indicate.
> --
> @@ -73,6 +77,34 @@
> "tribes/images/atlanteans/roadt_normal_00.png",
> "tribes/images/atlanteans/roadt_normal_01.png",
>},
> + waterway = {
> + "tribes/images/atlanteans/waterway_0.png",
> + },
> + },
> +
> + resource_indicators = {
Duplicate - remove
> + [""] = {
> + [0] = "atlanteans_resi_none",
> + },
> + coal = {
> + [10] = "atlanteans_resi_coal_1",
> + [20] = "atlanteans_resi_coal_2",
> + },
> + iron = {
> + [10] = "atlanteans_resi_iron_1",
> + [20] = "atlanteans_resi_iron_2",
> + },
> + gold = {
> + [10] = "atlanteans_resi_gold_1",
> + [20] = "atl
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 4616. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/508307520. Appveyor build 4403. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-4403. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ferry 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 4592. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/505198456. Appveyor build 4356. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-4356. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ferry 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 4569. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/503691873. Appveyor build 4356. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-4356. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ferry 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 4317. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/462525707. Appveyor build 4112. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-4112. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ferry 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 4316. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/462452641. Appveyor build 4111. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-4111. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ferry 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
