widelands

Merge lp://staging/~widelands-dev/widelands/bug-1624216-horsepocalypse into lp://staging/widelands

  1. bug-1624216-horsepocalypse
  2. Merge into trunk
Proposed by GunChleoc
Status: Merged
Approved by: GunChleoc
Approved revision: no longer in the source branch.
Merged at revision: 8202
Proposed branch: lp://staging/~widelands-dev/widelands/bug-1624216-horsepocalypse
Merge into: lp://staging/widelands
Diff against target: 46 lines (+11/-11)
1 file modified
src/logic/map_objects/tribes/warehouse.cc (+11/-11)
To merge this branch: bzr merge lp://staging/~widelands-dev/widelands/bug-1624216-horsepocalypse
Reviewer Review Type Date Requested Status
Widelands Developers Pending
Review via email: mp+310221@code.staging.launchpad.net

Commit message

When a warehouse is destroyed, limit maximum number of fleeing units per unit type to 500.

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

Continuous integration builds have changed state:

Travis build 1581. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/174007207.
Appveyor build 1419. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1624216_horsepocalypse-1419.

Revision history for this message
Klaus Halfmann (klaus-halfmann) wrote :

Code looks good to me, no Idea if that inline comment will help.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Looks like your inline comment wasn't saved :(

Revision history for this message
xxx-deleted (janosch-peters-deactivatedaccount) wrote :

It could be coded a bit more elegantly(see my comments), but apart from that it LGTM. However, I did not test it because I dont know a straight forward way to get a warehous with 500+ workers.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Thanks for the review, the code looks much better now :)

There is a savegame attached to the bug that you can use for testing - attack the headquarters to the north.

Revision history for this message
bunnybot (widelandsofficial) wrote :

Bunnybot encountered an error while working on this merge proposal:

HTTP Error 500: Internal Server Error

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 1607. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/175655541.
Appveyor build 1445. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1624216_horsepocalypse-1445.

Revision history for this message
bunnybot (widelandsofficial) wrote :

Bunnybot encountered an error while working on this merge proposal:

('The read operation timed out',)

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 1607. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/175655541.
Appveyor build 1445. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1624216_horsepocalypse-1445.

Revision history for this message
bunnybot (widelandsofficial) wrote :

Bunnybot encountered an error while working on this merge proposal:

('The read operation timed out',)

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 1607. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/175655541.
Appveyor build 1445. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1624216_horsepocalypse-1445.

Revision history for this message
Notabilis (notabilis27) wrote :

Assuming this branch waits for a tester: Code looks good to me and testing the provided savegame with trunk and this branch it seems to work as planned.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Thanks!

@bunnybot merge

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: