Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands-website/update_ops_script into lp:widelands-website

Review: Resubmit Toni, as discussed in IRC. ptal. Kaputtnik, jup, will take care of merging. -- https://code.launchpad.net/~widelands-dev/widelands-website/update_ops_script/+merge/369163 Your team Widelands Developers is subscribed to branch lp:widelands-website.

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands-website/update_ops_script into lp:widelands-website

> # > -# This script requires root access. > +# This script requires sudo. > > set -ex > > -if [ -z "$STY" ] && [ -z "$TMUX" ]; then > +if [ -z "${TMUX}" ]; then > echo "Run inside screen or tmux in case SSH gets updated." &g

[Widelands-dev] [Merge] lp:~widelands-dev/widelands-website/update_ops_script into lp:widelands-website

SirVer has proposed merging lp:~widelands-dev/widelands-website/update_ops_script into lp:widelands-website. Commit message: Adapt the update script for the new server. Requested reviews: Widelands Developers (widelands-dev) For more details, see: https://code.launchpad.net/~widelands-dev

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/fix_context_selection into lp:widelands

SirVer has proposed merging lp:~widelands-dev/widelands/fix_context_selection into lp:widelands. Commit message: On newer libmesa versions, context creation fails if we use SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, SDL_GL_CONTEXT_PROFILE_COMPATIBILITY). Disabling this *should* still

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1825925-mac-dylib-fix into lp:widelands/build20

It's kinda fun that we both solved the same problem at the same time, isn't it? :) -- https://code.launchpad.net/~widelands-dev/widelands/bug-1825925-mac-dylib-fix/+merge/366418 Your team Widelands Developers is subscribed to branch lp:widelands/build20.

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1825925-mac-dylib-fix into lp:widelands/build20

I do feel that this might do the wrong thing for the executable itself (i.e. the widelands binary), since it also changes the id of it. Not sure what behavior this has. The code is also still containing the bug that the id line is parsed as if it where a dependency. Since I was not sure if

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/fix_fix_dependencies into lp:widelands

SirVer has proposed merging lp:~widelands-dev/widelands/fix_fix_dependencies into lp:widelands. Commit message: Fix the fix_dependency.py script. Requested reviews: Widelands Developers (widelands-dev) Related bugs: Bug #1825925 in widelands: "Mac OS builds for >= 10.9 have no sound

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1817607-macos-build into lp:widelands

Review: Approve Still works on my old Mac to build the nightlies. I did not yet achieve to build a newer version on my new mac, but I believe this to be an issue with my build environment. This is good to go, imho. --

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands-website/gdpr_1 into lp:widelands-website

> I am thinking about giving SirVer some memorial, or other thing which notice > him as the generous donor of the online-server. A way to say 'Thank you!' and > also a hint for other users on widelands.org. Please do not. Giving money is a small contribution compared to giving time and

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/fix_osx_nightlies into lp:widelands

Review: Approve @stonerl: There is no reason to apologize - I am super excited that you picked up, undusted and improved the Mac OS X build experience and untested things break with change. I am sorry to partially undo your modernizations efforts, but I do not have time to do more major

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/fix_osx_nightlies into lp:widelands

SirVer has proposed merging lp:~widelands-dev/widelands/fix_osx_nightlies into lp:widelands. Commit message: Unbreak nightly builds by un-modernizing some of the recent changes to the OS X toolchain. Requested reviews: Widelands Developers (widelands-dev) Related bugs: Bug #1788031

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/update_eris_to_1_1_2 into lp:widelands

Drive by: This will break safegame compatibility, I assume. -- https://code.launchpad.net/~widelands-dev/widelands/update_eris_to_1_1_2/+merge/349095 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/update_eris_to_1_1_2 into lp:widelands.

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands-website/django1_11 into lp:widelands-website

Review: Approve Thanks for this massive update and keeping our homepage alive and kicking! Drive by comment, only refering to the questions you've asked. I did not review the rest of the change - it seems massive. > Providing usernames for JS when writing PMs: This is maybe a security risk >

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

I hate exceptions. For everything. They add a second control flow - but it is invisible. Your functions can return a value - or they might throw an exception. The first thing is visible in its signature, the second is not. At the call site you cannot tell which exceptions might come your way.

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/irc_users into lp:widelands

I did the merge of master. @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/irc_users/+merge/335898 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/irc_users. ___ Mailing list:

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands-website/better_notification_mail into lp:widelands-website

Review: Approve lgtm. not tested though. -- https://code.launchpad.net/~widelands-dev/widelands-website/better_notification_mail/+merge/335028 Your team Widelands Developers is subscribed to branch lp:widelands-website. ___ Mailing list:

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

you are right, we should probably codify our style somewhere - since we are only using parts of the google c++ style. we have a long history of boost usage in Widelands, so we continue using it. We also have used exceptions a lot in the past, so we also continue using them. Were we a

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/net-relay into lp:widelands

Review: Approve @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/net-relay/+merge/332386 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/nethost-split. ___ Mailing list:

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/net-relay into lp:widelands

Actually, that was easy. This code lgtm. Will review the metaserver soon. -- https://code.launchpad.net/~widelands-dev/widelands/net-relay/+merge/332386 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/nethost-split.

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/net-relay into lp:widelands

> For the clients this mapping does not exist since they don't have to care > about other clients; the game state is always send by the game host. Okay, now I got it. using a string as identifier for all packets is not wise, since packets can be small and would grow in size tremendously

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/net-relay into lp:widelands

I am on business travel and therefore slow to respond. > It is returning the player IDs as seen by the relay, which are something > different than the player IDs in the GameHost/Client code. I think these IDs should stay private to Widelands and the metaserver/relay respectively. Instead when

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/net-relay into lp:widelands

4) I disagree. I think it looks wonderful! You only need std::move() to signalize that you move the front() of your queue and leave an empty vector - and without it, the code would read like a copy at this site. So the move adds readability. All in all the code now looks like I would expect it

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/keep_optr_in_ui into lp:widelands

The proposal to merge lp:~widelands-dev/widelands/keep_optr_in_ui into lp:widelands has been updated. Commit Message changed to: A more principled fix to dangling object pointers in the UI. The EditorGameBase class already tracks liveliness of MapObjects for us. So let's make use of this

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands-website/anti_spam_app into lp:widelands-website

> I use screen as a terminal multiplexer, i think your approach with tmux works > also with screen? Yes, it does. Thanks for deploying! -- https://code.launchpad.net/~widelands-dev/widelands-website/anti_spam_app/+merge/334232 Your team Widelands Developers is subscribed to branch

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands-website/anti_spam_app into lp:widelands-website

You can check the duration by using /usr/bin/time . You can avoid terminal connection drop by running tmux. Basically: $ tmux $ /usr/bin/time mysqldump Ups connection to the server dropped here at some point. ssh again, now $ tmux attach here you are, back in your old session. --

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/net-relay into lp:widelands

I added a few more NOCOMs again, sorry :(. 3) That design looks sufficient for what I had in mind. It is also nicely minimal. Good job! 4) Here is my suggestion: - Change bool try_receive(RecvPacket*) to std::unique_ptr try_receive(), with nullptr being what false was before. - The various

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/keep_optr_in_ui into lp:widelands

Can I get some testing feedback on this? It is a bit more involved change. -- https://code.launchpad.net/~widelands-dev/widelands/keep_optr_in_ui/+merge/334524 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/keep_optr_in_ui into

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/keep_optr_in_ui into lp:widelands

Diff comments: > > === modified file 'src/wui/buildingwindow.cc' > --- src/wui/buildingwindow.cc 2017-11-29 07:59:49 + > +++ src/wui/buildingwindow.cc 2017-11-30 11:02:11 + > @@ -381,8 +392,13 @@ > === > */ > void BuildingWindow::act_start_stop() { > - if

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/keep_optr_in_ui into lp:widelands

It does not fix 1735090, but it seems to fix 1619970. At least I was unable to repro this after clicking the help button a thousand times. -- https://code.launchpad.net/~widelands-dev/widelands/keep_optr_in_ui/+merge/334524 Your team Widelands Developers is requested to review the proposed merge

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/keep_optr_in_ui into lp:widelands

SirVer has proposed merging lp:~widelands-dev/widelands/keep_optr_in_ui into lp:widelands. Commit message: A more principled fix to dangling object pointers in the UI. The EditorGameBase class already tracks liveliness of MapObjects for us. So let's make use of this instead of keeping

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/replace-regex into lp:widelands

gcc <= 4.8 does not ship regex in their std library. I do not remember if it compiles and crashes at runtime or if it doesn't compile. https://stackoverflow.com/questions/12530406/is-gcc-4-8-or-earlier-buggy-about-regular-expressions --

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/add_player_to_note_economy into lp:widelands

@bunnybot merge force still mac os x woes. -- https://code.launchpad.net/~widelands-dev/widelands/add_player_to_note_economy/+merge/334371 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/add_player_to_note_economy.

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/fix_invalid_memory_access_on_dismantle into lp:widelands

SirVer has proposed merging lp:~widelands-dev/widelands/fix_invalid_memory_access_on_dismantle into lp:widelands. Commit message: Check if our window is dying before investigating a building note. Patch by Klaus Halfmann. Requested reviews: Widelands Developers (widelands-dev) Related bugs

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1734046-statistics_menu into lp:widelands

Review: Approve two nits - always prefer lambdas over bind if you can. Diff comments: > > === modified file 'src/wui/game_options_menu.cc' > --- src/wui/game_options_menu.cc 2017-02-21 07:56:18 + > +++ src/wui/game_options_menu.cc 2017-11-29 05:07:40 + > @@ -114,14 +114,17 @@

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/add_player_to_note_economy into lp:widelands

The proposal to merge lp:~widelands-dev/widelands/add_player_to_note_economy into lp:widelands has been updated. Commit Message changed to: Replace economy identifications through using pointers in the UI code. This works since these identifiers are never send over the network and economy

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/fix_asan_building into lp:widelands

SirVer has proposed merging lp:~widelands-dev/widelands/fix_asan_building into lp:widelands. Commit message: Truly only include ASAN by default in debug builds. Related to https://bugs.launchpad.net/widelands/+bug/1734843, but does not address the fundamental problem of compile.sh. Requested

Re: [Widelands-dev] [Merge] lp:~hjd/widelands/osx-update into lp:widelands

When I run `pip install python` on my system, it prints out this caveat: --- This formula installs a python2 executable to /usr/local/bin. If you wish to have this formula's python executable in your PATH then add the following to ~/.zshrc: export PATH="/usr/local/opt/python/libexec/bin:$PATH"

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/net-relay into lp:widelands

I numbered the points for easier reference: > 1) I changed the code to my suggestion. Please revert if you disagree. > 2) It would be nice to get syncstream data into this somehow to make it > easier to debug desyncs going forward. Sorry, that was initial excitement while reviewing, it

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands-website/anti_spam_app into lp:widelands-website

Review: Approve lgtm! -- https://code.launchpad.net/~widelands-dev/widelands-website/anti_spam_app/+merge/334232 Your team Widelands Developers is subscribed to branch lp:widelands-website. ___ Mailing list: https://launchpad.net/~widelands-dev Post

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/Mission4 into lp:widelands

did not properly review, just one thing inlined. Diff comments: > === modified file 'data/campaigns/campaigns.conf' > --- data/campaigns/campaigns.conf 2017-05-07 17:31:18 + > +++ data/campaigns/campaigns.conf 2017-11-25 23:51:00 + > @@ -90,6 +90,11 @@ >

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/fix_asan_crash_nethost into lp:widelands

SirVer has proposed merging lp:~widelands-dev/widelands/fix_asan_crash_nethost into lp:widelands. Commit message: Renamed some variables and fixed an ASAN reported crash. The bug was that in MultiPlayerPlayerGroup, settings_->settings().players[id] was used. This vector always has the len

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/fix_wrong_integer into lp:widelands

SirVer has proposed merging lp:~widelands-dev/widelands/fix_wrong_integer into lp:widelands. Commit message: Fix a crash in ui.lua In some cases, we asked this function to scroll to a fraction of a pixel which it does not support. Round down to avoid this case. Found with my UI fuzzer

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/net-uuid into lp:widelands

@bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/net-uuid/+merge/332264 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/net-uuid. ___ Mailing list: https://launchpad.net/~widelands-dev Post to

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/net-uuid into lp:widelands

> Seems like I am quite talented with breaking bunnybot though, sorry. Hardly your fault :/. The whole setup with launchpad -> bzr -> git -> github is kinda fragile by design. > So the problem was merging with the same branch? Yes, that was fine for bzr. But in the git import it deleted all

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/net-uuid into lp:widelands

r8472 has messed up the export to GitHub and the git repository state in the related git branch. Travis will not run again for this branch and merging this into trunk will likely mess up the trunk states in git as well. I will do some surgery on this branch and only retain the diff, throwing

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/net-uuid into lp:widelands

The proposal to merge lp:~widelands-dev/widelands/net-uuid into lp:widelands has been updated. Commit Message changed to: Improved the relogin code by adding semi-constant - 12 hour valid - user ids. Also improved doubled logins with registered accounts. For more details, see:

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/net-uuid into lp:widelands

Review: Approve Looks great! Thanks again. @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/net-uuid/+merge/332264 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/net-uuid. ___ Mailing list:

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1734088-asan-fleet-active into lp:widelands

Great find! Thanks, Gun! I highly appreciate that you put your focus on improving stability of Widelands. It seems that we suffered there a bit since b19 - that shows in the tournament where most players prefer to play b19 because it is (perceived) to be more stable. --

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/net-uuid into lp:widelands

> The option to use a temporary UUID has been removed and the interval for the > "permanent" ones reduced to 12 hours. So I guess this branch is ready for > merge now? Will have another quick read through in a minute, but yes, I think so. I will merge this and also merge the metaserver branch

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/net-uuid into lp:widelands

2) 12h sounds good to me. We could go more fancy and rewrite the last_started_timestamp every few real world seconds, but just doing 12h is fine IMHO. 3) Sorry, that was unclear from me. I suggest renaming the function. I did this real quick while thinking how it should be named and

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/net-uuid into lp:widelands

1) SHA1 is sufficient for our crypto needs for sure. And using a boost function is preferable for now too. We deal with it going away once it happens. Thanks! 2) I was thinking about this a bit and I suggest removing the option and always use the "permanent", i.e. 24h valid UUID. We could

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/net-relay into lp:widelands

indeed, done now. -- https://code.launchpad.net/~widelands-dev/widelands/net-relay/+merge/332386 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/net-relay into lp:widelands. ___ Mailing list:

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/net-relay into lp:widelands

Finally reviewed this! I left a few comments in the code using NOCOM as markers. Really nice work. The structure is understandable and the feature is an absolute killer! Thanks for working on this. -- https://code.launchpad.net/~widelands-dev/widelands/net-relay/+merge/332386 Your team

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/dynamic_tribe_loading into lp:widelands

I cannot look into this right now, but here is what I usually do in such scenarios: I try to pull out independent changes from a branch into always smaller patches and check them in independently. Eventually the problem surfaces more clearly. --

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/clang-tidy-round1 into lp:widelands

Please leave a short comment when you merge force to explain why it was necessary and why you are confident that the branch is green. > Am 12.11.2017 um 08:23 schrieb GunChleoc : > > @bunnybot merge force > -- >

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/clang-tidy-round1 into lp:widelands

Review: Approve One important comment inline - please address. Otherwise lgtm. Diff comments: > > === modified file 'src/logic/editor_game_base.cc' > --- src/logic/editor_game_base.cc 2017-09-02 19:34:45 + > +++ src/logic/editor_game_base.cc 2017-11-10 19:17:16 + > @@ -73,7

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/dynamic_tribe_loading into lp:widelands

Hooray for testing! -- https://code.launchpad.net/~widelands-dev/widelands/dynamic_tribe_loading/+merge/329198 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/dynamic_tribe_loading. ___ Mailing list:

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/dynamic_tribe_loading into lp:widelands

Review: Approve Reviewed and tested. The new scenario is a lot of fun! @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/dynamic_tribe_loading/+merge/329198 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/dynamic_tribe_loading.

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/dynamic_tribe_loading into lp:widelands

Diff comments: > === modified file 'data/tribes/init.lua' > --- data/tribes/init.lua 2017-02-12 09:10:57 + > +++ data/tribes/init.lua 2017-09-03 11:19:20 + > @@ -11,12 +11,33 @@ > -- > -- Basic load order (first wares, then immovables etc.) is important, > -- because checks

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/net-relay into lp:widelands

This merge request tripped up bunnybot because the number of comments went down when it was reopened. The bug is fixed now, so other branches should now get merged as normal again. -- https://code.launchpad.net/~widelands-dev/widelands/net-relay/+merge/332386 Your team Widelands Developers is

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/fix_work_area_preview into lp:widelands

@bunnybot merge force -- https://code.launchpad.net/~widelands-dev/widelands/fix_work_area_preview/+merge/332688 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/fix_work_area_preview. ___ Mailing list:

Re: [Widelands-dev] [Merge] lp:~hjd/widelands/macos-brew-update-workaround into lp:widelands

Awesome! Thanks for the quick turnaround on the fix. @bunnybot merge > Am 25.10.2017 um 02:01 schrieb bunnybot : > > Continuous integration builds have changed state: > > Travis build 2714. State: passed. Details: >

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands

Diff comments: > > === modified file 'src/ui_fsmenu/options.cc' > --- src/ui_fsmenu/options.cc 2017-09-11 16:59:41 + > +++ src/ui_fsmenu/options.cc 2017-10-24 19:19:16 + > @@ -432,7 +421,8 @@ > language_dropdown_.add(_("Try system language"), "", nullptr, > current_locale ==

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/fix_work_area_preview into lp:widelands

Travis fails on Mac OS X, unrelated to this branch. I filed https://bugs.launchpad.net/widelands/+bug/1727021. Otherwise this branch passes CI. -- https://code.launchpad.net/~widelands-dev/widelands/fix_work_area_preview/+merge/332688 Your team Widelands Developers is subscribed to branch

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands

more comments inline. Diff comments: > > === modified file 'src/ui_fsmenu/options.cc' > --- src/ui_fsmenu/options.cc 2017-09-11 16:59:41 + > +++ src/ui_fsmenu/options.cc 2017-10-14 16:12:18 + > @@ -432,7 +421,7 @@ > language_dropdown_.add(_("Try system language"), "", nullptr,

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/fix_work_area_preview into lp:widelands

SirVer has proposed merging lp:~widelands-dev/widelands/fix_work_area_preview into lp:widelands. Commit message: Render some overlays even if the field is not seen by the player. Requested reviews: Widelands Developers (widelands-dev) Related bugs: Bug #1726269 in widelands: "work

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands

Review: Approve code lgtm, a few nits. not tested. Diff comments: > > === modified file 'src/ui_fsmenu/options.cc' > --- src/ui_fsmenu/options.cc 2017-09-11 16:59:41 + > +++ src/ui_fsmenu/options.cc 2017-10-14 16:12:18 + > @@ -432,7 +421,7 @@ > language_dropdown_.add(_("Try

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1687100-reveal_fields into lp:widelands

> I don't have a problem with removing it. please do and ping me for another round of reviews here. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1687100-reveal_fields/+merge/323721 Your team Widelands Developers is subscribed to branch

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands-website/settings_unicode into lp:widelands-website

Lgtm. Let’s ship it! > Am 17.10.2017 um 22:04 schrieb kaputtnik : > > kaputtnik has proposed merging > lp:~widelands-dev/widelands-website/settings_unicode into > lp:widelands-website. > > Requested reviews: > Widelands Developers (widelands-dev) > > For more details, see: >

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1687100-reveal_fields into lp:widelands

I starting looking through the code and there is a lot to digest here. I have a few questions: - Why do we require the player command? It seems that we only want to mess with the normal rules of vision through scripting which has to run in parallel on each machine anyways, i.e. should not

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/reveal_hide_animations into lp:widelands

What is the status of this branch? Is it ready for reviewing? -- https://code.launchpad.net/~widelands-dev/widelands/reveal_hide_animations/+merge/327062 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1687100-reveal_fields.

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands-website/searching_with_haystack_whoosh into lp:widelands-website

> If you agree to exchange django-sphinx with django-haystack and whoosh i'll > set up the alpha site for testing. my 2c: this is defintively a future looking change that I fully support. I am sorry that I am currently a bit vanished. My paternity leave is over and I am back at work full

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/market1 into lp:widelands

*new CmdProposeTrade(get_gametime(), > object->get_owner()->player_number(), trade)); > +} > + > +int Game::propose_trade(const Trade& trade) { > + // TODO(sirver,trading): Check if a trade is possible (i.e. if there is > a > + // p

Re: [Widelands-dev] [Merge] lp:~trimardio/widelands-website/scheduling_module into lp:widelands-website

argl.. somehow firefox or launchpad ate my inline comments -- https://code.launchpad.net/~trimardio/widelands-website/scheduling_module/+merge/331477 Your team Widelands Developers is subscribed to branch lp:widelands-website. ___ Mailing list:

Re: [Widelands-dev] [Merge] lp:~trimardio/widelands-website/scheduling_module into lp:widelands-website

No, now they are there. I am confused -- https://code.launchpad.net/~trimardio/widelands-website/scheduling_module/+merge/331477 Your team Widelands Developers is subscribed to branch lp:widelands-website. ___ Mailing list:

Re: [Widelands-dev] [Merge] lp:~trimardio/widelands-website/scheduling_module into lp:widelands-website

Review: Needs Fixing Did not test and could not finish the code review yet - my daughter woke up. I'll try to do it till the end of the weekend. Diff comments: > === added file 'media/css/kalendae.css' > --- media/css/kalendae.css1970-01-01 00:00:00 + > +++ media/css/kalendae.css

Re: [Widelands-dev] [Merge] lp:~trimardio/widelands-website/scheduling into lp:widelands-website

Can you propose that other branch for merging once you are ready? Only then can we know if the diff is correct or if bzr is still confused. -- https://code.launchpad.net/~trimardio/widelands-website/scheduling/+merge/330997 Your team Widelands Developers is requested to review the proposed merge

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands-website/update_jvascript into lp:widelands-website

Review: Approve testing -- https://code.launchpad.net/~widelands-dev/widelands-website/update_jvascript/+merge/331244 Your team Widelands Developers is subscribed to branch lp:widelands-website. ___ Mailing list: https://launchpad.net/~widelands-dev

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands-website/update_jvascript into lp:widelands-website

This is not reviable - too many changed lines. I clicked around the alphe page and tested all the things you asked for. No issues. I suggest rolling this out and looking what breaks. -- https://code.launchpad.net/~widelands-dev/widelands-website/update_jvascript/+merge/331244 Your team

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/market1 into lp:widelands

I am unsure as of now how, but this breaks the casern and something in seafaring. I have to investigate how I broke this. -- https://code.launchpad.net/~widelands-dev/widelands/market1/+merge/331233 Your team Widelands Developers is requested to review the proposed merge of

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/market1 into lp:widelands

Still very big, but the smallest functional piece I could make. Forward looking I'll probably have essentially one branch per TODO which should make the diffs even smaller. -- https://code.launchpad.net/~widelands-dev/widelands/market1/+merge/331233 Your team Widelands Developers is requested

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/market1 into lp:widelands

SirVer has proposed merging lp:~widelands-dev/widelands/market1 into lp:widelands. Commit message: First working land-based trading implementation. A trade works like this: A player proposes to another players market to initiate a trade. A trade consists of the three pieces of information

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/gcc_optimize into lp:widelands

Review: Approve > Can anyone reproduce this on another OS? I do not have access to gcc 6.2, but -O2 vs -O3 should not make a big difference for Widelands. Thanks for fixing this! @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/gcc_optimize/+merge/331049 Your team

Re: [Widelands-dev] [Merge] lp:~trimardio/widelands-website/scheduling into lp:widelands-website

The diff is pretty messed up (for example local_settings.py.sample is reported to be removed and added). Could you refork from trunk and make a new branch with a clean history? -- https://code.launchpad.net/~trimardio/widelands-website/scheduling/+merge/330997 Your team Widelands Developers is

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/market into lp:widelands

This was a bunnybot bug, fixed in https://github.com/widelands/bunnybot/commit/a12a09ac28b71ff62206627ff693d78fc94cf1c4. @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/market/+merge/330830 Your team Widelands Developers is subscribed to branch

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/market into lp:widelands

removed non-breaking space from commit message. @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/market/+merge/330830 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/market. ___ Mailing list:

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/market into lp:widelands

The proposal to merge lp:~widelands-dev/widelands/market into lp:widelands has been updated. Commit Message changed to: Adds a new building type called 'Market' that will be the trading building for land-based trading. The building is only added to the barbarians and is is disabled in all

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/market into lp:widelands

Fixed the nit that Gun asked for. Thanks for testing and code review! @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/market/+merge/330830 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/market.

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/fix_resolution_change_mouse_bug into lp:widelands

Thanks. There is not much code to review, so I'm gonna merge. @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/fix_resolution_change_mouse_bug/+merge/330888 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/fix_resolution_change_mouse_bug.

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/fix_resolution_change_mouse_bug into lp:widelands

SirVer has proposed merging lp:~widelands-dev/widelands/fix_resolution_change_mouse_bug into lp:widelands. Commit message: Resize MapView if resolution changes. Requested reviews: Widelands Developers (widelands-dev) Related bugs: Bug #1717696 in widelands: "Mouse click-selector can't

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/market into lp:widelands

Review: Resubmit > I just started a multiplayer game with varying starting conditions on > collectors and got 4 coroutine errors: Should all be fixed now, please take another look. -- https://code.launchpad.net/~widelands-dev/widelands/market/+merge/330830 Your team Widelands Developers is

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/market into lp:widelands

The proposal to merge lp:~widelands-dev/widelands/market into lp:widelands has been updated. Commit Message changed to: Adds a new building type called 'Market' that will be the trading building for land-based trading. The building is only added to the barbarians and is is disabled in all

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/market into lp:widelands

SirVer has proposed merging lp:~widelands-dev/widelands/market into lp:widelands. Commit message: Adds a new building type called 'Market' that will be the trading building for land-based trading. This only adds the (massive) boilerplate and a simple test that the new building can actually

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/kill_overlay_manager into lp:widelands

This is the last refactoring branch in the pipeline for now. This is a refactoring that decreases coupling and makes the clearer in many places. It also makes overlay bugs less likely since overlays no longer need to be registered, they are just drawn as the current state of the playing field

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/reduce_overlay_manager_use3 into lp:widelands

Thanks! @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/reduce_overlay_manager_use3/+merge/330496 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/reduce_overlay_manager_use3. ___ Mailing list:

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/fix_ui_bugs into lp:widelands

Will fix the warnings in https://code.launchpad.net/~widelands-dev/widelands/reduce_overlay_manager_use3/+merge/330496. -- https://code.launchpad.net/~widelands-dev/widelands/fix_ui_bugs/+merge/330176 Your team Widelands Developers is subscribed to branch

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/reduce_overlay_manager_use3 into lp:widelands

> Is there a reason you can't remove enum class OverlayLevel {} right now? If the answer is "yes", ignore this comment, otherwise remove. No, it could be removed. but since it is part of the public API of the class it would entail further changes. Since the whole class is deleted in the next

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/reduce_overlay_manager_use3 into lp:widelands

SirVer has proposed merging lp:~widelands-dev/widelands/reduce_overlay_manager_use3 into lp:widelands. Commit message: Draw build help slope indicators in the draw routine directly, not using the FieldOverlayManager. Requested reviews: Widelands Developers (widelands-dev) For more details

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/reduce_overlay_manager_use3 into lp:widelands

The proposal to merge lp:~widelands-dev/widelands/reduce_overlay_manager_use3 into lp:widelands has been updated. Commit Message changed to: Draw build help slope indicators in the draw routine directly, not using the FieldOverlayManager. For more details, see:

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/reduce_overlay_manager_use3 into lp:widelands

For testing: this changes the way the help icons for road buildings are displayed. I mean the red, yellow and green indicators while building a road that shows the steepness of the road. -- https://code.launchpad.net/~widelands-dev/widelands/reduce_overlay_manager_use3/+merge/330496 Your team

  1   2   3   4   5   6   7   8   9   10   >