[Widelands-dev] [Merge] lp:~widelands-dev/widelands/frisian-buildings into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/frisian-buildings into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/frisian-buildings/+merge/349856 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/frisian-buildings. ___ 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/frisian-buildings into lp:widelands
Continuous integration builds have changed state: Travis build 3709. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/406860500. Appveyor build 3508. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_frisian_buildings-3508. -- https://code.launchpad.net/~widelands-dev/widelands/frisian-buildings/+merge/349856 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/frisian-buildings. ___ 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/inputwarequeue_display into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/inputwarequeue_display into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/inputwarequeue_display/+merge/350385 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/inputwarequeue_display. ___ 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/frisian_music into lp:widelands
I would like to play around with the sheet music a bit because I think some of the harmonies can be improved, and trying to explain it with words doesn't work. This will have to wait until August though. -- https://code.launchpad.net/~widelands-dev/widelands/frisian_music/+merge/350429 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/frisian_music 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/frisian_music into lp:widelands
Yep I composed that song, you can find it here as sheet music: https://musescore.com/user/18659851/scores/515845 -- https://code.launchpad.net/~widelands-dev/widelands/frisian_music/+merge/350429 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/frisian_music 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/bug-1736901-shift-market into lp:widelands
Review: Resubmit Sphinx is fixed, so this branch is ready now -- https://code.launchpad.net/~widelands-dev/widelands/bug-1736901-shift-market/+merge/349630 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1736901-shift-market. ___ 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/inputwarequeue_display into lp:widelands
@bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/inputwarequeue_display/+merge/350385 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/inputwarequeue_display. ___ 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/frisian_music into lp:widelands
Continuous integration builds have changed state: Travis build 3705. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/406816829. Appveyor build 3504. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_frisian_music-3504. -- https://code.launchpad.net/~widelands-dev/widelands/frisian_music/+merge/350429 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/frisian_music 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/inputwarequeue_display into lp:widelands
Review: Approve I can't reproduce my bug any longer either, so as far as I am concerned this can go in. Thanks! -- https://code.launchpad.net/~widelands-dev/widelands/inputwarequeue_display/+merge/350385 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/inputwarequeue_display. ___ 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/frisian-buildings into lp:widelands
Review: Approve You have done a beautiful job on those buildings! :) There are 2 buildings that I'd still like to see changed in a follow-up branch though: The Port and the Tower. Their tower heights work really well as works of art, but they are obscuring the fields behind them, so it becomes a UI issue. @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/frisian-buildings/+merge/349856 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/frisian-buildings. ___ 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/remove-savegame-compatibility-after-economy-change into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/remove-savegame-compatibility-after-economy-change into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/remove-savegame-compatibility-after-economy-change/+merge/349390 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/remove-savegame-compatibility-after-economy-change. ___ 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/inputwarequeue_display into lp:widelands
Implemented your suggestion. WaresQueue returns only a variable, but WorkersQueue calls std::vector::size(), so this is more efficient. -- https://code.launchpad.net/~widelands-dev/widelands/inputwarequeue_display/+merge/350385 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/inputwarequeue_display 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/remove-savegame-compatibility-after-economy-change into lp:widelands
@bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/remove-savegame-compatibility-after-economy-change/+merge/349390 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/remove-savegame-compatibility-after-economy-change. ___ 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/inputwarequeue_display into lp:widelands
1 small comment for performance Diff comments: > === modified file 'src/economy/input_queue.cc' > --- src/economy/input_queue.cc2018-04-07 16:59:00 + > +++ src/economy/input_queue.cc2018-07-22 09:47:47 + > @@ -120,6 +120,13 @@ > update(); > } > > +uint32_t InputQueue::get_missing() const { > + if (get_filled() >= max_fill_ || request_ == nullptr || > !request_->is_open()) { How is get_filled() calculated? It it's more than just returning a variable, it will be more efficient to have const auto currently_filed = get_filled(); at the top. > + return 0; > + } > + return max_fill_ - get_filled() - std::min(max_fill_, > request_->get_num_transfers()); > +} > + > constexpr uint16_t kCurrentPacketVersion = 3; > > void InputQueue::read(FileRead& fr, Game& game, MapObjectLoader& mol) { -- https://code.launchpad.net/~widelands-dev/widelands/inputwarequeue_display/+merge/350385 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/inputwarequeue_display 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/frisian_music into lp:widelands
Benedikt Straub has proposed merging lp:~widelands-dev/widelands/frisian_music into lp:widelands. Commit message: Includes Klaus Halfmann's Frisian-style music Requested reviews: Widelands Developers (widelands-dev) Related bugs: Bug #1780804 in widelands: "A song for the Frisisans" https://bugs.launchpad.net/widelands/+bug/1780804 For more details, see: https://code.launchpad.net/~widelands-dev/widelands/frisian_music/+merge/350429 -- Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/frisian_music into lp:widelands. === added file 'data/music/ingame_27.ogg' Binary files data/music/ingame_27.ogg 1970-01-01 00:00:00 + and data/music/ingame_27.ogg 2018-07-22 12:32:51 + differ ___ 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/inputwarequeue_display into lp:widelands
I think I found it. request_->get_num_transfers() is actually the right function here, but merely passing through it´s return value isn´t enough. I made InputQueue::get_missing() more intelligent to account for capacity and existing inputs itself. I can´t reproduce the bug anymore now. I also added an assert to check that the calculations at least make sense. -- https://code.launchpad.net/~widelands-dev/widelands/inputwarequeue_display/+merge/350385 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/inputwarequeue_display 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/inputwarequeue_display into lp:widelands
I´ll look into that bug… it can´t be the fault of get_num_transfers() because although I provide that function, I don´t use it for the calculations, so the problem is probably that request_->get_open_count() doesn´t do what it should… -- https://code.launchpad.net/~widelands-dev/widelands/inputwarequeue_display/+merge/350385 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/inputwarequeue_display 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