Page MenuHomeWildfire Games

Don't set hitpoints for completed foundations
ClosedPublic

Authored by temple on Apr 5 2018, 1:40 AM.

Details

Summary

If a foundation is affected by a ranged aura (e.g. the Seleucid hero with -20% enemy building hp as in the replay in #5113) then the completed structure won't be affected by the aura yet so its max hitpoints won't be the same as the foundation's hitpoints, which means with the current code it won't start at full health. Since entities are created with hitpoints equal to max hitpoints (except for foundations which have an initial value specified), there's no need to set the hitpoints in Foundation. History at rP7352.

Test Plan

See that the replay in #5113 works now, for example.

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

temple created this revision.Apr 5 2018, 1:40 AM
temple edited the summary of this revision. (Show Details)
temple edited the summary of this revision. (Show Details)Apr 5 2018, 1:43 AM
Vulcan added a subscriber: Vulcan.Apr 5 2018, 10:15 AM

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

Linter detected issues:
Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/components/Foundation.js
| 322| »   »   var·pos·=·cmpPosition.GetPosition2D();
|    | [NORMAL] JSHintBear:
|    | 'pos' is already defined.

binaries/data/mods/public/simulation/components/Foundation.js
| 324| »   »   var·rot·=·cmpPosition.GetRotation();
|    | [NORMAL] JSHintBear:
|    | 'rot' is already defined.

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

mimo added a subscriber: mimo.Apr 5 2018, 6:45 PM

And why not setting the relative health (as is done in Promotion or Transform)? Having foundations which are usable without full health may be useful for mods even if we don't use it in vanilla game.

temple added a comment.Apr 5 2018, 6:58 PM
In D1434#58509, @mimo wrote:

And why not setting the relative health (as is done in Promotion or Transform)? Having foundations which are usable without full health may be useful for mods even if we don't use it in vanilla game.

? Foundations are completed (turned into new buildings) when their relative health (i.e. build progress) is 1, but new buildings are initialized at max hitpoints.

mimo added a comment.Apr 5 2018, 7:11 PM
In D1434#58513, @temple wrote:
In D1434#58509, @mimo wrote:

And why not setting the relative health (as is done in Promotion or Transform)? Having foundations which are usable without full health may be useful for mods even if we don't use it in vanilla game.

? Foundations are completed (turned into new buildings) when their relative health (i.e. build progress) is 1, but new buildings are initialized at max hitpoints.

What i meant is that, if instead of setting health(structure) = health(foundation) as is done in Foundation, we do (as in Promotion or Transform) health(structure) = max(structure) * health(foundation)/max(foundation), that would fix the bug and allow an easy change for mods (they would just have to add a new Foundation propriety to tell it when to turn the foundation to structure).

temple updated this revision to Diff 6345.Apr 7 2018, 7:40 PM
elexis added a subscriber: elexis.Apr 7 2018, 7:43 PM

Perhaps one should leave a code comment, since the difference between the equations isn't trivial.

Vulcan added a comment.Apr 7 2018, 9:15 PM

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

Linter detected issues:
Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/components/Foundation.js
| 322| »   »   var·pos·=·cmpPosition.GetPosition2D();
|    | [NORMAL] JSHintBear:
|    | 'pos' is already defined.

binaries/data/mods/public/simulation/components/Foundation.js
| 324| »   »   var·rot·=·cmpPosition.GetRotation();
|    | [NORMAL] JSHintBear:
|    | 'rot' is already defined.

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

mimo accepted this revision.Apr 8 2018, 11:00 AM
mimo added inline comments.
binaries/data/mods/public/simulation/components/Foundation.js
404 ↗(On Diff #6345)

if reusing progress, you should use Math.max(progress,1)

This revision is now accepted and ready to land.Apr 8 2018, 11:00 AM
temple added inline comments.Apr 8 2018, 7:09 PM
binaries/data/mods/public/simulation/components/Foundation.js
404 ↗(On Diff #6345)

How can progress be > 1?
SetHitpoints won't let you set over max anyway.

This revision was automatically updated to reflect the committed changes.