Details
- Reviewers
wraitii Silier - Group Reviewers
Restricted Owners Package (Owns No Changed Paths) - Commits
- rP23519: Support friendly fire for projectile attacks.
Set a unit's FF boolean to true and see that as it misses a target it can damage your own units. (Easier testable with D1971.)
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- /ps/trunk
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 8396 Build 13715: Vulcan Build Jenkins Build 13714: arc lint + arc unit
Event Timeline
binaries/data/mods/public/globalscripts/Templates.js | ||
---|---|---|
205–206 | this is not true anymore, so you or have to give friendlyfire = true to every ranged attack with splash damage in template currently without friendlyFire element or keep this logic, what I do not suggest because current logic if not in template = true is at least strange | |
206 | this information is redundant and I highly suggest to remove it as it is the same as information at line 199, or if that would cause problems for dealing damage please reuse ret.attack[type].friendlyFire here |
Design-wise:
- Should FriendlyFire be a boolean of <FriendlyFire/> ? Making it a boolean seems OK to me. I'm not sure what is most consistent with the rest of the codebase
- Should it be added to the attack or to the projectile?
- Is there any case to make for an attack that would have friendly fire when hitting directly, but no friendly fire from splash damage? I think some magic/SF attacks might want something like that...
On (2), I don't think this should be added to the Ranged attack root. Rather, it should be added to the projectile - because friendly fire for non-ranged attacks makes no sense, and "ranged" attacks only real difference is the presence of a projectile. See rP20676 and rP22184. If you add it to the projectile, the splash damage definition can keep its FriendlyFire attribute, since Melee attacks dealing splash damage with or without friendly fire makes sense. In fact, the splash from ranged attack should probably be moved under projectile but that's out of scope.
binaries/data/mods/public/globalscripts/Templates.js | ||
---|---|---|
200 | If you make it a non-optional element, then you can do template.Attack[type].FriendlyFire == "false", which is nicer. | |
205–206 | We enable Friendly Fire by default - I think we should make it explicit indeed. |
That's melee splash damage - which we support perfectly, and which should be able to do friendly fire or not (think of a Sci-Fi melee sword that only kills enemies or something).
- Created FF under "Projectile".
- Updated templates accordingly.
- Refactored splash data.
- Updated tests accordingly.
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (operator-linebreak): | | '||' should be placed at the end of the line. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Templates.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Templates.js | 87| 87| // If the elements are still strings, split them by space or by '+' | 88| 88| if (typeof sublist == "string") | 89| 89| sublist = sublist.split(/[+\s]+/); | 90| |- if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) | 91| |- || (c[0] != "!" && classes.indexOf(c) != -1))) | | 90|+ if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) || | | 91|+ (c[0] != "!" && classes.indexOf(c) != -1))) | 92| 92| return true; | 93| 93| } | 94| 94| binaries/data/mods/public/globalscripts/Templates.js | 91| » » ····················||·(c[0]·!=·"!"·&&·classes.indexOf(c)·!=·-1))) | | [NORMAL] JSHintBear: | | Misleading line break before '||'; readers may interpret this as an expression boundary. | | [NORMAL] ESLintBear (spaced-comment): | | Expected space or tab after '//' in comment. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js | 526| 526| | 527| 527| let horizSpeed = +this.template[type].Projectile.Speed; | 528| 528| let gravity = +this.template[type].Projectile.Gravity; | 529| |- //horizSpeed /= 2; gravity /= 2; // slow it down for testing | | 529|+ // horizSpeed /= 2; gravity /= 2; // slow it down for testing | 530| 530| | 531| 531| let cmpPosition = Engine.QueryInterface(this.entity, IID_Position); | 532| 532| if (!cmpPosition || !cmpPosition.IsInWorld()) | | [NORMAL] ESLintBear (no-trailing-spaces): | | Trailing spaces not allowed. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js | 573| 573| // TODO: Use unit rotation to implement x/z offsets. | 574| 574| let deltaLaunchPoint = new Vector3D(0, this.template[type].Projectile.LaunchPoint["@y"], 0.0); | 575| 575| let launchPoint = Vector3D.add(selfPosition, deltaLaunchPoint); | 576| |- | | 576|+ | 577| 577| let cmpVisual = Engine.QueryInterface(this.entity, IID_Visual); | 578| 578| if (cmpVisual) | 579| 579| { | | [NORMAL] ESLintBear (curly): | | Unnecessary { after 'if' condition. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js | 610| 610| "statusEffects": this.template[type].StatusEffects | 611| 611| }; | 612| 612| if (this.template[type].Splash) | 613| |- { | | 613|+ | 614| 614| data.splash = { | 615| 615| "friendlyFire": this.template[type].Splash.FriendlyFire != "false", | 616| 616| "radius": +this.template[type].Splash.Range, | 618| 618| "strengths": this.GetAttackStrengths(type + ".Splash"), | 619| 619| "bonus": this.GetBonusTemplate(type + ".Splash") | 620| 620| }; | 621| |- } | | 621|+ | 622| 622| cmpTimer.SetTimeout(SYSTEM_ENTITY, IID_Damage, "MissileHit", timeToTarget * 1000 + +this.template[type].Delay, data); | 623| 623| } | 624| 624| else if (type == "Capture") | | [NORMAL] ESLintBear (curly): | | Unnecessary { after 'else'. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js | 647| 647| }); | 648| 648| } | 649| 649| else | 650| |- { | | 650|+ | 651| 651| // Melee attack - hurt the target immediately | 652| 652| cmpDamage.CauseDamage({ | 653| 653| "strengths": this.GetAttackStrengths(type), | 657| 657| "type": type, | 658| 658| "attackerOwner": attackerOwner | 659| 659| }); | 660| |- } | | 660|+ | 661| 661| }; | 662| 662| | 663| 663| /** binaries/data/mods/public/simulation/components/Attack.js | 516| ·» let·cmpDamage·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_Damage); | | [NORMAL] ESLintBear (no-mixed-spaces-and-tabs): | | Mixed spaces and tabs. binaries/data/mods/public/simulation/components/Attack.js | 622| » » cmpTimer.SetTimeout(SYSTEM_ENTITY,·IID_Damage,·"MissileHit",·timeToTarget·*·1000·+·+this.template[type].Delay,·data); | | [NORMAL] JSHintBear: | | Confusing plusses. | | [NORMAL] ESLintBear (quote-props): | | Unquoted property 'turnLength' found. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | 25| 25| | 26| 26| let cmpDamage = ConstructComponent(SYSTEM_ENTITY, "Damage"); | 27| 27| let cmpTimer = ConstructComponent(SYSTEM_ENTITY, "Timer"); | 28| |- cmpTimer.OnUpdate({ turnLength: 1 }); | | 28|+ cmpTimer.OnUpdate({ "turnLength": 1 }); | 29| 29| let attacker = 11; | 30| 30| let atkPlayerEntity = 1; | 31| 31| let attackerOwner = 6; | | [NORMAL] ESLintBear (comma-spacing): | | A space is required after ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | 63| 63| "attackerOwner": attackerOwner, | 64| 64| "position": targetPos, | 65| 65| "projectileId": 9, | 66| |- "direction": new Vector3D(1,0,0) | | 66|+ "direction": new Vector3D(1, 0,0) | 67| 67| }; | 68| 68| | 69| 69| AddMock(atkPlayerEntity, IID_Player, { | | [NORMAL] ESLintBear (comma-spacing): | | A space is required after ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | 63| 63| "attackerOwner": attackerOwner, | 64| 64| "position": targetPos, | 65| 65| "projectileId": 9, | 66| |- "direction": new Vector3D(1,0,0) | | 66|+ "direction": new Vector3D(1,0, 0) | 67| 67| }; | 68| 68| | 69| 69| AddMock(atkPlayerEntity, IID_Player, { | | [NORMAL] ESLintBear (quote-props): | | Unquoted property 'turnLength' found. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | 114| 114| | 115| 115| function TestDamage() | 116| 116| { | 117| |- cmpTimer.OnUpdate({ turnLength: 1 }); | | 117|+ cmpTimer.OnUpdate({ "turnLength": 1 }); | 118| 118| TS_ASSERT(damageTaken); | 119| 119| damageTaken = false; | 120| 120| } | | [NORMAL] ESLintBear (key-spacing): | | Extra space after key 'hack'. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | 156| 156| "origin": origin, | 157| 157| "radius": 10, | 158| 158| "shape": "Linear", | 159| |- "strengths": { "hack" : 100, "pierce" : 0, "crush": 0 }, | | 159|+ "strengths": { "hack": 100, "pierce" : 0, "crush": 0 }, | 160| 160| "direction": new Vector3D(1, 747, 0), | 161| 161| "playersToDamage": [2], | 162| 162| "type": "Ranged", | | [NORMAL] ESLintBear (key-spacing): | | Extra space after key 'pierce'. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | 156| 156| "origin": origin, | 157| 157| "radius": 10, | 158| 158| "shape": "Linear", | 159| |- "strengths": { "hack" : 100, "pierce" : 0, "crush": 0 }, | | 159|+ "strengths": { "hack" : 100, "pierce": 0, "crush": 0 }, | 160| 160| "direction": new Vector3D(1, 747, 0), | 161| 161| "playersToDamage": [2], | 162| 162| "type": "Ranged", | | [NORMAL] ESLintBear (comma-spacing): | | A space is required after ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | 163| 163| "attackerOwner": attackerOwner | 164| 164| }; | 165| 165| | 166| |- let fallOff = function(x,y) | | 166|+ let fallOff = function(x, y) | 167| 167| { | 168| 168| return (1 - x * x / (data.radius * data.radius)) * (1 - 25 * y * y / (data.radius * data.radius)); | 169| 169| }; | | [NORMAL] ESLintBear (key-spacing): | | Extra space after key 'hack'. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | 315| 315| "origin": new Vector2D(3, 4), | 316| 316| "radius": radius, | 317| 317| "shape": "Circular", | 318| |- "strengths": { "hack" : 100, "pierce" : 0, "crush": 0 }, | | 318|+ "strengths": { "hack": 100, "pierce" : 0, "crush": 0 }, | 319| 319| "playersToDamage": [2], | 320| 320| "type": "Ranged", | 321| 321| "attackerOwner": 1 | | [NORMAL] ESLintBear (key-spacing): | | Extra space after key 'pierce'. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | 315| 315| "origin": new Vector2D(3, 4), | 316| 316| "radius": radius, | 317| 317| "shape": "Circular", | 318| |- "strengths": { "hack" : 100, "pierce" : 0, "crush": 0 }, | | 318|+ "strengths": { "hack" : 100, "pierce": 0, "crush": 0 }, | 319| 319| "playersToDamage": [2], | 320| 320| "type": "Ranged", | 321| 321| "attackerOwner": 1 binaries/data/mods/public/simulation/components/tests/test_Damage.js | 125| » type·=·data.type·=·"Ranged"; | | [NORMAL] ESLintBear (no-multi-assign): | | Unexpected chained assignment. | | [NORMAL] ESLintBear (indent): | | Expected indentation of 2 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Damage.js | 23| 23| let cmpTimer = Engine.QueryInterface(SYSTEM_ENTITY, IID_Timer); | 24| 24| let turnLength = cmpTimer.GetLatestTurnLength(); | 25| 25| return new Vector3D( | 26| |- (curPos.x * (turnLength - lateness) + prevPos.x * lateness) / turnLength, | | 26|+ (curPos.x * (turnLength - lateness) + prevPos.x * lateness) / turnLength, | 27| 27| 0, | 28| 28| (curPos.z * (turnLength - lateness) + prevPos.z * lateness) / turnLength); | 29| 29| }; | | [NORMAL] ESLintBear (indent): | | Expected indentation of 2 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Damage.js | 24| 24| let turnLength = cmpTimer.GetLatestTurnLength(); | 25| 25| return new Vector3D( | 26| 26| (curPos.x * (turnLength - lateness) + prevPos.x * lateness) / turnLength, | 27| |- 0, | | 27|+ 0, | 28| 28| (curPos.z * (turnLength - lateness) + prevPos.z * lateness) / turnLength); | 29| 29| }; | 30| 30| | | [NORMAL] ESLintBear (indent): | | Expected indentation of 2 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Damage.js | 25| 25| return new Vector3D( | 26| 26| (curPos.x * (turnLength - lateness) + prevPos.x * lateness) / turnLength, | 27| 27| 0, | 28| |- (curPos.z * (turnLength - lateness) + prevPos.z * lateness) / turnLength); | | 28|+ (curPos.z * (turnLength - lateness) + prevPos.z * lateness) / turnLength); | 29| 29| }; | 30| 30| | 31| 31| /** | | [NORMAL] ESLintBear (curly): | | Unnecessary { after 'if' condition. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Damage.js | 113| 113| | 114| 114| // Do this first in case the direct hit kills the target | 115| 115| if ("splash" in data) | 116| |- { | | 116|+ | 117| 117| this.CauseSplashDamage({ | 118| 118| "attacker": data.attacker, | 119| 119| "origin": Vector2D.from3D(data.position), | 126| 126| "type": data.type, | 127| 127| "attackerOwner": data.attackerOwner | 128| 128| }); | 129| |- } | | 129|+ | 130| 130| | 131| 131| let cmpProjectileManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_ProjectileManager); | 132| 132| | | [NORMAL] ESLintBear (curly): | | Unnecessary { after 'else'. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Damage.js | 218| 218| damageMultiplier = 0; | 219| 219| } | 220| 220| else // In case someone calls this function with an invalid shape. | 221| |- { | | 221|+ | 222| 222| warn("The " + data.shape + " splash damage shape is not implemented!"); | 223| |- } | | 223|+ | 224| 224| | 225| 225| if (data.splashBonus) | 226| 226| damageMultiplier *= GetDamageBonus(data.attacker, ent, data.type, data.splashBonus); Executing section cli...
Link to build: https://jenkins.wildfiregames.com/job/differential/1784/display/redirect
binaries/data/mods/public/simulation/components/Attack.js | ||
---|---|---|
199 | Maybe it could be optionnal ? | |
binaries/data/mods/public/simulation/components/Damage.js | ||
113–114 | Depends whether we need to check for the key or the value, doesn't it ? In this case we are definitely checking for the value. Maybe we shouldn't do splash if radius is 0 ? | |
binaries/data/mods/public/simulation/components/tests/test_Damage.js | ||
354 | Isn't it supposed to be a "false" string ? | |
453–458 | Maybe data.splash = { "friendlyFire": "false", "radius": 10, "shape": "Circular" }; |
In the test, ExecuteQueryAroundPos is mocked and that is where, in "Damage.js", the FF is checked (EntitiesNearPoint). That might be the reason why FF is not tested for splash damage either?
binaries/data/mods/public/simulation/components/Attack.js | ||
---|---|---|
199 | Could be, but:
(See above in "Templates.js".) | |
binaries/data/mods/public/simulation/components/Damage.js | ||
113–114 | I think it would be strange to have data.splash but without a radius. Do you have a use-case? |
binaries/data/mods/public/simulation/components/Attack.js | ||
---|---|---|
199 | None of the other items are optional, so in terms of coherency it's probably better if it's not, but I agree that repeating these everywhere is kind of annoying... | |
binaries/data/mods/public/simulation/components/Damage.js | ||
113–114 | I guess with techs / auras it might sorta happen? And then possibly we want to disable splash? |
- Updated "Templates.js" because of FF under Projectile.
- Check for data.splash in "Damage.js".
- Split "Look for nearby units"-line.
- Refactored new data.splash.
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (operator-linebreak): | | '||' should be placed at the end of the line. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Templates.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Templates.js | 87| 87| // If the elements are still strings, split them by space or by '+' | 88| 88| if (typeof sublist == "string") | 89| 89| sublist = sublist.split(/[+\s]+/); | 90| |- if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) | 91| |- || (c[0] != "!" && classes.indexOf(c) != -1))) | | 90|+ if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) || | | 91|+ (c[0] != "!" && classes.indexOf(c) != -1))) | 92| 92| return true; | 93| 93| } | 94| 94| binaries/data/mods/public/globalscripts/Templates.js | 91| » » ····················||·(c[0]·!=·"!"·&&·classes.indexOf(c)·!=·-1))) | | [NORMAL] JSHintBear: | | Misleading line break before '||'; readers may interpret this as an expression boundary. | | [NORMAL] ESLintBear (quote-props): | | Unquoted property 'turnLength' found. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | 25| 25| | 26| 26| let cmpDamage = ConstructComponent(SYSTEM_ENTITY, "Damage"); | 27| 27| let cmpTimer = ConstructComponent(SYSTEM_ENTITY, "Timer"); | 28| |- cmpTimer.OnUpdate({ turnLength: 1 }); | | 28|+ cmpTimer.OnUpdate({ "turnLength": 1 }); | 29| 29| let attacker = 11; | 30| 30| let atkPlayerEntity = 1; | 31| 31| let attackerOwner = 6; | | [NORMAL] ESLintBear (comma-spacing): | | A space is required after ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | 63| 63| "attackerOwner": attackerOwner, | 64| 64| "position": targetPos, | 65| 65| "projectileId": 9, | 66| |- "direction": new Vector3D(1,0,0) | | 66|+ "direction": new Vector3D(1, 0,0) | 67| 67| }; | 68| 68| | 69| 69| AddMock(atkPlayerEntity, IID_Player, { | | [NORMAL] ESLintBear (comma-spacing): | | A space is required after ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | 63| 63| "attackerOwner": attackerOwner, | 64| 64| "position": targetPos, | 65| 65| "projectileId": 9, | 66| |- "direction": new Vector3D(1,0,0) | | 66|+ "direction": new Vector3D(1,0, 0) | 67| 67| }; | 68| 68| | 69| 69| AddMock(atkPlayerEntity, IID_Player, { | | [NORMAL] ESLintBear (quote-props): | | Unquoted property 'turnLength' found. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | 114| 114| | 115| 115| function TestDamage() | 116| 116| { | 117| |- cmpTimer.OnUpdate({ turnLength: 1 }); | | 117|+ cmpTimer.OnUpdate({ "turnLength": 1 }); | 118| 118| TS_ASSERT(damageTaken); | 119| 119| damageTaken = false; | 120| 120| } | | [NORMAL] ESLintBear (key-spacing): | | Extra space after key 'hack'. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | 156| 156| "origin": origin, | 157| 157| "radius": 10, | 158| 158| "shape": "Linear", | 159| |- "strengths": { "hack" : 100, "pierce" : 0, "crush": 0 }, | | 159|+ "strengths": { "hack": 100, "pierce" : 0, "crush": 0 }, | 160| 160| "direction": new Vector3D(1, 747, 0), | 161| 161| "playersToDamage": [2], | 162| 162| "type": "Ranged", | | [NORMAL] ESLintBear (key-spacing): | | Extra space after key 'pierce'. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | 156| 156| "origin": origin, | 157| 157| "radius": 10, | 158| 158| "shape": "Linear", | 159| |- "strengths": { "hack" : 100, "pierce" : 0, "crush": 0 }, | | 159|+ "strengths": { "hack" : 100, "pierce": 0, "crush": 0 }, | 160| 160| "direction": new Vector3D(1, 747, 0), | 161| 161| "playersToDamage": [2], | 162| 162| "type": "Ranged", | | [NORMAL] ESLintBear (comma-spacing): | | A space is required after ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | 163| 163| "attackerOwner": attackerOwner | 164| 164| }; | 165| 165| | 166| |- let fallOff = function(x,y) | | 166|+ let fallOff = function(x, y) | 167| 167| { | 168| 168| return (1 - x * x / (data.radius * data.radius)) * (1 - 25 * y * y / (data.radius * data.radius)); | 169| 169| }; | | [NORMAL] ESLintBear (key-spacing): | | Extra space after key 'hack'. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | 315| 315| "origin": new Vector2D(3, 4), | 316| 316| "radius": radius, | 317| 317| "shape": "Circular", | 318| |- "strengths": { "hack" : 100, "pierce" : 0, "crush": 0 }, | | 318|+ "strengths": { "hack": 100, "pierce" : 0, "crush": 0 }, | 319| 319| "playersToDamage": [2], | 320| 320| "type": "Ranged", | 321| 321| "attackerOwner": 1 | | [NORMAL] ESLintBear (key-spacing): | | Extra space after key 'pierce'. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Damage.js | 315| 315| "origin": new Vector2D(3, 4), | 316| 316| "radius": radius, | 317| 317| "shape": "Circular", | 318| |- "strengths": { "hack" : 100, "pierce" : 0, "crush": 0 }, | | 318|+ "strengths": { "hack" : 100, "pierce": 0, "crush": 0 }, | 319| 319| "playersToDamage": [2], | 320| 320| "type": "Ranged", | 321| 321| "attackerOwner": 1 binaries/data/mods/public/simulation/components/tests/test_Damage.js | 125| » type·=·data.type·=·"Ranged"; | | [NORMAL] ESLintBear (no-multi-assign): | | Unexpected chained assignment. | | [NORMAL] ESLintBear (spaced-comment): | | Expected space or tab after '//' in comment. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js | 527| 527| | 528| 528| let horizSpeed = +this.template[type].Projectile.Speed; | 529| 529| let gravity = +this.template[type].Projectile.Gravity; | 530| |- //horizSpeed /= 2; gravity /= 2; // slow it down for testing | | 530|+ // horizSpeed /= 2; gravity /= 2; // slow it down for testing | 531| 531| | 532| 532| let cmpPosition = Engine.QueryInterface(this.entity, IID_Position); | 533| 533| if (!cmpPosition || !cmpPosition.IsInWorld()) | | [NORMAL] ESLintBear (curly): | | Unnecessary { after 'else'. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js | 647| 647| }); | 648| 648| } | 649| 649| else | 650| |- { | | 650|+ | 651| 651| // Melee attack - hurt the target immediately | 652| 652| cmpDamage.CauseDamage({ | 653| 653| "strengths": this.GetAttackStrengths(type), | 657| 657| "type": type, | 658| 658| "attackerOwner": attackerOwner | 659| 659| }); | 660| |- } | | 660|+ | 661| 661| }; | 662| 662| | 663| 663| /** binaries/data/mods/public/simulation/components/Attack.js | 517| ·» let·cmpDamage·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_Damage); | | [NORMAL] ESLintBear (no-mixed-spaces-and-tabs): | | Mixed spaces and tabs. binaries/data/mods/public/simulation/components/Attack.js | 622| » » cmpTimer.SetTimeout(SYSTEM_ENTITY,·IID_Damage,·"MissileHit",·timeToTarget·*·1000·+·+this.template[type].Delay,·data); | | [NORMAL] JSHintBear: | | Confusing plusses. | | [NORMAL] ESLintBear (indent): | | Expected indentation of 2 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Damage.js | 23| 23| let cmpTimer = Engine.QueryInterface(SYSTEM_ENTITY, IID_Timer); | 24| 24| let turnLength = cmpTimer.GetLatestTurnLength(); | 25| 25| return new Vector3D( | 26| |- (curPos.x * (turnLength - lateness) + prevPos.x * lateness) / turnLength, | | 26|+ (curPos.x * (turnLength - lateness) + prevPos.x * lateness) / turnLength, | 27| 27| 0, | 28| 28| (curPos.z * (turnLength - lateness) + prevPos.z * lateness) / turnLength); | 29| 29| }; | | [NORMAL] ESLintBear (indent): | | Expected indentation of 2 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Damage.js | 24| 24| let turnLength = cmpTimer.GetLatestTurnLength(); | 25| 25| return new Vector3D( | 26| 26| (curPos.x * (turnLength - lateness) + prevPos.x * lateness) / turnLength, | 27| |- 0, | | 27|+ 0, | 28| 28| (curPos.z * (turnLength - lateness) + prevPos.z * lateness) / turnLength); | 29| 29| }; | 30| 30| | | [NORMAL] ESLintBear (indent): | | Expected indentation of 2 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Damage.js | 25| 25| return new Vector3D( | 26| 26| (curPos.x * (turnLength - lateness) + prevPos.x * lateness) / turnLength, | 27| 27| 0, | 28| |- (curPos.z * (turnLength - lateness) + prevPos.z * lateness) / turnLength); | | 28|+ (curPos.z * (turnLength - lateness) + prevPos.z * lateness) / turnLength); | 29| 29| }; | 30| 30| | 31| 31| /** | | [NORMAL] ESLintBear (curly): | | Unnecessary { after 'else'. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Damage.js | 217| 217| damageMultiplier = 0; | 218| 218| } | 219| 219| else // In case someone calls this function with an invalid shape. | 220| |- { | | 220|+ | 221| 221| warn("The " + data.shape + " splash damage shape is not implemented!"); | 222| |- } | | 222|+ | 223| 223| | 224| 224| if (data.splashBonus) | 225| 225| damageMultiplier *= GetDamageBonus(data.attacker, ent, data.type, data.splashBonus); Executing section cli...
Link to build: https://jenkins.wildfiregames.com/job/differential/1868/display/redirect
- Made Vulcan/Jenkins a tad happier.
- Check for this.template[type].Splash.Range > 0 in Attack.js.
binaries/data/mods/public/simulation/components/Attack.js | ||
---|---|---|
199 | Okay, a thought here. The argument to have it non-optional is coherency. I like things being optional, to keep templates as clean as possible. Would it be an idea to make more things optional? E.g.:
So just make everything that *can* be optional optional. |
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (indent): | | Expected indentation of 2 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Damage.js | 23| 23| let cmpTimer = Engine.QueryInterface(SYSTEM_ENTITY, IID_Timer); | 24| 24| let turnLength = cmpTimer.GetLatestTurnLength(); | 25| 25| return new Vector3D( | 26| |- (curPos.x * (turnLength - lateness) + prevPos.x * lateness) / turnLength, | | 26|+ (curPos.x * (turnLength - lateness) + prevPos.x * lateness) / turnLength, | 27| 27| 0, | 28| 28| (curPos.z * (turnLength - lateness) + prevPos.z * lateness) / turnLength); | 29| 29| }; | | [NORMAL] ESLintBear (indent): | | Expected indentation of 2 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Damage.js | 24| 24| let turnLength = cmpTimer.GetLatestTurnLength(); | 25| 25| return new Vector3D( | 26| 26| (curPos.x * (turnLength - lateness) + prevPos.x * lateness) / turnLength, | 27| |- 0, | | 27|+ 0, | 28| 28| (curPos.z * (turnLength - lateness) + prevPos.z * lateness) / turnLength); | 29| 29| }; | 30| 30| | | [NORMAL] ESLintBear (indent): | | Expected indentation of 2 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Damage.js | 25| 25| return new Vector3D( | 26| 26| (curPos.x * (turnLength - lateness) + prevPos.x * lateness) / turnLength, | 27| 27| 0, | 28| |- (curPos.z * (turnLength - lateness) + prevPos.z * lateness) / turnLength); | | 28|+ (curPos.z * (turnLength - lateness) + prevPos.z * lateness) / turnLength); | 29| 29| }; | 30| 30| | 31| 31| /** | | [NORMAL] ESLintBear (curly): | | Unnecessary { after 'else'. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Damage.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Damage.js | 217| 217| damageMultiplier = 0; | 218| 218| } | 219| 219| else // In case someone calls this function with an invalid shape. | 220| |- { | | 220|+ | 221| 221| warn("The " + data.shape + " splash damage shape is not implemented!"); | 222| |- } | | 222|+ | 223| 223| | 224| 224| if (data.splashBonus) | 225| 225| damageMultiplier *= GetDamageBonus(data.attacker, ent, data.type, data.splashBonus); binaries/data/mods/public/simulation/components/tests/test_Damage.js | 125| » type·=·data.type·=·"Ranged"; | | [NORMAL] ESLintBear (no-multi-assign): | | Unexpected chained assignment. | | [NORMAL] ESLintBear (spaced-comment): | | Expected space or tab after '//' in comment. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js | 527| 527| | 528| 528| let horizSpeed = +this.template[type].Projectile.Speed; | 529| 529| let gravity = +this.template[type].Projectile.Gravity; | 530| |- //horizSpeed /= 2; gravity /= 2; // slow it down for testing | | 530|+ // horizSpeed /= 2; gravity /= 2; // slow it down for testing | 531| 531| | 532| 532| let cmpPosition = Engine.QueryInterface(this.entity, IID_Position); | 533| 533| if (!cmpPosition || !cmpPosition.IsInWorld()) | | [NORMAL] ESLintBear (curly): | | Unnecessary { after 'else'. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js | 647| 647| }); | 648| 648| } | 649| 649| else | 650| |- { | | 650|+ | 651| 651| // Melee attack - hurt the target immediately | 652| 652| cmpDamage.CauseDamage({ | 653| 653| "strengths": this.GetAttackStrengths(type), | 657| 657| "type": type, | 658| 658| "attackerOwner": attackerOwner | 659| 659| }); | 660| |- } | | 660|+ | 661| 661| }; | 662| 662| | 663| 663| /** binaries/data/mods/public/simulation/components/Attack.js | 517| ·» let·cmpDamage·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_Damage); | | [NORMAL] ESLintBear (no-mixed-spaces-and-tabs): | | Mixed spaces and tabs. binaries/data/mods/public/simulation/components/Attack.js | 622| » » cmpTimer.SetTimeout(SYSTEM_ENTITY,·IID_Damage,·"MissileHit",·timeToTarget·*·1000·+·+this.template[type].Delay,·data); | | [NORMAL] JSHintBear: | | Confusing plusses. | | [NORMAL] ESLintBear (operator-linebreak): | | '||' should be placed at the end of the line. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Templates.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Templates.js | 87| 87| // If the elements are still strings, split them by space or by '+' | 88| 88| if (typeof sublist == "string") | 89| 89| sublist = sublist.split(/[+\s]+/); | 90| |- if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) | 91| |- || (c[0] != "!" && classes.indexOf(c) != -1))) | | 90|+ if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) || | | 91|+ (c[0] != "!" && classes.indexOf(c) != -1))) | 92| 92| return true; | 93| 93| } | 94| 94| binaries/data/mods/public/globalscripts/Templates.js | 91| » » ····················||·(c[0]·!=·"!"·&&·classes.indexOf(c)·!=·-1))) | | [NORMAL] JSHintBear: | | Misleading line break before '||'; readers may interpret this as an expression boundary. Executing section cli...
Link to build: https://jenkins.wildfiregames.com/job/differential/1880/display/redirect
binaries/data/mods/public/simulation/components/Attack.js | ||
---|---|---|
199 | I would say that gravity should be always stated and delay can be optional. Or friendlyfire is not optional or make it false by default so if not in template it is false. It goes against common sence. Element is called FriendlyFire so it should be false by default. It is not called DisableFriendylFire. Personally I think if there is good template inheritance for units, a lot of things does not have to be repeated. Problem can be with structures. |
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (indent): | | Expected indentation of 2 tabs but found 3. |----| | /zpool0/trunk/binaries/data/mods/public/simulation/components/Damage.js | |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Damage.js | 23| 23| let cmpTimer = Engine.QueryInterface(SYSTEM_ENTITY, IID_Timer); | 24| 24| let turnLength = cmpTimer.GetLatestTurnLength(); | 25| 25| return new Vector3D( | 26| |- (curPos.x * (turnLength - lateness) + prevPos.x * lateness) / turnLength, | | 26|+ (curPos.x * (turnLength - lateness) + prevPos.x * lateness) / turnLength, | 27| 27| 0, | 28| 28| (curPos.z * (turnLength - lateness) + prevPos.z * lateness) / turnLength); | 29| 29| }; | | [NORMAL] ESLintBear (indent): | | Expected indentation of 2 tabs but found 3. |----| | /zpool0/trunk/binaries/data/mods/public/simulation/components/Damage.js | |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Damage.js | 24| 24| let turnLength = cmpTimer.GetLatestTurnLength(); | 25| 25| return new Vector3D( | 26| 26| (curPos.x * (turnLength - lateness) + prevPos.x * lateness) / turnLength, | 27| |- 0, | | 27|+ 0, | 28| 28| (curPos.z * (turnLength - lateness) + prevPos.z * lateness) / turnLength); | 29| 29| }; | 30| 30| | | [NORMAL] ESLintBear (indent): | | Expected indentation of 2 tabs but found 3. |----| | /zpool0/trunk/binaries/data/mods/public/simulation/components/Damage.js | |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Damage.js | 25| 25| return new Vector3D( | 26| 26| (curPos.x * (turnLength - lateness) + prevPos.x * lateness) / turnLength, | 27| 27| 0, | 28| |- (curPos.z * (turnLength - lateness) + prevPos.z * lateness) / turnLength); | | 28|+ (curPos.z * (turnLength - lateness) + prevPos.z * lateness) / turnLength); | 29| 29| }; | 30| 30| | 31| 31| /** | | [NORMAL] ESLintBear (curly): | | Unnecessary { after 'else'. |----| | /zpool0/trunk/binaries/data/mods/public/simulation/components/Damage.js | |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Damage.js | 216| 216| damageMultiplier = 0; | 217| 217| } | 218| 218| else // In case someone calls this function with an invalid shape. | 219| |- { | | 219|+ | 220| 220| warn("The " + data.shape + " splash damage shape is not implemented!"); | 221| |- } | | 221|+ | 222| 222| | 223| 223| if (data.splashBonus) | 224| 224| damageMultiplier *= GetDamageBonus(data.attacker, ent, data.type, data.splashBonus); binaries/data/mods/public/simulation/components/tests/test_Damage.js | 125| » type·=·data.type·=·"Ranged"; | | [NORMAL] ESLintBear (no-multi-assign): | | Unexpected chained assignment. | | [NORMAL] ESLintBear (operator-linebreak): | | '||' should be placed at the end of the line. |----| | /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js | |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js | 87| 87| // If the elements are still strings, split them by space or by '+' | 88| 88| if (typeof sublist == "string") | 89| 89| sublist = sublist.split(/[+\s]+/); | 90| |- if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) | 91| |- || (c[0] != "!" && classes.indexOf(c) != -1))) | | 90|+ if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) || | | 91|+ (c[0] != "!" && classes.indexOf(c) != -1))) | 92| 92| return true; | 93| 93| } | 94| 94| binaries/data/mods/public/globalscripts/Templates.js | 91| » » ····················||·(c[0]·!=·"!"·&&·classes.indexOf(c)·!=·-1))) | | [NORMAL] JSHintBear: | | Misleading line break before '||'; readers may interpret this as an expression boundary. | | [NORMAL] ESLintBear (spaced-comment): | | Expected space or tab after '//' in comment. |----| | /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js | |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js | 535| 535| | 536| 536| let horizSpeed = +this.template[type].Projectile.Speed; | 537| 537| let gravity = +this.template[type].Projectile.Gravity; | 538| |- //horizSpeed /= 2; gravity /= 2; // slow it down for testing | | 538|+ // horizSpeed /= 2; gravity /= 2; // slow it down for testing | 539| 539| | 540| 540| let cmpPosition = Engine.QueryInterface(this.entity, IID_Position); | 541| 541| if (!cmpPosition || !cmpPosition.IsInWorld()) | | [NORMAL] ESLintBear (curly): | | Unnecessary { after 'else'. |----| | /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js | |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js | 655| 655| }); | 656| 656| } | 657| 657| else | 658| |- { | | 658|+ | 659| 659| // Melee attack - hurt the target immediately | 660| 660| cmpDamage.CauseDamage({ | 661| 661| "strengths": this.GetAttackStrengths(type), | 665| 665| "type": type, | 666| 666| "attackerOwner": attackerOwner | 667| 667| }); | 668| |- } | | 668|+ | 669| 669| }; | 670| 670| | 671| 671| /** binaries/data/mods/public/simulation/components/Attack.js | 525| ·» let·cmpDamage·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_Damage); | | [NORMAL] ESLintBear (no-mixed-spaces-and-tabs): | | Mixed spaces and tabs. binaries/data/mods/public/simulation/components/Attack.js | 630| » » cmpTimer.SetTimeout(SYSTEM_ENTITY,·IID_Damage,·"MissileHit",·timeToTarget·*·1000·+·+this.template[type].Delay,·data); | | [NORMAL] JSHintBear: | | Confusing plusses. Executing section cli...
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/97/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (operator-linebreak): | | '||' should be placed at the end of the line. |----| | /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js | |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js | 87| 87| // If the elements are still strings, split them by space or by '+' | 88| 88| if (typeof sublist == "string") | 89| 89| sublist = sublist.split(/[+\s]+/); | 90| |- if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) | 91| |- || (c[0] != "!" && classes.indexOf(c) != -1))) | | 90|+ if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) || | | 91|+ (c[0] != "!" && classes.indexOf(c) != -1))) | 92| 92| return true; | 93| 93| } | 94| 94| binaries/data/mods/public/globalscripts/Templates.js | 91| » » ····················||·(c[0]·!=·"!"·&&·classes.indexOf(c)·!=·-1))) | | [NORMAL] JSHintBear: | | Misleading line break before '||'; readers may interpret this as an expression boundary. Executing section cli...
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/473/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/127/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (operator-linebreak): | | '||' should be placed at the end of the line. |----| | /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js | |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js | 87| 87| // If the elements are still strings, split them by space or by '+' | 88| 88| if (typeof sublist == "string") | 89| 89| sublist = sublist.split(/[+\s]+/); | 90| |- if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) | 91| |- || (c[0] != "!" && classes.indexOf(c) != -1))) | | 90|+ if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) || | | 91|+ (c[0] != "!" && classes.indexOf(c) != -1))) | 92| 92| return true; | 93| 93| } | 94| 94| binaries/data/mods/public/globalscripts/Templates.js | 91| » » ····················||·(c[0]·!=·"!"·&&·classes.indexOf(c)·!=·-1))) | | [NORMAL] JSHintBear: | | Misleading line break before '||'; readers may interpret this as an expression boundary. Executing section cli...
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/636/display/redirect
binaries/data/mods/public/simulation/components/Attack.js | ||
---|---|---|
620–621 | could change that one as well. | |
binaries/data/mods/public/simulation/components/tests/test_Attack.js | ||
82 | Shouldn't that be text, as it would be in the template ? | |
binaries/data/mods/public/simulation/components/tests/test_Damage.js | ||
42 | Shouldn't that be text, as it would be in the template ? |
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/347/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (operator-linebreak): | | '||' should be placed at the end of the line. |----| | /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js | |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js | 87| 87| // If the elements are still strings, split them by space or by '+' | 88| 88| if (typeof sublist == "string") | 89| 89| sublist = sublist.split(/[+\s]+/); | 90| |- if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) | 91| |- || (c[0] != "!" && classes.indexOf(c) != -1))) | | 90|+ if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) || | | 91|+ (c[0] != "!" && classes.indexOf(c) != -1))) | 92| 92| return true; | 93| 93| } | 94| 94| binaries/data/mods/public/globalscripts/Templates.js | 91| » » ····················||·(c[0]·!=·"!"·&&·classes.indexOf(c)·!=·-1))) | | [NORMAL] JSHintBear: | | Misleading line break before '||'; readers may interpret this as an expression boundary. Executing section cli...
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/863/display/redirect
quick look, looks good.
critical look comes later,
you should add tests for friendlyfire = true.
testing just one side of coin is bad practice
- Rebased.
- Add test for FF == true for splash.
A test for FF == true for normal ranged attacks could be added.
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (operator-linebreak): | | '||' should be placed at the end of the line. |----| | /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js | |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js | 87| 87| // If the elements are still strings, split them by space or by '+' | 88| 88| if (typeof sublist == "string") | 89| 89| sublist = sublist.split(/[+\s]+/); | 90| |- if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) | 91| |- || (c[0] != "!" && classes.indexOf(c) != -1))) | | 90|+ if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) || | | 91|+ (c[0] != "!" && classes.indexOf(c) != -1))) | 92| 92| return true; | 93| 93| } | 94| 94| binaries/data/mods/public/globalscripts/Templates.js | 91| » » ····················||·(c[0]·!=·"!"·&&·classes.indexOf(c)·!=·-1))) | | [NORMAL] JSHintBear: | | Misleading line break before '||'; readers may interpret this as an expression boundary. binaries/data/mods/public/simulation/components/Attack.js | 422| » return·{ | | [NORMAL] ESLintBear (consistent-return): | | Function expected no return value. Executing section cli...
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1797/display/redirect
binaries/data/mods/public/simulation/components/DelayedDamage.js | ||
---|---|---|
72 ↗ | (On Diff #11405) | could you please to break this to multiple lines ? |
Build failure - The Moirai have given mortals hearts that can endure.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1816/display/redirect
It is correct to allow templates decide if friendly fire is enabled instead hardcoding it.
Changes to code are complete and correct.
All required templates have been updated.
Test are passing.