Page MenuHomeWildfire Games

Make Promotion.js use the common Transform helper
Needs ReviewPublic

Authored by wraitii on Sat, Jun 29, 5:32 PM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Trac Tickets
#4334
Summary

I don't see a valid reason that Promotion should be an exception, but I could have missed it.

Test Plan

I anybody knows of said good reason, please comment. Otherwise this is relatively straightforward.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
temp
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 8113
Build 13207: Vulcan BuildJenkins
Build 13205: arc lint + arc unit

Unit TestsFailed

TimeTest
0 msJenkins > cxxtest_debug.xml::[failed-to-read]
Failed to read test report file /mnt/data/jenkins-phabricator/workspace/differential/cxxtest_debug.xml org.dom4j.DocumentException: Error on line 1 of document : Content is not allowed in prolog. Nested exception: Content is not allowed in prolog. at org.dom4j.io.SAXReader.read(SAXReader.java:482)
0 msJenkins > TestAllocators::test_da
0 msJenkins > TestAtlasObjectXML::test_parse_attributes1
0 msJenkins > TestAtlasObjectXML::test_parse_attributes2
0 msJenkins > TestAtlasObjectXML::test_parse_basic
View Full Test Results (1 Failed · 322 Passed)

Event Timeline

wraitii created this revision.Sat, Jun 29, 5:32 PM

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

Link to build: https://jenkins.wildfiregames.com/job/differential/1843/display/redirect

elexis updated the Trac tickets for this revision.Sat, Jun 29, 5:34 PM
bb added a subscriber: bb.Mon, Jul 1, 12:49 PM

Transform.js got added in rP18467 to enable upgrading, as transform.js is meant to be generic and should be able to perform any change of template, this change makes should make things more clean and easier to handle.

Notice that we now also carry over the capturable data, which is a good thing (it didn't harm anything up to now since there is no unit that can promote and be captured)

binaries/data/mods/public/simulation/components/Promotion.js
65–67

that seems to be missing in tranform.js

69–75

same here

87

we still need that call

Stan added a subscriber: Stan.Mon, Jul 1, 1:24 PM

Would be nice to have a test maybe ?

Thanks for giving this a look, indeed we should validate that all checks done here are also done in Transform, and perhaps check which checks done in Transform aren't done here.
The experience value might not need to be passed here depending on how IncreaseXP does things, so we'll need to be careful.

Adding tests for Promotion sounds good. We should also add static checking for serialisation.