Page MenuHomeWildfire Games

Do not mark unit as injured when receives 0 damage
ClosedPublic

Authored by Silier on Jan 28 2019, 9:36 PM.

Details

Summary

Currently if unit is damaged with 0, it is marked as injured.

Do not do anything when damage is 0.
Do not mark units as injured when they are full health and killed at one shot.
What happens on death has been moved to separate function.
Applied early returns.

Test Plan

Check that entities still die and do not stay with 0 or less HP.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 7162
Build 11676: Vulcan BuildJenkins
Build 11675: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Stan awarded a token.Jan 28 2019, 9:38 PM
Stan added reviewers: bb, elexis.
Vulcan added a subscriber: Vulcan.Jan 28 2019, 9:38 PM

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

Linter detected issues:
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'.
Executing section cli...

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

elexis added a comment.EditedJan 29 2019, 1:21 PM

Was there a reason for the == case?

Agree.

Like subjectively?

Imarok added a subscriber: Imarok.Jan 29 2019, 3:14 PM
In D1769#71333, @elexis wrote:

Was there a reason for the == case?

Was there since leper implemented Healing in rP11536

Stan added a subscriber: Stan.

I guess it's there not to keep marking the unit as injured every time you get through this. Dunno what's the performance impact, but maybe it's negligible. One could also check for the RangeManager entity flag each time, but that's more lines.

Silier added a comment.EditedJan 29 2019, 8:14 PM

If unit is marked as injured, healers get it as unit to heal. So if unit with maxed health would be marked as as injured, healers would waist their time for unit that does not need it. I think, can someone think that this is not good change and disagree ?

Also there is actually check in UnitAI if unit is can be healed where is hp == maxHp taken into account, but unit still stays marked as injured.

Stan added a comment.Jan 29 2019, 8:37 PM
In D1769#71392, @Angen wrote:

If unit is marked as injured, healers get it as unit to heal. So if unit with maxed health would be marked as as injured, healers would waist their time for unit that does not need it. I think, can someone think that this is not good change and disagree ?

So "injured" is only used by healers ? It doesn't set a variant in cmpVisual ? (Just checked, no)

Also there is actually check in UnitAI if unit is can be healed where is hp == maxHp taken into account, but unit still stays marked as injured.

Does the duplication make sense, or is it just UnitAI being weird ? In this case maybe it would be nice to include it in this or another patch.

binaries/data/mods/public/simulation/components/Health.js
96–97

This is another example it's fixed in D1770, but maybe the checks should be unified.

276

I guess the weirdness above is fixed here. Unit gets an increase, then cancel.

In unitAi healer filters possible targets with injured mark using Health.prototype.IsUnhealable function (and another checks) so unit with maxed health is not healed and stays injured and checked in next search if still in range.

wraitii requested changes to this revision.Jan 30 2019, 8:36 AM
wraitii added a subscriber: wraitii.

Let's just refactor this function entirely instead. See inline comments.

binaries/data/mods/public/simulation/components/Health.js
241–244

Handle death in a separate function and return early, add a check for "amount > 0" at the very top - no need to do anything on 0 damage - then move the "injured" change below the death-check.

This revision now requires changes to proceed.Jan 30 2019, 8:36 AM

If we do a big refactoring in the same patch, then the >= 0 > 0 change will be hidden therein. So perhaps better to do that separately?

Commit message can specify that imo. And anybody looking will find this diff and notice what happened.

Stan added a comment.Jan 31 2019, 3:49 PM

Needs a rebase after D1770

Silier added a comment.Feb 3 2019, 3:30 PM

@wraitii extend this diff or make another one?

Silier updated this revision to Diff 7727.Apr 12 2019, 5:18 PM
Silier edited the summary of this revision. (Show Details)
Silier edited the test plan for this revision. (Show Details)

refactor

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [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
| 177| 177| Health.prototype.Reduce = function(amount)
| 178| 178| {
| 179| 179| 	if (!amount)
| 180|    |-		return {"killed": false, "change": 0};
|    | 180|+		return { "killed": false, "change": 0};
| 181| 181| 	// Before changing the value, activate Fogging if necessary to hide changes
| 182| 182| 	let cmpFogging = Engine.QueryInterface(this.entity, IID_Fogging);
| 183| 183| 	if (cmpFogging)
|    | [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
| 177| 177| Health.prototype.Reduce = function(amount)
| 178| 178| {
| 179| 179| 	if (!amount)
| 180|    |-		return {"killed": false, "change": 0};
|    | 180|+		return {"killed": false, "change": 0 };
| 181| 181| 	// Before changing the value, activate Fogging if necessary to hide changes
| 182| 182| 	let cmpFogging = Engine.QueryInterface(this.entity, IID_Fogging);
| 183| 183| 	if (cmpFogging)
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /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
| 185| 185| 
| 186| 186| 	let state = { "killed": false };
| 187| 187| 	let oldHitpoints = this.hitpoints;
| 188|    |-	
|    | 188|+
| 189| 189| 	if (amount >= this.hitpoints)
| 190| 190| 	{
| 191| 191| 		state.killed = this.DeathCheck();
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /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
| 192| 192| 		state.change = this.hitpoints - oldHitpoints;
| 193| 193| 		return state;
| 194| 194| 	}
| 195|    |-	
|    | 195|+
| 196| 196| 	if (this.hitpoints == this.GetMaxHitpoints())
| 197| 197| 	{
| 198| 198| 		let cmpRangeManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_RangeManager);
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /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
| 199| 199| 		if (cmpRangeManager)
| 200| 200| 			cmpRangeManager.SetEntityFlag(this.entity, "injured", true);
| 201| 201| 	}
| 202|    |-	
|    | 202|+
| 203| 203| 	this.hitpoints -= amount;
| 204| 204| 	this.RegisterHealthChanged(oldHitpoints);
| 205| 205| 	state.change = this.hitpoints - oldHitpoints;
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /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
| 217| 217| 	// might get called multiple times)
| 218| 218| 	if (!this.hitpoints)
| 219| 219| 		return false;
| 220|    |-	
|    | 220|+
| 221| 221| 	let oldHitpoints = this.hitpoints;
| 222| 222| 	this.hitpoints = 0;
| 223| 223| 	this.RegisterHealthChanged(oldHitpoints);
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /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
| 221| 221| 	let oldHitpoints = this.hitpoints;
| 222| 222| 	this.hitpoints = 0;
| 223| 223| 	this.RegisterHealthChanged(oldHitpoints);
| 224|    |-		
|    | 224|+
| 225| 225| 	let cmpDeathDamage = Engine.QueryInterface(this.entity, IID_DeathDamage);
| 226| 226| 	if (cmpDeathDamage)
| 227| 227| 		cmpDeathDamage.CauseDeathDamage();
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /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
| 251| 251| 		error("Invalid template.DeathType: " + this.template.DeathType);
| 252| 252| 	    break;
| 253| 253| 	}
| 254|    |-	
|    | 254|+
| 255| 255| 	Engine.DestroyEntity(this.entity);
| 256| 256| 	return true;
| 257| 257| }
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /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
| 254| 254| 	
| 255| 255| 	Engine.DestroyEntity(this.entity);
| 256| 256| 	return true;
| 257|    |-}
|    | 257|+};
| 258| 258| 
| 259| 259| Health.prototype.Increase = function(amount)
| 260| 260| {

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

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

Silier planned changes to this revision.Apr 12 2019, 5:27 PM

found flaw

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [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
| 177| 177| Health.prototype.Reduce = function(amount)
| 178| 178| {
| 179| 179| 	if (!amount || !this.hitpoints)
| 180|    |-		return {"killed": false, "change": 0};
|    | 180|+		return { "killed": false, "change": 0};
| 181| 181| 	// Before changing the value, activate Fogging if necessary to hide changes
| 182| 182| 	let cmpFogging = Engine.QueryInterface(this.entity, IID_Fogging);
| 183| 183| 	if (cmpFogging)
|    | [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
| 177| 177| Health.prototype.Reduce = function(amount)
| 178| 178| {
| 179| 179| 	if (!amount || !this.hitpoints)
| 180|    |-		return {"killed": false, "change": 0};
|    | 180|+		return {"killed": false, "change": 0 };
| 181| 181| 	// Before changing the value, activate Fogging if necessary to hide changes
| 182| 182| 	let cmpFogging = Engine.QueryInterface(this.entity, IID_Fogging);
| 183| 183| 	if (cmpFogging)
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /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
| 182| 182| 	let cmpFogging = Engine.QueryInterface(this.entity, IID_Fogging);
| 183| 183| 	if (cmpFogging)
| 184| 184| 		cmpFogging.Activate();
| 185|    |-	
|    | 185|+
| 186| 186| 	if (amount >= this.hitpoints)
| 187| 187| 		return this.DeathCheck();
| 188| 188| 	
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /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
| 185| 185| 	
| 186| 186| 	if (amount >= this.hitpoints)
| 187| 187| 		return this.DeathCheck();
| 188|    |-	
|    | 188|+
| 189| 189| 	if (this.hitpoints == this.GetMaxHitpoints())
| 190| 190| 	{
| 191| 191| 		let cmpRangeManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_RangeManager);
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /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
| 192| 192| 		if (cmpRangeManager)
| 193| 193| 			cmpRangeManager.SetEntityFlag(this.entity, "injured", true);
| 194| 194| 	}
| 195|    |-	
|    | 195|+
| 196| 196| 	let oldHitpoints = this.hitpoints;
| 197| 197| 	this.hitpoints -= amount;
| 198| 198| 	this.RegisterHealthChanged(oldHitpoints);
|    | [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
| 196| 196| 	let oldHitpoints = this.hitpoints;
| 197| 197| 	this.hitpoints -= amount;
| 198| 198| 	this.RegisterHealthChanged(oldHitpoints);
| 199|    |-	return {"killed": false, "change": (this.hitpoints - oldHitpoints)};
|    | 199|+	return { "killed": false, "change": (this.hitpoints - oldHitpoints)};
| 200| 200| };
| 201| 201| 
| 202| 202| /**
|    | [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
| 196| 196| 	let oldHitpoints = this.hitpoints;
| 197| 197| 	this.hitpoints -= amount;
| 198| 198| 	this.RegisterHealthChanged(oldHitpoints);
| 199|    |-	return {"killed": false, "change": (this.hitpoints - oldHitpoints)};
|    | 199|+	return {"killed": false, "change": (this.hitpoints - oldHitpoints) };
| 200| 200| };
| 201| 201| 
| 202| 202| /**
|    | [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
| 209| 209| 	// (The entity will exist a little while after calling DestroyEntity so this
| 210| 210| 	// might get called multiple times)
| 211| 211| 	if (!this.hitpoints)
| 212|    |-		return {"killed": false, "change": 0};
|    | 212|+		return { "killed": false, "change": 0};
| 213| 213| 	
| 214| 214| 	let oldHitpoints = this.hitpoints;
| 215| 215| 	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
| 209| 209| 	// (The entity will exist a little while after calling DestroyEntity so this
| 210| 210| 	// might get called multiple times)
| 211| 211| 	if (!this.hitpoints)
| 212|    |-		return {"killed": false, "change": 0};
|    | 212|+		return {"killed": false, "change": 0 };
| 213| 213| 	
| 214| 214| 	let oldHitpoints = this.hitpoints;
| 215| 215| 	this.hitpoints = 0;
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /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
| 210| 210| 	// might get called multiple times)
| 211| 211| 	if (!this.hitpoints)
| 212| 212| 		return {"killed": false, "change": 0};
| 213|    |-	
|    | 213|+
| 214| 214| 	let oldHitpoints = this.hitpoints;
| 215| 215| 	this.hitpoints = 0;
| 216| 216| 	this.RegisterHealthChanged(oldHitpoints);
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /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
| 214| 214| 	let oldHitpoints = this.hitpoints;
| 215| 215| 	this.hitpoints = 0;
| 216| 216| 	this.RegisterHealthChanged(oldHitpoints);
| 217|    |-		
|    | 217|+
| 218| 218| 	let cmpDeathDamage = Engine.QueryInterface(this.entity, IID_DeathDamage);
| 219| 219| 	if (cmpDeathDamage)
| 220| 220| 		cmpDeathDamage.CauseDeathDamage();
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /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
| 244| 244| 		error("Invalid template.DeathType: " + this.template.DeathType);
| 245| 245| 	    break;
| 246| 246| 	}
| 247|    |-	
|    | 247|+
| 248| 248| 	Engine.DestroyEntity(this.entity);
| 249| 249| 	return {"killed": true, "change": -oldHitpoints};
| 250| 250| }
|    | [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
| 246| 246| 	}
| 247| 247| 	
| 248| 248| 	Engine.DestroyEntity(this.entity);
| 249|    |-	return {"killed": true, "change": -oldHitpoints};
|    | 249|+	return { "killed": true, "change": -oldHitpoints};
| 250| 250| }
| 251| 251| 
| 252| 252| Health.prototype.Increase = function(amount)
|    | [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
| 246| 246| 	}
| 247| 247| 	
| 248| 248| 	Engine.DestroyEntity(this.entity);
| 249|    |-	return {"killed": true, "change": -oldHitpoints};
|    | 249|+	return {"killed": true, "change": -oldHitpoints };
| 250| 250| }
| 251| 251| 
| 252| 252| Health.prototype.Increase = function(amount)
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /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
| 247| 247| 	
| 248| 248| 	Engine.DestroyEntity(this.entity);
| 249| 249| 	return {"killed": true, "change": -oldHitpoints};
| 250|    |-}
|    | 250|+};
| 251| 251| 
| 252| 252| Health.prototype.Increase = function(amount)
| 253| 253| {

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

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

Stan added inline comments.Apr 12 2019, 6:39 PM
binaries/data/mods/public/simulation/components/Health.js
234

IsDead/IsAlive/ShouldDie/ ?

wraitii requested changes to this revision.Apr 13 2019, 10:36 AM

Some minor changes are still needed but imo this looks much better.

binaries/data/mods/public/simulation/components/Health.js
232

This will then return null so this can be removed and the above comment amended to "Handle what happens when the entity dies".

234

HandleDeath rather, as this should only be called when we are actually dying. Reduce() should handle all "checks" such as the hotpoint check.

238

Likewise this comment should be moved back to Reduce()

240

return;
(see below)

256

Move this back in Reduce(), this function should only handle what happens on death.

This revision now requires changes to proceed.Apr 13 2019, 10:36 AM
Silier updated this revision to Diff 7748.Apr 14 2019, 4:27 PM

move some code back to Reduce and rename function

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [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
| 177| 177| Health.prototype.Reduce = function(amount)
| 178| 178| {
| 179| 179| 	if (!amount || !this.hitpoints)
| 180|    |-		return {"killed": false, "change": 0};
|    | 180|+		return { "killed": false, "change": 0};
| 181| 181| 	// Before changing the value, activate Fogging if necessary to hide changes
| 182| 182| 	let cmpFogging = Engine.QueryInterface(this.entity, IID_Fogging);
| 183| 183| 	if (cmpFogging)
|    | [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
| 177| 177| Health.prototype.Reduce = function(amount)
| 178| 178| {
| 179| 179| 	if (!amount || !this.hitpoints)
| 180|    |-		return {"killed": false, "change": 0};
|    | 180|+		return {"killed": false, "change": 0 };
| 181| 181| 	// Before changing the value, activate Fogging if necessary to hide changes
| 182| 182| 	let cmpFogging = Engine.QueryInterface(this.entity, IID_Fogging);
| 183| 183| 	if (cmpFogging)
|    | [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
| 192| 192| 		this.hitpoints = 0;
| 193| 193| 		this.RegisterHealthChanged(oldHitpoints);
| 194| 194| 	    this.HandleDeath();
| 195|    |-		return {"killed": true, "change": -oldHitpoints};
|    | 195|+		return { "killed": true, "change": -oldHitpoints};
| 196| 196| 	}
| 197| 197| 
| 198| 198| 	if (this.hitpoints == this.GetMaxHitpoints())
|    | [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
| 192| 192| 		this.hitpoints = 0;
| 193| 193| 		this.RegisterHealthChanged(oldHitpoints);
| 194| 194| 	    this.HandleDeath();
| 195|    |-		return {"killed": true, "change": -oldHitpoints};
|    | 195|+		return {"killed": true, "change": -oldHitpoints };
| 196| 196| 	}
| 197| 197| 
| 198| 198| 	if (this.hitpoints == this.GetMaxHitpoints())
|    | [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
| 204| 204| 
| 205| 205| 	this.hitpoints -= amount;
| 206| 206| 	this.RegisterHealthChanged(oldHitpoints);
| 207|    |-	return {"killed": false, "change": (this.hitpoints - oldHitpoints)};
|    | 207|+	return { "killed": false, "change": (this.hitpoints - oldHitpoints)};
| 208| 208| };
| 209| 209| 
| 210| 210| /**
|    | [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
| 204| 204| 
| 205| 205| 	this.hitpoints -= amount;
| 206| 206| 	this.RegisterHealthChanged(oldHitpoints);
| 207|    |-	return {"killed": false, "change": (this.hitpoints - oldHitpoints)};
|    | 207|+	return {"killed": false, "change": (this.hitpoints - oldHitpoints) };
| 208| 208| };
| 209| 209| 
| 210| 210| /**
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /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
| 243| 243| 	}
| 244| 244| 
| 245| 245| 	Engine.DestroyEntity(this.entity);
| 246|    |-}
|    | 246|+};
| 247| 247| 
| 248| 248| Health.prototype.Increase = function(amount)
| 249| 249| {

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

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

wraitii accepted this revision.Apr 15 2019, 1:42 PM

Acceptable as such, with the two notes handled before committing.
Commit message should detail the behaviour change (i.e. this diff's title).

binaries/data/mods/public/simulation/components/Health.js
195

appears to be a whittling oddity here

199

I feel like this warrants a comment such as "If we were not marked injured, do it now" because it reads weird.

This revision is now accepted and ready to land.Apr 15 2019, 1:42 PM
Stan added inline comments.Apr 15 2019, 2:21 PM
binaries/data/mods/public/simulation/components/Health.js
199

Couldn't that be (!unitInjured && amount > 0) ?

Stan added inline comments.Apr 15 2019, 2:23 PM
binaries/data/mods/public/simulation/components/Health.js
175

@return {{ "killed": boolean, "change": number }} The new state of the entity.

232

@return {{ "killed": boolean, "change": number }} The new state of the entity.

Silier added inline comments.Apr 15 2019, 2:42 PM
binaries/data/mods/public/simulation/components/Health.js
199

hitpoints==maxhittpoints means !injured
amount > 0 is guaranteed by first if in function

Stan added inline comments.Apr 15 2019, 3:38 PM
binaries/data/mods/public/simulation/components/Health.js
199

Since this.GetHitpoints() == this.GetMaxHitpoints() is used in quite a few places, one could make a function called IsInjured() and replace it in the

and unify this.hitpoints and this.GetHitpoints calls.

I'm keeping this as accepted, the committed should do the "IsInjured" function thingy (in a separate commit ideally) before committing

binaries/data/mods/public/simulation/components/Health.js
199

Yes that's better than a comment actually.

Stan added a comment.Apr 15 2019, 8:38 PM

@wraitii usually the reviewer commits the patch. @Angen doesn't have commit access.

I can do separate patch for injured function and to unify this.hittpoints calling but I do not see reason for this diff to wait for it :)

Silier updated this revision to Diff 7754.Apr 15 2019, 9:57 PM
Silier edited the summary of this revision. (Show Details)

some white spaces and comments

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [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
| 177| 177| Health.prototype.Reduce = function(amount)
| 178| 178| {
| 179| 179| 	if (!amount || !this.hitpoints)
| 180|    |-		return {"killed": false, "change": 0};
|    | 180|+		return { "killed": false, "change": 0};
| 181| 181| 	// Before changing the value, activate Fogging if necessary to hide changes
| 182| 182| 	let cmpFogging = Engine.QueryInterface(this.entity, IID_Fogging);
| 183| 183| 	if (cmpFogging)
|    | [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
| 177| 177| Health.prototype.Reduce = function(amount)
| 178| 178| {
| 179| 179| 	if (!amount || !this.hitpoints)
| 180|    |-		return {"killed": false, "change": 0};
|    | 180|+		return {"killed": false, "change": 0 };
| 181| 181| 	// Before changing the value, activate Fogging if necessary to hide changes
| 182| 182| 	let cmpFogging = Engine.QueryInterface(this.entity, IID_Fogging);
| 183| 183| 	if (cmpFogging)
|    | [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
| 192| 192| 		this.hitpoints = 0;
| 193| 193| 		this.RegisterHealthChanged(oldHitpoints);
| 194| 194| 		this.HandleDeath();
| 195|    |-		return {"killed": true, "change": -oldHitpoints};
|    | 195|+		return { "killed": true, "change": -oldHitpoints};
| 196| 196| 	}
| 197| 197| 
| 198| 198| 	// If we are not marked as injured, do it now
|    | [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
| 192| 192| 		this.hitpoints = 0;
| 193| 193| 		this.RegisterHealthChanged(oldHitpoints);
| 194| 194| 		this.HandleDeath();
| 195|    |-		return {"killed": true, "change": -oldHitpoints};
|    | 195|+		return {"killed": true, "change": -oldHitpoints };
| 196| 196| 	}
| 197| 197| 
| 198| 198| 	// If we are not marked as injured, do it now
|    | [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
| 205| 205| 
| 206| 206| 	this.hitpoints -= amount;
| 207| 207| 	this.RegisterHealthChanged(oldHitpoints);
| 208|    |-	return {"killed": false, "change": (this.hitpoints - oldHitpoints)};
|    | 208|+	return { "killed": false, "change": (this.hitpoints - oldHitpoints)};
| 209| 209| };
| 210| 210| 
| 211| 211| /**
|    | [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
| 205| 205| 
| 206| 206| 	this.hitpoints -= amount;
| 207| 207| 	this.RegisterHealthChanged(oldHitpoints);
| 208|    |-	return {"killed": false, "change": (this.hitpoints - oldHitpoints)};
|    | 208|+	return {"killed": false, "change": (this.hitpoints - oldHitpoints) };
| 209| 209| };
| 210| 210| 
| 211| 211| /**
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /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
| 244| 244| 	}
| 245| 245| 
| 246| 246| 	Engine.DestroyEntity(this.entity);
| 247|    |-}
|    | 247|+};
| 248| 248| 
| 249| 249| Health.prototype.Increase = function(amount)
| 250| 250| {

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

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

I intend to commit this probably this week, unless someone else does so first.

Stan added inline comments.Apr 15 2019, 11:05 PM
binaries/data/mods/public/simulation/components/Health.js
175

Actually you can use mine. If your editor is smart it will automatically guess the return type to be an object and offer autosuggestion which is nice :)

lyv added a subscriber: lyv.Apr 16 2019, 5:19 PM
lyv added inline comments.
binaries/data/mods/public/simulation/components/Health.js
190

(Just out of curiosity, is the case mentioned in this comment handled?)

lyv removed a subscriber: lyv.Apr 16 2019, 5:19 PM
wraitii added inline comments.Apr 16 2019, 5:39 PM
binaries/data/mods/public/simulation/components/Health.js
190

Yes by virtue of the check on the first line.
The comment probably should be moved though now.

lyv added inline comments.Apr 16 2019, 6:47 PM
binaries/data/mods/public/simulation/components/Health.js
190

Ah yes, completely missed that line. And yes, the comment would be more helpful up there.

Silier updated this revision to Diff 7759.Apr 16 2019, 7:34 PM

move comment to better place

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [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
| 180| 180| 	// (The entity will exist a little while after calling DestroyEntity so this
| 181| 181| 	// might get called multiple times)
| 182| 182| 	if (!amount || !this.hitpoints)
| 183|    |-		return {"killed": false, "change": 0};
|    | 183|+		return { "killed": false, "change": 0};
| 184| 184| 	// Before changing the value, activate Fogging if necessary to hide changes
| 185| 185| 	let cmpFogging = Engine.QueryInterface(this.entity, IID_Fogging);
| 186| 186| 	if (cmpFogging)
|    | [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
| 180| 180| 	// (The entity will exist a little while after calling DestroyEntity so this
| 181| 181| 	// might get called multiple times)
| 182| 182| 	if (!amount || !this.hitpoints)
| 183|    |-		return {"killed": false, "change": 0};
|    | 183|+		return {"killed": false, "change": 0 };
| 184| 184| 	// Before changing the value, activate Fogging if necessary to hide changes
| 185| 185| 	let cmpFogging = Engine.QueryInterface(this.entity, IID_Fogging);
| 186| 186| 	if (cmpFogging)
|    | [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
| 193| 193| 		this.hitpoints = 0;
| 194| 194| 		this.RegisterHealthChanged(oldHitpoints);
| 195| 195| 		this.HandleDeath();
| 196|    |-		return {"killed": true, "change": -oldHitpoints};
|    | 196|+		return { "killed": true, "change": -oldHitpoints};
| 197| 197| 	}
| 198| 198| 
| 199| 199| 	// If we are not marked as injured, do it now
|    | [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
| 193| 193| 		this.hitpoints = 0;
| 194| 194| 		this.RegisterHealthChanged(oldHitpoints);
| 195| 195| 		this.HandleDeath();
| 196|    |-		return {"killed": true, "change": -oldHitpoints};
|    | 196|+		return {"killed": true, "change": -oldHitpoints };
| 197| 197| 	}
| 198| 198| 
| 199| 199| 	// If we are not marked as injured, do it now
|    | [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
| 206| 206| 
| 207| 207| 	this.hitpoints -= amount;
| 208| 208| 	this.RegisterHealthChanged(oldHitpoints);
| 209|    |-	return {"killed": false, "change": (this.hitpoints - oldHitpoints)};
|    | 209|+	return { "killed": false, "change": (this.hitpoints - oldHitpoints)};
| 210| 210| };
| 211| 211| 
| 212| 212| /**
|    | [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
| 206| 206| 
| 207| 207| 	this.hitpoints -= amount;
| 208| 208| 	this.RegisterHealthChanged(oldHitpoints);
| 209|    |-	return {"killed": false, "change": (this.hitpoints - oldHitpoints)};
|    | 209|+	return {"killed": false, "change": (this.hitpoints - oldHitpoints) };
| 210| 210| };
| 211| 211| 
| 212| 212| /**
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /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
| 245| 245| 	}
| 246| 246| 
| 247| 247| 	Engine.DestroyEntity(this.entity);
| 248|    |-}
|    | 248|+};
| 249| 249| 
| 250| 250| Health.prototype.Increase = function(amount)
| 251| 251| {

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

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

Stan added a comment.Apr 17 2019, 9:38 AM

Can you fix the JSDOC comment ? :)

binaries/data/mods/public/simulation/components/Health.js
183

Are you sure that's not true ?

This comment was removed by wraitii.
Stan added a comment.EditedApr 17 2019, 10:29 AM

That works however I'm not sure what are the bounds of the generated number. I tried to divide it by std::limits<int>::max() then cast it to float but I only get 10^-6 numbers. Also most of the generation is between 0 and 1

Wrong diff.

Silier added inline comments.Apr 17 2019, 3:37 PM
binaries/data/mods/public/simulation/components/Health.js
183

yes, when you look at current code, killed = true only if entity is killed by taking damage, if it is dead allready, killed is false

the reason is that if some entity tries to injure dead entity, it will not get loot and statistics. see CauseDamage in Damage.js

This revision was automatically updated to reflect the committed changes.