Differential D1726

Change Max Health back into a nonNegativeInteger
AbandonedPublic
Actions

Authored by wraitii on Jan 2 2019, 4:48 PM.

Details

Reviewers

None

Group Reviewers

Restricted Owners Package

(Owns No Changed Paths)

Summary

Following rP22003, we can set Health back into an integer type like God intended.

Sim/GUI components need to continue treating it as a float since fractional health makes sense.

Test Plan

Check for completeness but I've grepped so we should be good.

Diff Detail

Repository

rP 0 A.D. Public Repository

Branch

temp

Lint

Lint OK

Unit

No Unit Test Coverage

Build Status

Buildable 7571
Build 12341: Vulcan Build	Jenkins
Build 12340: arc lint + arc unit

Event Timeline

wraitii created this revision.Jan 2 2019, 4:48 PM

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

Linter detected issues:
Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Health.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Health.js
|  64|  64| 	this.UpdateActor();
|  65|  65| };
|  66|  66| 
|  67|    |-//// Interface functions ////
|    |  67|+// // Interface functions ////
|  68|  68| 
|  69|  69| /**
|  70|  70|  * Returns the current hitpoint value.
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Health.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Health.js
|  96|  96| 
|  97|  97| 	var cmpRangeManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_RangeManager);
|  98|  98| 	if (cmpRangeManager)
|  99|    |-	{
|    |  99|+	
| 100| 100| 		if (this.hitpoints < this.GetMaxHitpoints())
| 101| 101| 			cmpRangeManager.SetEntityFlag(this.entity, "injured", true);
| 102| 102| 		else
| 103| 103| 			cmpRangeManager.SetEntityFlag(this.entity, "injured", false);
| 104|    |-	}
|    | 104|+	
| 105| 105| 
| 106| 106| 	this.RegisterHealthChanged(old);
| 107| 107| };
|    | [NORMAL] ESLintBear (operator-linebreak):
|    | '||' should be placed at the end of the line.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Health.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Health.js
| 113| 113| 
| 114| 114| Health.prototype.IsUnhealable = function()
| 115| 115| {
| 116|    |-	return (this.template.Unhealable == "true"
| 117|    |-		|| this.GetHitpoints() <= 0
|    | 116|+	return (this.template.Unhealable == "true" ||
|    | 117|+		this.GetHitpoints() <= 0
| 118| 118| 		|| this.GetHitpoints() >= this.GetMaxHitpoints());
| 119| 119| };
| 120| 120| 
|    | [NORMAL] ESLintBear (operator-linebreak):
|    | '||' should be placed at the end of the line.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Health.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Health.js
| 114| 114| Health.prototype.IsUnhealable = function()
| 115| 115| {
| 116| 116| 	return (this.template.Unhealable == "true"
| 117|    |-		|| this.GetHitpoints() <= 0
| 118|    |-		|| this.GetHitpoints() >= this.GetMaxHitpoints());
|    | 117|+		|| this.GetHitpoints() <= 0 ||
|    | 118|+		this.GetHitpoints() >= this.GetMaxHitpoints());
| 119| 119| };
| 120| 120| 
| 121| 121| Health.prototype.GetIdleRegenRate = function()
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required after '{'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Health.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Health.js
| 258| 258| 		cmpFogging.Activate();
| 259| 259| 
| 260| 260| 	if (this.hitpoints == this.GetMaxHitpoints())
| 261|    |-		return {"old": this.hitpoints, "new":this.hitpoints};
|    | 261|+		return { "old": this.hitpoints, "new":this.hitpoints};
| 262| 262| 
| 263| 263| 	// If we're already dead, don't allow resurrection
| 264| 264| 	if (this.hitpoints == 0)
|    | [NORMAL] ESLintBear (key-spacing):
|    | Missing space before value for key 'new'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Health.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Health.js
| 258| 258| 		cmpFogging.Activate();
| 259| 259| 
| 260| 260| 	if (this.hitpoints == this.GetMaxHitpoints())
| 261|    |-		return {"old": this.hitpoints, "new":this.hitpoints};
|    | 261|+		return {"old": this.hitpoints, "new": this.hitpoints};
| 262| 262| 
| 263| 263| 	// If we're already dead, don't allow resurrection
| 264| 264| 	if (this.hitpoints == 0)
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required before '}'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Health.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Health.js
| 258| 258| 		cmpFogging.Activate();
| 259| 259| 
| 260| 260| 	if (this.hitpoints == this.GetMaxHitpoints())
| 261|    |-		return {"old": this.hitpoints, "new":this.hitpoints};
|    | 261|+		return {"old": this.hitpoints, "new":this.hitpoints };
| 262| 262| 
| 263| 263| 	// If we're already dead, don't allow resurrection
| 264| 264| 	if (this.hitpoints == 0)
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required before '}'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Health.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Health.js
| 276| 276| 
| 277| 277| 	this.RegisterHealthChanged(old);
| 278| 278| 
| 279|    |-	return { "old": old, "new": this.hitpoints};
|    | 279|+	return { "old": old, "new": this.hitpoints };
| 280| 280| };
| 281| 281| 
| 282| 282| //// Private functions ////
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Health.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Health.js
| 279| 279| 	return { "old": old, "new": this.hitpoints};
| 280| 280| };
| 281| 281| 
| 282|    |-//// Private functions ////
|    | 282|+// // Private functions ////
| 283| 283| 
| 284| 284| Health.prototype.CreateCorpse = function(leaveResources)
| 285| 285| {

binaries/data/mods/public/simulation/components/Health.js
| 117| »   »   ||·this.GetHitpoints()·<=·0
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/Health.js
| 118| »   »   ||·this.GetHitpoints()·>=·this.GetMaxHitpoints());
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/Health.js
| 171| »   var·cmpTimer·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_Timer);
|    | [NORMAL] JSHintBear:
|    | 'cmpTimer' is already defined.

binaries/data/mods/public/simulation/components/Health.js
| 230| »   »   »   }
|    | [NORMAL] JSHintBear:
|    | Expected a 'break' statement before 'case'.

Link to build: https://jenkins.wildfiregames.com/job/differential/876/

Harbormaster completed remote builds in B6657: Diff 7196.Jan 2 2019, 7:03 PM

Is it really how god intended it though?
However I think a spear cav not being promoted at 149.9999997/150 is absolutely not what was intended.
I was under the impression that such things should be real numbers.

I am not really sure though.

Actually I don't understand why this is necessary. Attack damage and resource costs aren't integers either, right? Perhaps you could elaborate why health should be?

None of these are integer (and this doesn't change that) in the actual JS code, but the GUI pretends they are and the templates could/should pretend they are too. The only thing this does is make it impossible to create a 0.5HP unit by default