Page MenuHomeWildfire Games

-ConquestCritical consistency
ClosedPublic

Authored by Nescio on Sep 8 2017, 7:01 PM.

Details

Summary

Changes:

  • template_structure_economic.xml is no longer conquest critical, instead of individually at farmstead, market, storehouse (cf. template_structure_resource.xml)
  • healers are no longer conquest critical
  • fishing boats are no longer conquest critical
  • merchant ships are no longer conquest critical

For comparison: (land) traders are currently not conquest critical either (unchanged)

Besides, it's quite annoying to have to hunt for a non-combatant, which can neither build nor attack, abandoned somewhere.

Test Plan

Tiny patch, little to test

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Nescio created this revision.Sep 8 2017, 7:01 PM
Owners added a subscriber: Restricted Owners Package.Sep 8 2017, 7:01 PM
elexis added a subscriber: elexis.Sep 8 2017, 7:49 PM
elexis added inline comments.
binaries/data/mods/public/simulation/templates/template_unit_mechanical_ship.xml
33 ↗(On Diff #3585)

Dunno if others want it, I'd advise against it. Right now its designed to only work for traders. If you dont want to find the last units, play with Conquest Structures is what we usually say.

binaries/data/mods/public/simulation/templates/template_unit_mechanical_ship_merchant.xml
27 ↗(On Diff #3585)

See #3229
The idea was that players can still do economy for their allies if they still have economical units and the according dropsites.
But ok for me as it's not historically realistic to have a civ that only survived on ships without a single worker.

Vulcan added a subscriber: Vulcan.Sep 8 2017, 10:33 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jenkins-master:8080/job/phabricator/1999/ for more details.

bb added a subscriber: bb.Sep 8 2017, 11:59 PM
bb added inline comments.
binaries/data/mods/public/simulation/templates/template_unit_mechanical_ship_merchant.xml
27 ↗(On Diff #3585)

disagree on this change since on extinct volcano one is not that unlikely to end up with only merchant ships and still having just ally docks to trade and thus helping the ally.

Nescio updated this revision to Diff 3604.Sep 9 2017, 12:16 PM
Nescio retitled this revision from Ships are bribable; fishing boats and merchant ships are no longer conquest critical to fishing boats, merchant ships, and healers are no longer conquest critical.
Nescio edited the summary of this revision. (Show Details)

Updated to take into account earlier comment by elexis.

Vulcan added a comment.Sep 9 2017, 1:03 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jenkins-master:8080/job/phabricator/2003/ for more details.

Nescio added inline comments.Sep 9 2017, 1:34 PM
binaries/data/mods/public/simulation/templates/template_unit_mechanical_ship_merchant.xml
27 ↗(On Diff #3585)

For comparison, land traders and markets are currently not conquest critical either (unchanged), so why should merchant ships be different?

Nescio added a comment.Sep 9 2017, 2:04 PM

Something else: it seems merchant ship currently benefit from (docks) ship armour technologies and simultaneously from (market) trader convoy armour and speed researches; is this as intended? (If not, I could easily fix it.)

Nescio updated this revision to Diff 3624.Sep 10 2017, 5:05 PM
Nescio retitled this revision from fishing boats, merchant ships, and healers are no longer conquest critical to -ConquestCritical consistency.
Nescio edited the summary of this revision. (Show Details)

minor change

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jenkins-master:8080/job/phabricator/2011/ for more details.

mimo accepted this revision.Sep 20 2017, 6:54 PM
mimo added a subscriber: mimo.

Patch looks good to me. I've no strong opinions about merchant ships, but i agree it should be consistent with land traders. So ok for the patch.

This revision is now accepted and ready to land.Sep 20 2017, 6:54 PM
This revision was automatically updated to reflect the committed changes.