Page MenuHomeWildfire Games

Do not require a garrisonHolder to have health.
ClosedPublic

Authored by wraitii on Oct 15 2019, 8:51 AM.

Details

Summary

Refs. D1268.

Test Plan

Verify that a garrisonHolder without health can be garrisoned and a garrisonHolder with health still behaves as usual.

Event Timeline

Freagarach created this revision.Oct 15 2019, 8:51 AM

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

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/456/display/redirect

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

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/457/display/redirect

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

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

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

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

bb added a subscriber: bb.Feb 12 2020, 8:17 PM
bb added inline comments.
binaries/data/mods/public/simulation/components/GarrisonHolder.js
12–13

Is it me or does it sound weird to have this set for units without a health component?

binaries/data/mods/public/simulation/components/tests/test_GarrisonHolder.js
181–183

Don't add a disabled health component, rather test it before adding the health mock.

One could consider testing more stuff without a health component (garrisoning and ejecting and stuff)

Freagarach planned changes to this revision.Feb 13 2020, 5:59 PM

Fair points.

Freagarach updated this revision to Diff 11344.Feb 13 2020, 8:41 PM
Freagarach marked 2 inline comments as done.
  • Eject health optional.
  • Extended test.

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_GarrisonHolder.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_GarrisonHolder.js
| 135| 135| 	TS_ASSERT_EQUALS(cmpGarrisonHolder.IsFull(), false);
| 136| 136| 	TS_ASSERT_EQUALS(cmpGarrisonHolder.UnloadAll(), true);
| 137| 137| 	TS_ASSERT_UNEVAL_EQUALS(cmpGarrisonHolder.GetEntities(), []);
| 138|    |-}
|    | 138|+};
| 139| 139| 
| 140| 140| // No health component yet.
| 141| 141| testGarrisonAllowed();

binaries/data/mods/public/simulation/components/tests/test_GarrisonHolder.js
| 138| }
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
Executing section cli...

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

wraitii commandeered this revision.May 30 2020, 4:29 PM
wraitii updated this revision to Diff 12059.
wraitii added a reviewer: Freagarach.

Was about to nitpick but the nit was trivial so just did it myself.
Also rebased.

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

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

bb accepted this revision.May 31 2020, 9:40 PM
bb removed a reviewer: Restricted Owners Package.
bb added inline comments.
binaries/data/mods/public/simulation/components/GarrisonHolder.js
509

Very correct, EjectHealth could be 0

binaries/data/mods/public/simulation/components/tests/test_GarrisonHolder.js
146

The counterpart doesn't appear present

This revision is now accepted and ready to land.May 31 2020, 9:40 PM
This revision was automatically updated to reflect the committed changes.

Thanks for the review both and thanks for the commit @bb!