widelands

Merge lp://staging/~widelands-dev/widelands/bug-1075562-initial-trainers into lp://staging/widelands

  1. bug-1075562-initial-trainers
  2. Merge into trunk
Proposed by Notabilis
Status: Merged
Merged at revision: 8468
Proposed branch: lp://staging/~widelands-dev/widelands/bug-1075562-initial-trainers
Merge into: lp://staging/widelands
Diff against target: 107 lines (+9/-0)
9 files modified
data/tribes/scripting/starting_conditions/atlanteans/fortified_village.lua (+1/-0)
data/tribes/scripting/starting_conditions/atlanteans/headquarters.lua (+1/-0)
data/tribes/scripting/starting_conditions/atlanteans/trading_outpost.lua (+1/-0)
data/tribes/scripting/starting_conditions/barbarians/fortified_village.lua (+1/-0)
data/tribes/scripting/starting_conditions/barbarians/headquarters.lua (+1/-0)
data/tribes/scripting/starting_conditions/barbarians/trading_outpost.lua (+1/-0)
data/tribes/scripting/starting_conditions/empire/fortified_village.lua (+1/-0)
data/tribes/scripting/starting_conditions/empire/headquarters.lua (+1/-0)
data/tribes/scripting/starting_conditions/empire/trading_outpost.lua (+1/-0)
To merge this branch: bzr merge lp://staging/~widelands-dev/widelands/bug-1075562-initial-trainers
Reviewer Review Type Date Requested Status
TiborB Approve
Klaus Halfmann review, compile, test Approve
Review via email: mp+332959@code.staging.launchpad.net

Commit message

Adding initial trainers to starting conditions.

Description of the change

This implements the suggestion of comment #60 in the linked bug report. It avoids the problem with getting stuck when no trainers can be created since all weapons are delivered to the barracks.

Note that it is still possible to get stuck when building more than one of each training center. In that case, the problem has to be fixed by reducing the amount of weapons in the barracks.

The amount of initial weapons has not been reduced so the game becomes a bit easier by this branch (more initial weapons for soldiers).

The Trading Outpost starting condition is not modified by this branch but also suffers from the problem. Should it be changed, too?

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

I had never played other than Headquarters starting condition, but would say that 'Trading Outpost' should have the same settings.

Revision history for this message
GunChleoc (gunchleoc) wrote :

I agree - we could even throw in a 4th trainer for the Trading Outpost, since it's the Beginner's or "help the AI" setting.

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

Code looks straight foreward,
will compile and play this now
- as training for the tournament.

Revision history for this message
kaputtnik (franku) wrote :

Klaus, since only some data in the datafolder was changed there is no need to compile this branch :) Just use the option --datadir=

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 2717. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/294033839.
Appveyor build 2530. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1075562_initial_trainers-2530.

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

Appveyor faled with
> No artifacts found matching 'Widelands-_widelands_dev_widelands_bug_1075562_initial_trainers-2530-Debug-x86.exe' path

No idea why this happens? But all 4 variants are OK

Ichecekd the code and even started every variant.
Fine foe me unloess we want the change for Trading Outpost
in here, too.

review: Approve (review, compile, test)
Revision history for this message
Notabilis (notabilis27) wrote :

I added four trainers to the Trading Outpost condition. In this condition there are quite some additional workers available anyway, so why not also a trainer more than normal.

The artifacts-error also happens with succeeding builds. The reason for the failure might be the missing libicuuc57.dll a few lines further down. I *guess* we have to update our script to link against libicuuc58.dll since version 58.2 of the library is installed earlier.

Revision history for this message
TiborB (tiborb95) wrote :

Ok for me as well.

review: Approve
Revision history for this message
kaputtnik (franku) wrote :

Why not merging?

@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: