Merge lp://qastaging/~widelands-dev/widelands/reed-compatibility into lp://qastaging/widelands

Continuous integration builds have changed state:

Travis build 5058. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/537337141.
Appveyor build 4838. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_reed_compatibility-4838.

So ... you created a diff of 800 lines by passing objects around so you can call a method in only a handful of places?? I like it! :)
The changes are mostly looking good to me. Replacing the GOTO might be broken, though, see my diff comment below.

I haven't tested the changes, mostly because I have no sensible idea how to do so. But based on the code I guess it shouldn't break anything.

> So ... you created a diff of 800 lines by passing objects around so you can call a method in only a handful of places?? I like it! :)

Lol yep. It will save a bit of memory though if the objects are created only once.

For triggering the bug, you need a savegame created with bzr9118 or older that has reed or wheat fields on it.

Continuous integration builds have changed state:

Travis build 5154. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/541619525.
Appveyor build 4936. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_reed_compatibility-4936.

I have added a second if statement for the test variable to simulate the effect of the goto and break out of the outer loop. Thanks for the review!

@bunybot merge

Sorry, I am having second thoughts about the goto-part. Actually, I am not only unsure about the goto-replacement but over the whole code part independent of your changes. :-/
If I understand it right, the code iterates over the possible jobs in the production site. If there is a job the loaded worker can work on, we iterate over all working positions of the site and check if *any* of these is empty, so we can assign the worker. Shouldn't we only check the working positions that are for the required job? Otherwise, the code could break with production sites which have multiple types of worker positions (Mines probably? These need a miner and a master miner).

Maybe I am just confused and the code is fine, but it would be good if someone could check it.

This code is pretty old, so I'm sure we would have had bug reports for it in the past if it was broken. It should be fine if iterating starts with the most advanced working position, because then we won't put a master miner i a miner's slot and end up stumped when we have to put a miner in a master miner's slot, which is not allowed.

I'll reinstate the goto to make sure we can't possibly have any accidental semantic changes, so that we can get this bugfix in.

Done. Thanks for the review!

@bunnybot merge

Inputqueues again

@bunnybot merge force

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

Travis build 5164. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/542121335.

@bunnybot merge force