Changeset View
Standalone View
binaries/data/mods/public/simulation/components/Health.js
Show First 20 Lines • Show All 215 Lines • ▼ Show 20 Lines | if (amount >= this.hitpoints) | ||||
// (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) | ||||
{ | { | ||||
state.killed = true; | state.killed = true; | ||||
PlaySound("death", this.entity); | PlaySound("death", this.entity); | ||||
// If SpawnEntityOnDeath is set, spawn the entity | // If SpawnEntityOnDeath is set, spawn the entity | ||||
leper: Pointless comment (while we're here). | |||||
if(this.template.SpawnEntityOnDeath) | if(this.template.SpawnEntityOnDeath) | ||||
Done Inline Actionsif ( while we're here. leper: `if (` while we're here. | |||||
this.CreateDeathSpawnedEntity(); | this.CreateDeathSpawnedEntity(); | ||||
if (this.template.DeathType == "corpse") | if (this.template.DeathType == "corpse") | ||||
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… | |||||
{ | { | ||||
this.CreateCorpse(); | this.CreateCorpse(); | ||||
Engine.DestroyEntity(this.entity); | Engine.DestroyEntity(this.entity); | ||||
} | } | ||||
else if (this.template.DeathType == "vanish") | else if (this.template.DeathType == "vanish") | ||||
{ | { | ||||
Engine.DestroyEntity(this.entity); | Engine.DestroyEntity(this.entity); | ||||
} | } | ||||
else if (this.template.DeathType == "remain") | else if (this.template.DeathType == "remain") | ||||
{ | { | ||||
var resource = this.CreateCorpse(true); | var 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); | 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; | this.hitpoints = 0; | ||||
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); | this.RegisterHealthChanged(oldHitpoints); | ||||
let cmpAttack = Engine.QueryInterface(this.entity, IID_Attack); | |||||
if (cmpAttack) | |||||
cmpAttack.CauseDeathDamage(); | |||||
fatherbushidoUnsubmitted 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? | |||||
Mate-86AuthorUnsubmitted 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 | |||||
fatherbushidoUnsubmitted 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)… | |||||
leperUnsubmitted 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… | |||||
} | } | ||||
} | } | ||||
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).