Page MenuHomeWildfire Games

deprecate template_unit_cavalry_melee.xml and *_ranged.xml
Needs ReviewPublic

Authored by Nescio on Aug 16 2020, 10:04 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary
template_unit_cavalry_melee.xml
template_unit_cavalry_ranged.xml

do little more than add a health value and a visible class and are thus not particularly useful. This patch removes them. Advantages are:

  • meaningful values are defined in a single template, rather than arbitrarily scattered over two levels;
  • citizen cavalry templates are more parallel to champion and hero cavalry templates, which don't have *melee* or *ranged* either;
  • shorter template file names.

Similar patches have been committed in the past:

  • D1734/rP22061 removed template_entity_full.xml and template_entity_quasi.xml.
  • D1760/rP22204 removed template_unit_mechanical.xml.
Test Plan

Check for completeness and correctness. Effectively no values should be changed. Verify everything works as before.

Unit TestsFailed

TimeTest
0 msJenkins > cxxtest_debug.xml::[failed-to-read]
Failed to read test report file E:\Jenkins\workspace\vs2015-differential\cxxtest_debug.xml org.dom4j.DocumentException: Error on line 347 of document : Content is not allowed in trailing section. at org.dom4j.io.SAXReader.read(SAXReader.java:511)
0 msJenkins > TestAllocators::Debug Build & Tests / test_da
0 msJenkins > TestAllocators::Release Build & Tests / test_da
0 msJenkins > TestAllocators::test_da
0 msJenkins > TestAllocators::test_da
View Full Test Results (1 Failed · 1,699 Passed)

Event Timeline

Nescio created this revision.Aug 16 2020, 10:04 PM
Owners added a subscriber: Restricted Owners Package.Aug 16 2020, 10:04 PM
Nescio requested review of this revision.Aug 16 2020, 10:09 PM
Nescio added a subscriber: bb.Aug 26 2020, 5:33 PM

@bb, @Freagarach, other team members, any opinions on this?

Nescio updated this revision to Diff 13344.Aug 29 2020, 12:06 PM
  • rebased
Nescio updated this revision to Diff 14087.Nov 19 2020, 8:00 PM
Nescio removed a subscriber: bb.

Successful build - Chance fights ever on the side of the prudent.

builderr-release-macos.txt
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libengine.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libatlas.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgui.a(precompiled.o) has no symbols

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/2030/display/redirect

Nescio edited subscribers, added: Silier; removed: Restricted Owners Package, wraitii, Freagarach.Dec 2 2020, 8:24 PM

@Angen, interested in reviewing this one?

Silier added a comment.Dec 3 2020, 4:28 PM

not really, i like the hierarchy

Nescio added a comment.Dec 3 2020, 5:16 PM
template_unit_champion_cavalry_melee.xml
template_unit_champion_cavalry_ranged.xml
template_unit_hero_cavalry_melee.xml
template_unit_hero_cavalry_ranged.xml

don't exist either.

In D2959#140125, @Angen wrote:

not really, i like the hierarchy

Agreed.

template_unit_champion_cavalry_melee.xml
template_unit_champion_cavalry_ranged.xml
template_unit_hero_cavalry_melee.xml
template_unit_hero_cavalry_ranged.xml

don't exist either.

Perhaps they should.

(for what it's worth, the champion versions don't exist because they all inherit from champion, but they really should inherit from champion|ranged|cavalry now or something.