widelands

Merge lp://qastaging/~widelands-dev/widelands/constructionsite_options into lp://qastaging/widelands

  1. constructionsite_options
  2. Merge into trunk
Proposed by Benedikt Straub
Status: Merged
Merged at revision: 9161
Proposed branch: lp://qastaging/~widelands-dev/widelands/constructionsite_options
Merge into: lp://qastaging/widelands
Diff against target: 550 lines (+126/-69)
9 files modified
src/logic/game.cc (+6/-4)
src/logic/game.h (+4/-2)
src/logic/map_objects/tribes/building_settings.cc (+1/-11)
src/logic/map_objects/tribes/building_settings.h (+29/-0)
src/logic/player.cc (+7/-3)
src/logic/playercommand.cc (+50/-34)
src/logic/playercommand.h (+6/-2)
src/wui/constructionsitewindow.cc (+7/-7)
src/wui/inputqueuedisplay.cc (+16/-6)
To merge this branch: bzr merge lp://qastaging/~widelands-dev/widelands/constructionsite_options
Reviewer Review Type Date Requested Status
Benedikt Straub Needs Resubmitting
GunChleoc Approve
Review via email: mp+369210@code.qastaging.launchpad.net

Commit message

Fixes for some errors in constructionsite settings

Description of the change

As happens so often, new bugs are found directly after the feature that causes them has been merged :)

– Fix two errors with ware priorities and max fills (we need to distinguish between the constructionsite´s real input queues and those for the settings)
– Minimum allowed capacity for milsites is 1, not 0
– Fix initial priority view in inputqueuedisplay
– Fix a bug in saveloading trainingsite-constructionsite-settings

To post a comment you must log in.
Revision history for this message
GunChleoc (gunchleoc) wrote :

1 small nit.

Revision history for this message
Benedikt Straub (nordfriese) wrote :

Replied to the diff comment.
Also found and fixed another flaw in the inputqueuedisplay.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Yep, that makes sense. Code LGTM, not tested yet.

review: Approve
Revision history for this message
Benedikt Straub (nordfriese) wrote :

Just found another issue in trainingsite-constructionsite saveloading which I´d like to fix with this branch

Revision history for this message
GunChleoc (gunchleoc) wrote :

Changes look good :)

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 5231. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/549597607.
Appveyor build 5010. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_constructionsite_options-5010.

Revision history for this message
GunChleoc (gunchleoc) wrote :

I did some testing, looks good.

I noticed that the heroes/rookies buttons order is reversed in comparison to the finished military buildings. Do you want to swap them before we merge this branch?

Revision history for this message
Benedikt Straub (nordfriese) wrote :

Repositioned the buttons.
I also pushed a revision that should fix the memory leak you found (not tested).

review: Needs Resubmitting
Revision history for this message
GunChleoc (gunchleoc) wrote :

The hero/rookie buttons now oscillate their state, so that needs fixing.

The memory leak seems to be gone.

Revision history for this message
Benedikt Straub (nordfriese) wrote :

Should be fixed now. I´m unable to compile and test at the moment though.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Yes, that fixed it :)

@bunnybot merge

Revision history for this message
bunnybot (widelandsofficial) wrote :

Refusing to merge, since Travis is not green. Use @bunnybot merge force for merging anyways.

Travis build 5246. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/550167492.

Revision history for this message
Benedikt Straub (nordfriese) wrote :

@bunnybot merge

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 5250. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/551743880.
Appveyor build 5029. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_constructionsite_options-5029.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to status/vote changes: