The proposal to merge lp:~widelands-dev/widelands/toggle_immovables into
lp:widelands has been updated.
Commit Message changed to:
Make display of bobs and immovables toggable.
Hotkeys are CTRL-<1-4>.
For more details, see:
I removed a blank line - xgettext can only pick up comments that are
immediately above the gettext invocation, so a blank space might keep it from
picking up the comment.
Code still LGTM, will do a bit of testing once the compiler is finished.
--
Review: Resubmit
I have now gone through the code base and removed all empty string
initializations. I also found a few more uninitialized variables in
MapTriangleRegion.
If you agree with these changes, the branch will be ready.
--
The proposal to merge
lp:~widelands-dev/widelands/bug-986611-cppcheck-uninitialized-variables into
lp:widelands has been updated.
Commit Message changed to:
Initialize a bunch of uninitialized member variables, adding constructors where
necessary. Turned some enums into enum classes. Removed
@bunnybot merge
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1664052-expedition-shipwindow-crash/+merge/318986
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1664052-expedition-shipwindow-crash.
Thanks for the review :)
Kaputtnik has already tested this branch, so I'll merge trunk and then get this
in.
Diff comments:
>
> === modified file 'src/logic/map_objects/tribes/ship.cc'
> --- src/logic/map_objects/tribes/ship.cc 2017-07-05 19:21:57 +
> +++
Thanks for clarification :-)
--
https://code.launchpad.net/~widelands-dev/widelands/toggle_immovables/+merge/328977
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/toggle_resources.
___
Mailing list:
> Didn't test it yet, but i think the order should be the same as we have icons
> in the tools menu?
true that. done.
> If the hotkey for buildhelp is changed in editor, will it still be 'space'
> for playing games?
It is not changed. It is duplicated - space still works. But for symmetry I
OK, the branch reworked, not there are two distinct switches.
Also thanks for the wiki page, I will insert there some info...
--
https://code.launchpad.net/~widelands-dev/widelands/ai_training_switch/+merge/329008
Your team Widelands Developers is requested to review the proposed merge of
Ah, forgotton:
If the hotkey for buildhelp is changed in editor, will it still be 'space' for
playing games?
--
https://code.launchpad.net/~widelands-dev/widelands/toggle_immovables/+merge/328977
Your team Widelands Developers is subscribed to branch
Didn't test it yet, but i think the order should be the same as we have icons
in the tools menu?
That is from left to right:
- ..
- Immovables (here: 3), suggestion: 2
- Animals (here: 4), suggestion: 3
- Resources (here: 2), suggestion: 4
- ..
So there a little dependency with the visual
I changed the hotkeys to use CTRL 1 to 4, doubling CTRL 1 with SPACE (which is
a slight ward). But CTRL 1-4 for the layers feels really good and quick. What
do you think?
--
https://code.launchpad.net/~widelands-dev/widelands/toggle_immovables/+merge/328977
Your team Widelands Developers is
The proposal to merge lp:~widelands-dev/widelands/toggle_immovables into
lp:widelands has been updated.
Commit Message changed to:
Make display of bobs and immovables toggable.
Hotkeys are CTRL-<1-3>.
For more details, see:
The proposal to merge lp:~widelands-dev/widelands/toggle_resources into
lp:widelands has been updated.
Status: Needs review => Merged
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/toggle_resources/+merge/328958
--
Your team Widelands Developers is subscribed to
Most people dont investigate content of .widelands folder... I dont know if
this can be done and how complex would be
--
https://code.launchpad.net/~widelands-dev/widelands/ai_training_switch/+merge/329008
Your team Widelands Developers is requested to review the proposed merge of
Done: https://wl.widelands.org/wiki/Ai%20Training/
> This is very uncommon to have such explanatory files in home folder.
I just hate empty folders in my home directory... ;)
--
https://code.launchpad.net/~widelands-dev/widelands/ai_training_switch/+merge/329008
Your team Widelands Developers
This is very uncommon to have such explanatory files in home folder. But wai
files could contain a comment with the wiki link
Wiki is good idea I can do it, but somebody could create empty wiki page for
this topic, I am not that familiar with our wiki to do it myself...
--
I want to mention my suggestion from the forum:
> Maybe adding a text file in there (in the folder ai), describing
> what this folder is used for. Something like:
> "This folder is used when using the command line switch
> 'aitrainingmode'. If you are interested in training the ai,
> see the
Will try that one now:
https://ci.appveyor.com/api/buildjobs/qsar8xtu6u7jsjjv/artifacts/Widelands-_widelands_dev_widelands_bug_1664052_expedition_shipwindow_crash-1872-Debug-x64.exe
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1664052-expedition-shipwindow-crash/+merge/318986
Your
Review: Approve review, compile
What I get is that you moved around some code from ::init to the CTor for that
shipwindow.
See som inline comments.
I see no obvious Problems in that code, no let mey try to download it from
appvoyer ors such.
Diff comments:
>
> === modified file
First of all, wai files are not merge-able. So your results can not be combined
with training results of someone else done concurrently.
The training was explained on the forum, but shortly, you need to pick
generated wai of some good AI performer and put it into 4 source wai files. And
go on
Mhh, this looks familiar, we had a similar Issue where some Notfication was
caught by some other ship.
Ill try to have a lok tonight.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1664052-expedition-shipwindow-crash/+merge/318986
Your team Widelands Developers is subscribed to
This is good comment, what about two switches:
--ai_training
--auto_speed
--
https://code.launchpad.net/~widelands-dev/widelands/ai_training_switch/+merge/329008
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/ai_training_switch into
I just gave it a quick spin, the switch is working :)
I have just been thinking of another use case: What if players want to train
the AI while playing the game? The autospeed would be a problem then. If you
think this use case won't happen, the branch can go in after the nits have been
fixed.
Diff comments:
>
> === modified file 'src/ai/defaultai.cc'
> --- src/ai/defaultai.cc 2017-08-01 12:03:03 +
> +++ src/ai/defaultai.cc 2017-08-15 08:22:00 +
> @@ -482,10 +482,15 @@
> void DefaultAI::late_initialization() {
> player_ =
I am currently running the report, which will probably take all day. I'll then
run it again without the string init to see if it makes a difference.
I'll then revert any changes in this branch that won't fix the report.
--
Diff comments:
>
> === modified file 'src/ai/defaultai.cc'
> --- src/ai/defaultai.cc 2017-08-01 12:03:03 +
> +++ src/ai/defaultai.cc 2017-08-15 08:22:00 +
> @@ -482,10 +482,15 @@
> void DefaultAI::late_initialization() {
> player_ =
OK, let's leave this as it is then.
--
https://code.launchpad.net/~widelands-dev/widelands/toggle_resources/+merge/328958
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/buildhelp_overlay.
___
Mailing list:
I left a comment for you in the diff to one of your comments
Diff comments:
>
> === modified file 'src/ai/defaultai.cc'
> --- src/ai/defaultai.cc 2017-08-01 12:03:03 +
> +++ src/ai/defaultai.cc 2017-08-15 08:22:00 +
> @@ -482,10 +482,15 @@
> void
@bunnybot merge
--
https://code.launchpad.net/~widelands-dev/widelands/toggle_resources/+merge/328958
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/buildhelp_overlay.
___
Mailing list:
> Well, it's just the same as initializing numbers to 0 - cppcheck is
> complaining, so I'm trying to get rid of that noise in the report.
That would be wrong though - plain old datatypes (POD) are not initialized in
cpp, so numbers are indeed a random value if not properly initialized.
Well, it's just the same as initializing numbers to 0 - cppcheck is
complaining, so I'm trying to get rid of that noise in the report.
The actual error message is:
src/game_io/game_preload_packet.h:38: (style) The struct 'GamePreloadPacket'
does not have a constructor although it has private
I have added some comments to the code and gotten rid of a duplicate dynamic
cast in interactive base.
--
https://code.launchpad.net/~widelands-dev/widelands/ai_training_switch/+merge/329008
Your team Widelands Developers is requested to review the proposed merge of
Testing works. I'll leave it up to you if you want to real with the
auto-switchon in this merge request or in the follow-up branch.
--
https://code.launchpad.net/~widelands-dev/widelands/toggle_resources/+merge/328958
Your team Widelands Developers is subscribed to branch
Review: Approve
Code LTGM. Let's wait with merging until we have decided on the hotkeys.
--
https://code.launchpad.net/~widelands-dev/widelands/toggle_immovables/+merge/328977
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/toggle_resources.
Review: Approve
> OverlayLevel is part of the public API of this class and kLevelForBuildHelp
> should never be used when 'register_overlay' is called. I therefore opted to
> not make it part of the enum class to make misuse harder.
This has convinced me, plus that we really don't want to use
The proposal to merge lp:~widelands-dev/widelands/buildhelp_overlay into
lp:widelands has been updated.
Status: Needs review => Merged
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/buildhelp_overlay/+merge/328956
--
Your team Widelands Developers is subscribed
Review: Approve
Yes, this is ready :)
@bunnybot merge
--
https://code.launchpad.net/~widelands-dev/widelands/buildhelp_overlay/+merge/328956
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/buildhelp_overlay.
___
38 matches
Mail list logo
