Page MenuHomeWildfire Games

remove unnecessary lines from ship templates
ClosedPublic

Authored by Nescio on May 17 2020, 6:38 PM.

Details

Summary

This patch simplifies the template_unit_ship* files by removing unnecessary lines:

  • <Cost>, <Health/Max>, <Loot>, and <Vision> are removed from the shared parent because they're superseded in practically all its children.
  • <Footprint> is moved to the specific units/* files that don't have footprint values yet, since different units use different actors of different sizes.
  • <GarrisonHolder> is moved into the shared parent, because it was defined in most its children (except the fireship).
  • <Identity> nodes follow the order specified in simulation/components/Identity.js.

See also D2850, D2855.

Test Plan

Check for completeness and correctness. Effectively no stats should be changed.

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.May 17 2020, 6:38 PM
Owners added a subscriber: Restricted Owners Package.May 17 2020, 6:38 PM
Nescio added inline comments.May 17 2020, 6:43 PM
binaries/data/mods/public/simulation/templates/template_unit_ship.xml
14–15 ↗(On Diff #11910)

A placeholder value that's not actually used, but is necessary to replace the <Footprint/Circle> inherited from template_unit.xml.

binaries/data/mods/public/simulation/templates/template_unit_ship_fishing.xml
49 ↗(On Diff #11910)

Already present in template_unit.xml.

binaries/data/mods/public/simulation/templates/template_unit_ship_merchant.xml
10 ↗(On Diff #11910)

Yes, merchant ships are the only ships that don't cost any wood. This is left unchanged. Maybe something for a future gameplay patch.

42–47 ↗(On Diff #11910)

Unnecessary, since merchant ships can only pick up treasures, and template_unit.xml already defines some values.

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2150/display/redirect

Nescio updated this revision to Diff 11911.May 17 2020, 6:59 PM
Nescio edited the summary of this revision. (Show Details)

Sort <Identity> nodes as specified in simulation/components/Identity.js.

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2151/display/redirect

Nescio edited the summary of this revision. (Show Details)May 17 2020, 7:02 PM
Nescio edited reviewers, added: Restricted Owners Package; removed: bb.Jul 2 2020, 9:50 PM
Nescio edited subscribers, added: bb; removed: Restricted Owners Package.

Not sure whether it is bad that one can garrison a dog in a fishing ship though,,,

binaries/data/mods/public/simulation/templates/template_unit_ship.xml
14–15 ↗(On Diff #11910)

So when it is merely a placeholder, why change it?

binaries/data/mods/public/simulation/templates/template_unit_ship_fishing.xml
29 ↗(On Diff #11911)

Can garrison also Dog now.

Nescio added a comment.Aug 5 2020, 7:55 PM

Not sure whether it is bad that one can garrison a dog in a fishing ship though,,,

It probably has no consequence at all (does anyone train dogs?), but infantry is allowed to, and adding the dog in the shared ship template is at least a bit more consistent than adding it in most of its children, but not all.

binaries/data/mods/public/simulation/templates/template_unit_ship.xml
14–15 ↗(On Diff #11910)

Since there needs to be something, we might as well have sensible values with a ratio that matches the overlay texture defined below.
Could also be useful for mods.

65–72 ↗(On Diff #11911)

These lines warrant a rebase.

Nescio added inline comments.Aug 5 2020, 7:59 PM
binaries/data/mods/public/simulation/templates/template_unit_ship.xml
65–72 ↗(On Diff #11911)

Actually it still seems to work, I just tested, thus no need for a rebase.

One does need to keep in mind that balancing may be affected by the change in footprints.

Nescio added a comment.Aug 6 2020, 9:04 AM

One does need to keep in mind that balancing may be affected by the change in footprints.

If I recall correctly, none of the footprints are actually changed (except for the unused on in the ship parent), they're just moved from the generic templates to the specific unit files.
Adjusting ship footprints is something for a future patch.

Freagarach edited reviewers, added: Freagarach; removed: Restricted Owners Package.Aug 7 2020, 5:32 PM
Freagarach accepted this revision.Aug 13 2020, 7:36 PM
  • Don't pretend footprints are universal: move to child-templates is good.
  • Move shared GarrisonHolder to parent is good.
    • Dogs are able to garrison fishing ships now, which seems not strange and deemed not a (large) gameplay change.
  • Cleans up nicely.

One *could* disable the ResourceGatherer in the ship parent and enable only in the fishing ship and merchant.

binaries/data/mods/public/simulation/templates/template_unit_ship_merchant.xml
10 ↗(On Diff #11910)

Perhaps because naval maps tend to be short on wood supplies and one can generate wood with these.

This revision is now accepted and ready to land.Aug 13 2020, 7:36 PM
This revision was landed with ongoing or failed builds.Aug 15 2020, 8:06 AM
This revision was automatically updated to reflect the committed changes.
Owners added a subscriber: Restricted Owners Package.Aug 15 2020, 8:06 AM