Changeset View
Standalone View
binaries/data/mods/public/simulation/components/Damage.js
| Show First 20 Lines • Show All 161 Lines • ▼ Show 20 Lines | |||||
| * @param {Vector3D} data.direction - the unit vector defining the direction. | * @param {Vector3D} data.direction - the unit vector defining the direction. | ||||
| * @param {number[]} data.playersToDamage - the array of player id's to damage. | * @param {number[]} data.playersToDamage - the array of player id's to damage. | ||||
| */ | */ | ||||
| Damage.prototype.CauseSplashDamage = function(data) | Damage.prototype.CauseSplashDamage = function(data) | ||||
| { | { | ||||
| // Get nearby entities and define variables | // Get nearby entities and define variables | ||||
| let nearEnts = this.EntitiesNearPoint(data.origin, data.radius, data.playersToDamage); | let nearEnts = this.EntitiesNearPoint(data.origin, data.radius, data.playersToDamage); | ||||
| let damageMultiplier = 1; | let damageMultiplier = 1; | ||||
| // Cycle through all the nearby entities and damage it appropriately based on its distance from the origin. | // Cycle through all the nearby entities and damage it appropriately based on its distance from the origin. | ||||
fatherbushido: I don't know if it's better here (for vs if).
'direction' instead of 'dir' is perhaps a better… | |||||
Not Done Inline ActionsI changed it because in vector.js I saw the comment "// Note that object creation is slow in JS, so use them only when necessary". We don't use dir with circular damage, but I thought it would be better to create it once every time rather than lots of times in the linear case. (Of course we could move the ifs outside the for loop, i.e. have two for loops, but that would repeat a lot of code.) temple: I changed it because in vector.js I saw the comment "// Note that object creation is slow in JS… | |||||
Not Done Inline ActionsI had understood. fatherbushido: I had understood. | |||||
Not Done Inline ActionsDon't forget to renomarlize. fatherbushido: Don't forget to renomarlize. | |||||
| for (let ent of nearEnts) | for (let ent of nearEnts) | ||||
| { | { | ||||
| let entityPosition = Engine.QueryInterface(ent, IID_Position).GetPosition2D(); | let entityPosition = Engine.QueryInterface(ent, IID_Position).GetPosition2D(); | ||||
Not Done Inline ActionsI reconsidered interpolating following your remarks. fatherbushido: I reconsidered interpolating following your remarks.
That's not a big cost and we'll just have… | |||||
Not Done Inline ActionsIt affects all splash damage so I think it's probably better in a separate patch. temple: It affects all splash damage so I think it's probably better in a separate patch. | |||||
| if (data.shape == 'Circular') // circular effect with quadratic falloff in every direction | if (data.shape == 'Circular') // circular effect with quadratic falloff in every direction | ||||
| damageMultiplier = 1 - data.origin.distanceToSquared(entityPosition) / (data.radius * data.radius); | damageMultiplier = 1 - data.origin.distanceToSquared(entityPosition) / (data.radius * data.radius); | ||||
| else if (data.shape == 'Linear') // linear effect with quadratic falloff in two directions (only used for certain missiles) | else if (data.shape == 'Linear') // linear effect with quadratic falloff in two directions (only used for certain missiles) | ||||
| { | { | ||||
| // Get position of entity relative to splash origin. | // Get position of entity relative to splash origin. | ||||
| let relativePos = entityPosition.sub(data.origin); | let relativePos = entityPosition.sub(data.origin); | ||||
| // The width of linear splash is one fifth of the normal splash radius. | // The width of linear splash is one fifth of the normal splash radius. | ||||
| let width = data.radius / 5; | let width = data.radius / 5; | ||||
| // Effectivly rotate the axis to align with the missile direction. | // Distances parallel and perpendicular to the missile direction | ||||
| let parallelDist = relativePos.dot(data.direction); // z axis | let dir = Vector2D.from3D(data.direction); | ||||
| let perpDist = Math.abs(relativePos.cross(data.direction)); // y axis | let parallelDist = relativePos.dot(dir); | ||||
| let perpDist = Math.abs(relativePos.cross(dir)); | |||||
Not Done Inline ActionsThe previous comment was not so bad, but why not. fatherbushido: The previous comment was not so bad, but why not. | |||||
| // Check that the unit is within the distance at which it will get damaged. | // Check that the unit is within the distance at which it will get damaged. | ||||
| if (parallelDist > -width && perpDist < width) // If in radius, quadratic falloff in both directions | if (parallelDist > -width && perpDist < width) // If in radius, quadratic falloff in both directions | ||||
Not Done Inline ActionsparallelDist > -width is not the condition to check parallelDist < data.radius (and applying Math.abs() at L187) fatherbushido: parallelDist > -width is not the condition to check
One should in fact check
```
parallelDist… | |||||
Not Done Inline ActionsThat's already checked when finding nearEnts. temple: That's already checked when finding nearEnts.
What parallelDist > -width is saying is only… | |||||
Not Done Inline ActionsCould you reconsider it (even if we choose to splash only the targets before or behind)? fatherbushido: Could you reconsider it (even if we choose to splash only the targets before or behind)? | |||||
Not Done Inline Actions(I am a bit stupid too: fatherbushido: (I am a bit stupid too:
parallelDist > -width could just be nuked
or replaced by parallelDist… | |||||
Not Done Inline ActionsI still would change that by nuking the first condition (or using the redundant one) fatherbushido: I still would change that by nuking the first condition (or using the redundant one) | |||||
| damageMultiplier = (data.radius * data.radius - parallelDist * parallelDist) / (data.radius * data.radius) * | damageMultiplier = (data.radius * data.radius - parallelDist * parallelDist) / (data.radius * data.radius) * | ||||
| (width * width - perpDist * perpDist) / (width * width); | (width * width - perpDist * perpDist) / (width * width); | ||||
Not Done Inline ActionsTo avoid implicit multiplication questions and perhaps simplify the expression, I would perhaps replace by (1 - parallelDist * parallelDist / (data.radius * data.radius)) * (1 - perpDist * perpDist / (width * width)); fatherbushido: To avoid implicit multiplication questions and perhaps simplify the expression, I would perhaps… | |||||
Not Done Inline ActionsparallelPos > -width -> parallelPos >=0 and I commit it :-) fatherbushido: parallelPos > -width -> parallelPos >=0 and I commit it :-) | |||||
| else | else | ||||
| damageMultiplier = 0; | damageMultiplier = 0; | ||||
| } | } | ||||
| else // In case someone calls this function with an invalid shape. | else // In case someone calls this function with an invalid shape. | ||||
| { | { | ||||
| warn("The " + data.shape + " splash damage shape is not implemented!"); | warn("The " + data.shape + " splash damage shape is not implemented!"); | ||||
| } | } | ||||
| // Call CauseDamage which reduces the hitpoints, posts network command, plays sounds.... | // Call CauseDamage which reduces the hitpoints, posts network command, plays sounds.... | ||||
| ▲ Show 20 Lines • Show All 86 Lines • Show Last 20 Lines | |||||
I don't know if it's better here (for vs if).
'direction' instead of 'dir' is perhaps a better name.
(We could - to check - also just parse the 2D one)