Changeset View
Standalone View
binaries/data/mods/public/simulation/components/Health.js
Show First 20 Lines • Show All 211 Lines • ▼ Show 20 Lines | Health.prototype.Reduce = function(amount) | ||||
var oldHitpoints = this.hitpoints; | var oldHitpoints = this.hitpoints; | ||||
if (amount >= this.hitpoints) | if (amount >= this.hitpoints) | ||||
{ | { | ||||
// If this is the first time we reached 0, then die. | // If this is the first time we reached 0, then die. | ||||
// (The entity will exist a little while after calling DestroyEntity so this | // (The entity will exist a little while after calling DestroyEntity so this | ||||
// might get called multiple times) | // might get called multiple times) | ||||
if (this.hitpoints) | if (this.hitpoints) | ||||
{ | { | ||||
this.hitpoints = 0; | |||||
this.RegisterHealthChanged(oldHitpoints); | |||||
state.killed = true; | state.killed = true; | ||||
let cmpDeathDamage = Engine.QueryInterface(this.entity, IID_DeathDamage); | |||||
if (cmpDeathDamage) | |||||
cmpDeathDamage.CauseDeathDamage(); | |||||
PlaySound("death", this.entity); | PlaySound("death", this.entity); | ||||
// If SpawnEntityOnDeath is set, spawn the entity | |||||
if(this.template.SpawnEntityOnDeath) | if (this.template.SpawnEntityOnDeath) | ||||
leper: Pointless comment (while we're here). | |||||
Done Inline Actionsif ( while we're here. leper: `if (` while we're here. | |||||
this.CreateDeathSpawnedEntity(); | this.CreateDeathSpawnedEntity(); | ||||
if (this.template.DeathType == "corpse") | switch (this.template.DeathType) | ||||
Done Inline ActionsWe could use a switch for these. And do the Engine.DestroyEntity(this.entity); call afterwards. leper: We could use a switch for these.
And do the `Engine.DestroyEntity(this.entity);` call… | |||||
{ | { | ||||
case "corpse": | |||||
this.CreateCorpse(); | this.CreateCorpse(); | ||||
Engine.DestroyEntity(this.entity); | break; | ||||
} | |||||
else if (this.template.DeathType == "vanish") | case "remain": | ||||
{ | |||||
Engine.DestroyEntity(this.entity); | |||||
} | |||||
else if (this.template.DeathType == "remain") | |||||
{ | { | ||||
var resource = this.CreateCorpse(true); | let resource = this.CreateCorpse(true); | ||||
if (resource != INVALID_ENTITY) | if (resource != INVALID_ENTITY) | ||||
Engine.BroadcastMessage(MT_EntityRenamed, { entity: this.entity, newentity: resource }); | Engine.BroadcastMessage(MT_EntityRenamed, { entity: this.entity, newentity: resource }); | ||||
Engine.DestroyEntity(this.entity); | |||||
} | } | ||||
Done Inline ActionsThat's not how we format switch statements. See https://trac.wildfiregames.com/wiki/Coding_Conventions. leper: That's not how we format `switch` statements. See https://trac.wildfiregames. | |||||
Done Inline ActionsI'd actually add a case "vanish": break; default: error("Invalid deathtype") break; (with a slightly nicer error message) here, to make this a bit more robust against future extensions. leper: I'd actually add a `case "vanish": break; default: error("Invalid deathtype") break;` (with a… | |||||
this.hitpoints = 0; | case "vanish": | ||||
Done Inline ActionsIf the issue hinted at in the below comment we could just move the hitpoints assignment to an earlier place, which shouldn't result in anything, as long as we leave the message sending part down here (someone actually test and verify this, I'm barely guessing here). leper: If the issue hinted at in the below comment we could just move the hitpoints assignment to an… | |||||
this.RegisterHealthChanged(oldHitpoints); | break; | ||||
default: | |||||
error("Invalid template.DeathType: " + this.template.DeathType); | |||||
Done Inline ActionsI already ask that but shouldn't we put that earlier? fatherbushido: I already ask that but shouldn't we put that earlier? | |||||
Done Inline ActionsYes, you asked and I replied in a comment: https://trac.wildfiregames.com/ticket/1910#comment:33 Mate-86: Yes, you asked and I replied in a comment: https://trac.wildfiregames.com/ticket/1910#comment:33 | |||||
Done Inline ActionsThat needs to be dug into. fatherbushido: That needs to be dug into.
Perhaps before the first if: "if (can this entity do death damage)… | |||||
Done Inline ActionsIt is quite unlikely for the entity to actually be gone by that point, however from a logical standpoint doing this first seems better. (One could also do the actual damage code in here, however that then creates some kind of coupling between Health and damge related things.) leper: It is quite unlikely for the entity to actually be gone by that point, however from a logical… | |||||
break; | |||||
} | |||||
Engine.DestroyEntity(this.entity); | |||||
} | } | ||||
} | } | ||||
else | else | ||||
{ | { | ||||
this.hitpoints -= amount; | this.hitpoints -= amount; | ||||
this.RegisterHealthChanged(oldHitpoints); | this.RegisterHealthChanged(oldHitpoints); | ||||
} | } | ||||
▲ Show 20 Lines • Show All 166 Lines • Show Last 20 Lines |
Pointless comment (while we're here).