Page MenuHomeWildfire Games

Do not mark unit as injured when receives 0 damage
ClosedPublic

Authored by Angen 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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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.

Angen 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
100 ↗(On Diff #7416)

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

270 ↗(On Diff #7416)

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

Angen added a comment.Jan 29 2019, 9:06 PM

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
247 ↗(On Diff #7416)

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

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

@wraitii extend this diff or make another one?

Angen updated this revision to Diff 7727.Fri, Apr 12, 5:18 PM
Angen edited the summary of this revision. (Show Details)
Angen 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

Angen planned changes to this revision.Fri, Apr 12, 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.Fri, Apr 12, 6:39 PM
binaries/data/mods/public/simulation/components/Health.js
206 ↗(On Diff #7728)

IsDead/IsAlive/ShouldDie/ ?

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

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

binaries/data/mods/public/simulation/components/Health.js
204 ↗(On Diff #7728)

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

206 ↗(On Diff #7728)

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

210 ↗(On Diff #7728)

Likewise this comment should be moved back to Reduce()

212 ↗(On Diff #7728)

return;
(see below)

249 ↗(On Diff #7728)

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

This revision now requires changes to proceed.Sat, Apr 13, 10:36 AM
Angen updated this revision to Diff 7748.Sun, Apr 14, 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.Mon, Apr 15, 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
194 ↗(On Diff #7748)

appears to be a whittling oddity here

198 ↗(On Diff #7748)

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.Mon, Apr 15, 1:42 PM
Stan added inline comments.Mon, Apr 15, 2:21 PM
binaries/data/mods/public/simulation/components/Health.js
198 ↗(On Diff #7748)

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

Stan added inline comments.Mon, Apr 15, 2:23 PM
binaries/data/mods/public/simulation/components/Health.js
175 ↗(On Diff #7748)

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

204 ↗(On Diff #7728)

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

Angen added inline comments.Mon, Apr 15, 2:42 PM
binaries/data/mods/public/simulation/components/Health.js
198 ↗(On Diff #7748)

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

Stan added inline comments.Mon, Apr 15, 3:38 PM
binaries/data/mods/public/simulation/components/Health.js
198 ↗(On Diff #7748)

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
198 ↗(On Diff #7748)

Yes that's better than a comment actually.

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

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

Angen added a comment.Mon, Apr 15, 9:15 PM

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 :)

Angen updated this revision to Diff 7754.Mon, Apr 15, 9:57 PM
Angen 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.Mon, Apr 15, 11:05 PM
binaries/data/mods/public/simulation/components/Health.js
175 ↗(On Diff #7754)

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 :)

smiley added a subscriber: smiley.Tue, Apr 16, 5:19 PM
smiley added inline comments.
binaries/data/mods/public/simulation/components/Health.js
189 ↗(On Diff #7754)

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

smiley removed a subscriber: smiley.Tue, Apr 16, 5:19 PM
wraitii added inline comments.Tue, Apr 16, 5:39 PM
binaries/data/mods/public/simulation/components/Health.js
189 ↗(On Diff #7754)

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

smiley added inline comments.Tue, Apr 16, 6:47 PM
binaries/data/mods/public/simulation/components/Health.js
189 ↗(On Diff #7754)

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

Angen updated this revision to Diff 7759.Tue, Apr 16, 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.Wed, Apr 17, 9:38 AM

Can you fix the JSDOC comment ? :)

binaries/data/mods/public/simulation/components/Health.js
183 ↗(On Diff #7759)

Are you sure that's not true ?

This comment was removed by wraitii.
Stan added a comment.EditedWed, Apr 17, 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.

Angen added inline comments.Wed, Apr 17, 3:37 PM
binaries/data/mods/public/simulation/components/Health.js
183 ↗(On Diff #7759)

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.