Page MenuHomeWildfire Games

UnitAI: Fix ESLint coding style warnings
Needs ReviewPublic

Authored by Krinkle on Sep 10 2019, 3:38 AM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Trac Tickets
#5524
Summary

This commit fixes the warnings from ESLint that are about coding style. These are the rules that can be satisfied without changing code semantics or run-time behaviour (and automatically fixable via eslint --fix …).

Before
/0ad-git/binaries/data/mods/public/simulation/components/UnitAI.js
   338:4    warning  Method 'Order.WalkToTarget' expected no return value           consistent-return
   792:5    warning  Unnecessary 'else' after 'return'                              no-else-return
  1063:14   warning  Missing space before value for key 'GARRISON'                  key-spacing
  1248:5    warning  Method 'Timer' expected no return value                        consistent-return
  2042:9    warning  '&&' should be placed at the end of the line                   operator-linebreak
  2780:41   warning  A space is required after '{'                                  object-curly-spacing
  2780:101  warning  A space is required before '}'                                 object-curly-spacing
  3078:62   warning  Missing semicolon                                              semi
  3304:1    warning  Expected indentation of 2 tabs but found 7                     indent
  3366:43   warning  Unnecessary use of boolean literals in conditional expression  no-unneeded-ternary
  3482:72   warning  A space is required after ','                                  comma-spacing
  3538:38   warning  A space is required after '{'                                  object-curly-spacing
  3538:76   warning  A space is required before '}'                                 object-curly-spacing
  3625:1    warning  Expected space or tab after '//' in comment                    spaced-comment
  3796:33   warning  A space is required after '{'                                  object-curly-spacing
  3796:60   warning  A space is required before '}'                                 object-curly-spacing
  3940:19   warning  'type' is already declared in the upper scope                  no-shadow
  4038:36   warning  A space is required after '{'                                  object-curly-spacing
  4038:88   warning  A space is required before '}'                                 object-curly-spacing
  4083:36   warning  A space is required after '{'                                  object-curly-spacing
  4083:80   warning  A space is required before '}'                                 object-curly-spacing
  4114:36   warning  A space is required after '{'                                  object-curly-spacing
  4114:68   warning  A space is required before '}'                                 object-curly-spacing
  4119:36   warning  A space is required after '{'                                  object-curly-spacing
  4119:80   warning  A space is required before '}'                                 object-curly-spacing
  4124:36   warning  A space is required after '{'                                  object-curly-spacing
  4124:92   warning  A space is required before '}'                                 object-curly-spacing
  4130:37   warning  A space is required after '{'                                  object-curly-spacing
  4130:75   warning  A space is required before '}'                                 object-curly-spacing
  4132:37   warning  A space is required after '{'                                  object-curly-spacing
  4132:79   warning  A space is required before '}'                                 object-curly-spacing
  4137:36   warning  A space is required after '{'                                  object-curly-spacing
  4137:81   warning  A space is required before '}'                                 object-curly-spacing
  4140:1    warning  Expected space or tab after '//' in comment                    spaced-comment
  4790:25   warning  'target' is already declared in the upper scope                no-shadow
  4805:25   warning  'target' is already declared in the upper scope                no-shadow
  4807:5    warning  '&&' should be placed at the end of the line                   operator-linebreak
  4808:5    warning  '&&' should be placed at the end of the line                   operator-linebreak
  4851:22   warning  'ent' is already declared in the upper scope                   no-shadow
  4871:18   warning  Multiple spaces found before 'Engine'                          no-multi-spaces
  4874:39   warning  'type' is already declared in the upper scope                  no-shadow
  4875:1    warning  Expected indentation of 3 tabs but found 4                     indent
  4913:18   warning  Multiple spaces found before 'Engine'                          no-multi-spaces
  4926:1    warning  Expected space or tab after '//' in comment                    spaced-comment
  5083:4    warning  Unnecessary 'else' after 'return'                              no-else-return
  5424:36   warning  Trailing spaces not allowed                                    no-trailing-spaces
  5706:9    warning  '&&' should be placed at the end of the line                   operator-linebreak
  5709:9    warning  '&&' should be placed at the end of the line                   operator-linebreak
  5732:7    warning  '&&' should be placed at the end of the line                   operator-linebreak
  5735:7    warning  '&&' should be placed at the end of the line                   operator-linebreak
  5877:22   warning  A space is required after '{'                                  object-curly-spacing
  5877:37   warning  A space is required before '}'                                 object-curly-spacing
  5904:1    warning  Expected space or tab after '//' in comment                    spaced-comment
  5923:2    warning  Missing semicolon                                              semi
  6121:1    warning  Expected space or tab after '//' in comment                    spaced-comment
  6127:5    warning  '&&' should be placed at the end of the line                   operator-linebreak
  6130:1    warning  Expected space or tab after '//' in comment                    spaced-comment
After
✖ 7 problems (0 errors, 7 warnings)

/0ad-git/binaries/data/mods/public/simulation/components/UnitAI.js
   338:4   warning  Method 'Order.WalkToTarget' expected no return value  consistent-return
  1259:5   warning  Method 'Timer' expected no return value               consistent-return
  3960:19  warning  'type' is already declared in the upper scope         no-shadow
  4810:25  warning  'target' is already declared in the upper scope       no-shadow
  4825:25  warning  'target' is already declared in the upper scope       no-shadow
  4871:22  warning  'ent' is already declared in the upper scope          no-shadow
  4894:39  warning  'type' is already declared in the upper scope         no-shadow

The remaining warnings require changes to lint config and/or (minor) code semantics to satisfy, which carries more risk, and needs more careful testing, which I'll do in a separate commit for reviewer convenience.

Test Plan

Slightly less unhappy Coala bears.

Event Timeline

Krinkle created this revision.Sep 10 2019, 3:38 AM
Krinkle edited the summary of this revision. (Show Details)

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/122/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/components/UnitAI.js
| 331| »   »   »   return·true;
|    | [NORMAL] ESLintBear (consistent-return):
|    | Method 'Order.WalkToTarget' expected no return value.

binaries/data/mods/public/simulation/components/UnitAI.js
| 916| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
| 941| »   »   »   "enter":·function(msg)·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
| 991| »   »   »   »   »   return·true;
|    | [NORMAL] ESLintBear (consistent-return):
|    | Method 'enter' expected no return value.

binaries/data/mods/public/simulation/components/UnitAI.js
|1054| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1090| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1122| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1282| »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1339| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1514| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1536| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1568| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1722| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1772| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1850| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2027| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2143| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2418| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2451| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2557| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2623| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2662| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2873| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|3054| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|3781| »   var·isWorkType·=·type·=>·type·==·"Gather"·||·type·==·"Trade"·||·type·==·"Repair"·||·type·==·"ReturnResource";
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'type' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4628| »   var·target·=·ents.find(target·=>·this.CanAttack(target));
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'target' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4643| »   var·target·=·ents.find(target·=>
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'target' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4689| »   var·ent·=·ents.find(ent·=>·this.CanHeal(ent));
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'ent' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4712| »   »   ····cmpAttack.GetAttackTypes().some(type·=>·cmpUnitAI.CheckTargetAttackRange(this.isGuardOf,·type)))
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'type' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|3743| »   »   var·order·=·{·"type":·type,·"data":·data·};
|    | [NORMAL] JSHintBear:
|    | 'order' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|3812| »   for·(var·i·=·0;·i·<·this.orderQueue.length;·++i)
|    | [NORMAL] JSHintBear:
|    | 'i' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5549| »   var·targets·=·this.GetTargetsFromUnit();
|    | [NORMAL] JSHintBear:
|    | 'targets' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5550| »   for·(var·targ·of·targets)
|    | [NORMAL] JSHintBear:
|    | 'targ' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5556| »   »   »   var·cmpIdentity·=·Engine.QueryInterface(targ,·IID_Identity);
|    | [NORMAL] JSHintBear:
|    | 'cmpIdentity' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5557| »   »   »   var·targetClasses·=·this.order.data.targetClasses;
|    | [NORMAL] JSHintBear:
|    | 'targetClasses' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5637| »   »   var·cmpVision·=·Engine.QueryInterface(this.entity,·IID_Vision);
|    | [NORMAL] JSHintBear:
|    | 'cmpVision' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5640| »   »   var·range·=·cmpVision.GetRange();
|    | [NORMAL] JSHintBear:
|    | 'range' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5645| »   »   var·cmpRanged·=·Engine.QueryInterface(this.entity,·iid);
|    | [NORMAL] JSHintBear:
|    | 'cmpRanged' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5648| »   »   var·range·=·iid·!==·IID_Attack·?·cmpRanged.GetRange()·:·cmpRanged.GetFullAttackRange();
|    | [NORMAL] JSHintBear:
|    | 'range' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5649| »   »   var·cmpVision·=·Engine.QueryInterface(this.entity,·IID_Vision);
|    | [NORMAL] JSHintBear:
|    | 'cmpVision' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5659| »   »   var·cmpVision·=·Engine.QueryInterface(this.entity,·IID_Vision);
|    | [NORMAL] JSHintBear:
|    | 'cmpVision' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5662| »   »   var·range·=·cmpVision.GetRange();
|    | [NORMAL] JSHintBear:
|    | 'range' is already defined.
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/631/display/redirect

elexis added a subscriber: elexis.Sep 12 2019, 11:59 PM
elexis added inline comments.
binaries/data/mods/public/simulation/components/UnitAI.js
800

Duplication should be avoided. I guess the return is not worth to speak of.
It could become if (x) doY(); else doZ(); return;, on the other hand someone could say that its just a coincidence that theres a return in both cases.
Diff is ok.

2055

yess about the &&

We had an unwritten law (as in someone had a strong opinion) on whitespace for the secnod line of if statements:

\tif (x &&
\t    z)

That's same amount of tabs and 4 spaces (i.e. same character count as if ().

I doubt one can make an ESlint rule for that.

I guess I dont care if I commit the tab or 4 spaces.

Aside, I would recommend to put every of these conjunctions (https://en.wikipedia.org/wiki/Conjunctive_normal_form) on a separate line, so it's easier to view the structure:

		if (this.order.data.attackType == "Capture" &&
			(this.GetStance().targetAttackersAlways || !this.order.data.force) &&
			this.order.data.target != msg.data.attacker &&
			this.GetBestAttackAgainst(msg.data.attacker, true) != "Capture")
3381

Unneeded parentheses (no ESlint warning about that by any chance?)

It warns about ? true : false? Nice.

I remember someone not liking the !! way (I liked it because it made the code much shorter in some places), perhaps one can use != undefined, but then one would have to make sure that only falsy values equal to undefined are passed (which probably is the case but I wouldnt want to rely on assumptions) , whatever

3642

Wheres that hash coming from? "Markdown-esque", I see.
Well, coding conventions say something wise - to do things consistenly in the file, then consitently in the folder, then consistently globally.
I never saw this before, so I'd recommend a simple // Foo.

Krinkle marked an inline comment as done.Sep 13 2019, 12:08 AM
Krinkle added inline comments.
binaries/data/mods/public/simulation/components/UnitAI.js
3381

I'll remove the parenthesis for now.

As follow-up later:

  1. Look into a rule for redundant parenthesis.
  2. Analyse this property across the code base and see if we can turn it into a proper boolean, or some other strict subset of values that can be validated without needing to rely on type casting (e.g. only undefined or truthy value).
3642

Sounds good to me.

Krinkle planned changes to this revision.Sep 13 2019, 12:08 AM
Krinkle updated this revision to Diff 9749.Sep 14 2019, 6:49 AM

Removed redundant parenthesis.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/158/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/components/UnitAI.js
| 331| »   »   »   return·true;
|    | [NORMAL] ESLintBear (consistent-return):
|    | Method 'Order.WalkToTarget' expected no return value.

binaries/data/mods/public/simulation/components/UnitAI.js
| 916| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
| 941| »   »   »   "enter":·function(msg)·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
| 991| »   »   »   »   »   return·true;
|    | [NORMAL] ESLintBear (consistent-return):
|    | Method 'enter' expected no return value.

binaries/data/mods/public/simulation/components/UnitAI.js
|1054| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1090| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1122| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1282| »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1339| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1514| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1536| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1568| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1722| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1772| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1850| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2027| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2143| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2418| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2451| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2557| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2623| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2662| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2873| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|3054| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|3781| »   var·isWorkType·=·type·=>·type·==·"Gather"·||·type·==·"Trade"·||·type·==·"Repair"·||·type·==·"ReturnResource";
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'type' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4628| »   var·target·=·ents.find(target·=>·this.CanAttack(target));
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'target' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4643| »   var·target·=·ents.find(target·=>
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'target' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4689| »   var·ent·=·ents.find(ent·=>·this.CanHeal(ent));
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'ent' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4712| »   »   ····cmpAttack.GetAttackTypes().some(type·=>·cmpUnitAI.CheckTargetAttackRange(this.isGuardOf,·type)))
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'type' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|3743| »   »   var·order·=·{·"type":·type,·"data":·data·};
|    | [NORMAL] JSHintBear:
|    | 'order' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|3812| »   for·(var·i·=·0;·i·<·this.orderQueue.length;·++i)
|    | [NORMAL] JSHintBear:
|    | 'i' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5549| »   var·targets·=·this.GetTargetsFromUnit();
|    | [NORMAL] JSHintBear:
|    | 'targets' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5550| »   for·(var·targ·of·targets)
|    | [NORMAL] JSHintBear:
|    | 'targ' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5556| »   »   »   var·cmpIdentity·=·Engine.QueryInterface(targ,·IID_Identity);
|    | [NORMAL] JSHintBear:
|    | 'cmpIdentity' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5557| »   »   »   var·targetClasses·=·this.order.data.targetClasses;
|    | [NORMAL] JSHintBear:
|    | 'targetClasses' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5637| »   »   var·cmpVision·=·Engine.QueryInterface(this.entity,·IID_Vision);
|    | [NORMAL] JSHintBear:
|    | 'cmpVision' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5640| »   »   var·range·=·cmpVision.GetRange();
|    | [NORMAL] JSHintBear:
|    | 'range' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5645| »   »   var·cmpRanged·=·Engine.QueryInterface(this.entity,·iid);
|    | [NORMAL] JSHintBear:
|    | 'cmpRanged' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5648| »   »   var·range·=·iid·!==·IID_Attack·?·cmpRanged.GetRange()·:·cmpRanged.GetFullAttackRange();
|    | [NORMAL] JSHintBear:
|    | 'range' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5649| »   »   var·cmpVision·=·Engine.QueryInterface(this.entity,·IID_Vision);
|    | [NORMAL] JSHintBear:
|    | 'cmpVision' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5659| »   »   var·cmpVision·=·Engine.QueryInterface(this.entity,·IID_Vision);
|    | [NORMAL] JSHintBear:
|    | 'cmpVision' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5662| »   »   var·range·=·cmpVision.GetRange();
|    | [NORMAL] JSHintBear:
|    | 'range' is already defined.
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/668/display/redirect

Krinkle marked an inline comment as done.Sep 14 2019, 6:57 AM
Krinkle added inline comments.
binaries/data/mods/public/simulation/components/UnitAI.js
2055

(Done)

On a related note, I think this makes a good case for always including curly braces.

In the following it is not obvious at first that the last line is not part of the conditional.


"Attacked": function(msg) {
	// If we are capturing and are attacked by something that we would not capture, attack that entity instead
	if (this.order.data.attackType == "Capture" &&
		(this.GetStance().targetAttackersAlways || !this.order.data.force) &&
		this.order.data.target != msg.data.attacker &&
		this.GetBestAttackAgainst(msg.data.attacker, true) != "Capture")
		this.RespondToTargetedEntities([msg.data.attacker]);
	},
Krinkle updated this revision to Diff 9750.Sep 14 2019, 6:58 AM
Krinkle marked an inline comment as done.

Added line breaks in a multi-line conditional.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/159/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/components/UnitAI.js
| 331| »   »   »   return·true;
|    | [NORMAL] ESLintBear (consistent-return):
|    | Method 'Order.WalkToTarget' expected no return value.

binaries/data/mods/public/simulation/components/UnitAI.js
| 916| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
| 941| »   »   »   "enter":·function(msg)·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
| 991| »   »   »   »   »   return·true;
|    | [NORMAL] ESLintBear (consistent-return):
|    | Method 'enter' expected no return value.

binaries/data/mods/public/simulation/components/UnitAI.js
|1054| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1090| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1122| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1282| »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1339| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1514| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1536| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1568| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1722| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1772| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1850| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2029| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2145| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2420| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2453| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2559| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2625| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2664| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2875| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|3056| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|3783| »   var·isWorkType·=·type·=>·type·==·"Gather"·||·type·==·"Trade"·||·type·==·"Repair"·||·type·==·"ReturnResource";
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'type' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4630| »   var·target·=·ents.find(target·=>·this.CanAttack(target));
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'target' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4645| »   var·target·=·ents.find(target·=>
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'target' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4691| »   var·ent·=·ents.find(ent·=>·this.CanHeal(ent));
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'ent' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4714| »   »   ····cmpAttack.GetAttackTypes().some(type·=>·cmpUnitAI.CheckTargetAttackRange(this.isGuardOf,·type)))
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'type' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|3745| »   »   var·order·=·{·"type":·type,·"data":·data·};
|    | [NORMAL] JSHintBear:
|    | 'order' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|3814| »   for·(var·i·=·0;·i·<·this.orderQueue.length;·++i)
|    | [NORMAL] JSHintBear:
|    | 'i' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5551| »   var·targets·=·this.GetTargetsFromUnit();
|    | [NORMAL] JSHintBear:
|    | 'targets' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5552| »   for·(var·targ·of·targets)
|    | [NORMAL] JSHintBear:
|    | 'targ' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5558| »   »   »   var·cmpIdentity·=·Engine.QueryInterface(targ,·IID_Identity);
|    | [NORMAL] JSHintBear:
|    | 'cmpIdentity' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5559| »   »   »   var·targetClasses·=·this.order.data.targetClasses;
|    | [NORMAL] JSHintBear:
|    | 'targetClasses' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5639| »   »   var·cmpVision·=·Engine.QueryInterface(this.entity,·IID_Vision);
|    | [NORMAL] JSHintBear:
|    | 'cmpVision' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5642| »   »   var·range·=·cmpVision.GetRange();
|    | [NORMAL] JSHintBear:
|    | 'range' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5647| »   »   var·cmpRanged·=·Engine.QueryInterface(this.entity,·iid);
|    | [NORMAL] JSHintBear:
|    | 'cmpRanged' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5650| »   »   var·range·=·iid·!==·IID_Attack·?·cmpRanged.GetRange()·:·cmpRanged.GetFullAttackRange();
|    | [NORMAL] JSHintBear:
|    | 'range' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5651| »   »   var·cmpVision·=·Engine.QueryInterface(this.entity,·IID_Vision);
|    | [NORMAL] JSHintBear:
|    | 'cmpVision' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5661| »   »   var·cmpVision·=·Engine.QueryInterface(this.entity,·IID_Vision);
|    | [NORMAL] JSHintBear:
|    | 'cmpVision' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5664| »   »   var·range·=·cmpVision.GetRange();
|    | [NORMAL] JSHintBear:
|    | 'range' is already defined.
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/669/display/redirect

Stan added a subscriber: Stan.Sep 14 2019, 12:12 PM
Stan added inline comments.
binaries/data/mods/public/simulation/components/UnitAI.js
3381

Natural behaviour is an optionnal a choice tag.

Which means it can either be present or not

<UnitAI>
  <NaturalBehaviour/>
</UnitAI>

Here


this.template.NaturalBehaviour === undefined //true

"NaturalBehaviour" in this.template == true // true

I think there is no check for whether text in there is correct.

<UnitAI>
   <NaturalBehaviour>defensive<NaturalBehaviour/>
</UnitAI>

this.template.NaturalBehaviour === undefined //false

this.template.NaturalBehaviour.length === "defensive".length // true

"NaturalBehaviour" in this.template == true // true
elexis added inline comments.Sep 14 2019, 2:21 PM
binaries/data/mods/public/simulation/components/UnitAI.js
2055

I suppose anyone who really wants to learn the content of the line will discover it very quickly, since every if statement has a consequent, so it can only be the last one.

For these cases it certainly isnt controversial to add the curly braces.
But that example doesnt compare to the many conditions that have only one or two lines.

2791

(I'm a fan of having each property on a separate line if it's more than one property, so that one can see all property names without having to read the values for instance.)

3381

!== undefined might more reflect the intended meaning.

We can make sure that its never false, null or 0.
Its defined in
m_ScriptInterface.SetProperty(m_Instance, "template", paramNode, true, false);
ending up in
ScriptInterface::ToJSVal<CParamNode>
then
CParamNode::ToJSVal
then
CParamNode::ConstructJSVal.

So as far as I see the object is a string or undefined, but never 0, null, or false.

Add to that the circumstancial evidence of all those .template == "false" and related boolean checks, for example
this.template.Unhealable == "true"
and the number conversions with + such as:
halfSize = Math.max(+template.get("Footprint/Square/@depth"), +template.get("Footprint/Square/@width")) / 2;
and undefined checks such as
if (this.template.Distance.MinDistance !== undefined)
So it would be even more consistent to use the undefined check isntead of !!, at least for simulation templates.

3642

ping

4058

(same)

4103

(same)

5896

(I suppose this should be a vector some time)

Stan added inline comments.Sep 14 2019, 6:54 PM
binaries/data/mods/public/simulation/components/UnitAI.js
5896

Would that be a 3D one with a useless y component or a 2D with y used as z ?

Krinkle marked an inline comment as done.Sep 14 2019, 7:02 PM
Krinkle added inline comments.
binaries/data/mods/public/simulation/components/UnitAI.js
3381

Those are nominal changes. I'm quite interested in making the XML to JS translation of some of these values more strict and easier to use (in particular around boolean values), but would be okay to handle that separately?

Krinkle marked 5 inline comments as done.Sep 14 2019, 7:57 PM
Krinkle added inline comments.
binaries/data/mods/public/simulation/components/UnitAI.js
2791

Done. I changed it for this line, and also for a few other ones in the same file where an object used more than ~ 100 chars and had multiple keys.

Krinkle updated this revision to Diff 9766.Sep 14 2019, 7:57 PM
Krinkle marked an inline comment as done.
Krinkle marked an inline comment as done.

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/688/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/173/display/redirect

elexis added inline comments.Sep 20 2019, 6:43 PM
binaries/data/mods/public/simulation/components/UnitAI.js
2055

Does the linter allow us to make the last tab for those 3 lines 4 spaces?

3381

Using` !== undefined` instead of !! in this line would be more consistent with the rest of the template parsing, no?

4829

That ); on a separate line looks wrong to me, doesn't it for you?
I understand why scopes, objects and arrays are on a separate line, but not these ones?
(I suppose there is no linter rule for that?)

Krinkle added inline comments.Sep 22 2019, 10:04 PM
binaries/data/mods/public/simulation/components/UnitAI.js
2055

No, it does not. Outside strings and comments, the only place where spaces are allowed for indentation is for subsequent lines of a block comment:

\t /**
\t _ * …
\t _ * …
\t _ */

I do personally dislike that the conditional body and conditions align directly. There's a number of different styles to avoid that, though. The easiest one would be to insert the braces which helps set the two apart.

Before
	"Attacked": function(msg) {
		if (this.order.data.attackType == "Capture" &&
			(this.GetStance()..force) &&
			this.order.data.target != msg.data.attacker &&
			this.GetBestAttackAgainst(msg,) != "Capture")
			this.RespondToTargetedEntities([msg.data.attacker]);
	},
After
	"Attacked": function(msg) {
		if (this.order.data.attackType == "Capture" &&
			(this.GetStance()..force) &&
			this.order.data.target != msg.data.attacker &&
			this.GetBestAttackAgainst(msg.) != "Capture")
		{
			this.RespondToTargetedEntities([msg.data.attacker]);
		}
	},
3381

It casts to a boolean on this line in trunk, and this patch currently does not change that.

Is it okay if I audit those in a separate patch so that we can isolate those and the testing required for them?

4829

I generally follow a rule that closing symbols must align in indentation with opening symbols. This makes it possible to read large amounts of code faster and/or with greater accuracy due to there being less visual ambiguity.

In this case these three lines together make up the body of the lambda passed to Array#find.

Removing the line break would make it hard to distinguish whether between find() && … ( || ) and find( => … && … ( || ) ).

The idea of aligning with the opening symbol was already present here before my change, and indeed to my knowledge there is no rule available to require or disallow that. It's a judgement call.

elexis added inline comments.Sep 25 2019, 12:38 PM
binaries/data/mods/public/simulation/components/UnitAI.js
2055

It's more that the conditions of the if align with the if.

I suppose the linter policy derives from our coding conventions, and the code derives from our coding conventions, not the coding conventions from the linter.

This was objected in several instances before, http://irclogs.wildfiregames.com/

2016/2016-01/2016-01-02-QuakeNet-#0ad-dev.log:17:47 <@leper> use spaces to align the condition for that while if you have to
2016/2016-01/2016-01-19-QuakeNet-#0ad-dev.log:19:06 <@leper> elexis: one tab and align with spaces
2016/2016-03/2016-03-15-QuakeNet-#0ad-dev.log:18:03 <@leper> 1 tab, spaces to align
2016/2016-04/2016-04-12-QuakeNet-#0ad-dev.log:15:50 <@leper> elexis: the second line of the if should use spaces to align with the first line
2016/2016-05/2016-05-17-QuakeNet-#0ad-dev.log:20:49 < elexis> leper: there are so many spaces that the second line of the if statement has more indentation than the return, so it doesnt look like its aligned

Ive discussed it with Philip in 2016 too and then became convinced that it's okay to align the conditions of the if statement with spaces.
So I don't really see why I'd change my mind just because some linter software doesn't support this.

If we decide to drop that style, then we can probably identify all cases and change all cases in one step.

I find 319 matches in JS files for (1 tab + 4 space) and 416 for 1 tab + 3 space.
There are less than 200 in cpp files.

So we should decide one way or the other, but what the linter supports sounds like something that shouldn't be decisive, the linter should adapt to whatever we come up with.

3381

return this.template.NaturalBehaviour !== undefined; also returns a boolean.
The last line is what should be here, so it should be changed to that, and Im not making two commits for this style change.
So if we are uncertain that the proposed change is correct, then we should work on that.
I've posted the steps to reproduce that information above already.
https://code.wildfiregames.com/source/0ad/browse/ps/trunk/source/simulation2/system/ParamNode.cpp

4829

I understand the idea, I committed some of them too some day ago, but I've changed my mind, because those aren't objects, aren't arrays, aren't function scopes. It doesn't make it any less readable if the closing parentheses is the same line as the function call.

It would also allow for var x = func(\n\tbar\n);`, but it's not a space that is opened like it is with objects, arrays or function scopes where one can insert arbitrary elements. meh

Krinkle marked 2 inline comments as done and an inline comment as not done.Sep 25 2019, 8:56 PM
Krinkle added inline comments.
binaries/data/mods/public/simulation/components/UnitAI.js
2055

I agree about code and convention first, linter second.

I'm bringing it up because it's not entirely arbitrary. The vast majority of JavaScript code in the world, as developed professionally by people who work on it daily and whom might be interested in contributing here, they generally interpret the language and stylistically prefer one of the ways provided by the linter. Because they too held code first and linter second, and implemented the rule in question.

At this point it's a trade off to be made:

  • Cost and impact of not having an indentation rule (sloppy code, wasted review efforts on telling people to line up code in a for JS-wise unusual manner, and inability for 0AD contributors to have code automatically formatted in their IDE or text editor through ESLint plugin).
  • Loss in value over our belief of readability for this particular change (it's not about all rules and all conventions, it's about this particular convention, how much value it brings to us and how bad/good the alternative is).
  • Loss in value over having different conventions in CPP and JS. Personally, I think it's manageable to diverge a little bit, they are after all very different languages. Being too strict on requiring similarity will result in illusions taking form that make code look like it behaves a certain way when ti doesn't to please a similarity with an unrelated programming language. Might not be the case here, but something to keep in mind more generally.
  • Alternatively - Cost of implementing and maintaining our own ESLint plugin. Not likely to be very rewarding if done locally, but perhaps something upstream is interested in accepting.

What is your recommendation for the short-term, to get us to a point where ESLint is warning-free? Disable the rule in this file, directory or block? Change indentation?

3381

I don't mind changing it to !== undefined if you're comfortable with that and confident that this change in behaviour is not a problem. Boolean casting to false accepts many more values than merely undefined. I currently do not know this code base well enough to confidently say the difference won't be observable and cause bugs. Hence I would lean towards doing that code change separate from a style change.

But if we know where it's used, okay with spending time on testing it as part of review, I will update it then.

I optimise my patches for needing very little testing from you because my short-term goal is not to change the code behaviour, but to make it pass the linter, so that it can start providing useful feedback about real changes in behaviour and slowly raising our bar of standard quality. This is only the first phase to clear out the noise. But I am getting used to now that you either don't trust that style changes won't cause bugs and want to test extensively regardless, or that you prefer to do it all in one go within a given area of code. That's not how I usually work, but it's no problem, I'll adapt :)

Krinkle marked 4 inline comments as done.Sep 29 2019, 1:34 AM
Krinkle added inline comments.
binaries/data/mods/public/simulation/components/UnitAI.js
3381

I've done a full audit and indeed NaturalBehaviour is only ever assigned a string, and thus yields the undefined value is accessed otherwise.

3385

This IsAnimal() check appears redundant. Would it be okay to remove as part of this change?

Krinkle updated this revision to Diff 9998.Sep 29 2019, 1:35 AM

Updated IsAnimal to use stricter !== undefined check instead of bool cast.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/345/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/components/UnitAI.js
| 331| »   »   »   return·true;
|    | [NORMAL] ESLintBear (consistent-return):
|    | Method 'Order.WalkToTarget' expected no return value.

binaries/data/mods/public/simulation/components/UnitAI.js
| 928| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
| 953| »   »   »   "enter":·function(msg)·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1003| »   »   »   »   »   return·true;
|    | [NORMAL] ESLintBear (consistent-return):
|    | Method 'enter' expected no return value.

binaries/data/mods/public/simulation/components/UnitAI.js
|1066| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1102| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1134| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1298| »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1355| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1530| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1552| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1584| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1738| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1788| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1866| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2045| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2161| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2439| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2472| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2578| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2644| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2683| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2897| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|3078| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|3811| »   var·isWorkType·=·type·=>·type·==·"Gather"·||·type·==·"Trade"·||·type·==·"Repair"·||·type·==·"ReturnResource";
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'type' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4666| »   var·target·=·ents.find(target·=>·this.CanAttack(target));
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'target' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4681| »   var·target·=·ents.find(target·=>
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'target' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4727| »   var·ent·=·ents.find(ent·=>·this.CanHeal(ent));
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'ent' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4750| »   »   ····cmpAttack.GetAttackTypes().some(type·=>·cmpUnitAI.CheckTargetAttackRange(this.isGuardOf,·type)))
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'type' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|3773| »   »   var·order·=·{·"type":·type,·"data":·data·};
|    | [NORMAL] JSHintBear:
|    | 'order' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|3842| »   for·(var·i·=·0;·i·<·this.orderQueue.length;·++i)
|    | [NORMAL] JSHintBear:
|    | 'i' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5587| »   var·targets·=·this.GetTargetsFromUnit();
|    | [NORMAL] JSHintBear:
|    | 'targets' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5588| »   for·(var·targ·of·targets)
|    | [NORMAL] JSHintBear:
|    | 'targ' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5594| »   »   »   var·cmpIdentity·=·Engine.QueryInterface(targ,·IID_Identity);
|    | [NORMAL] JSHintBear:
|    | 'cmpIdentity' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5595| »   »   »   var·targetClasses·=·this.order.data.targetClasses;
|    | [NORMAL] JSHintBear:
|    | 'targetClasses' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5675| »   »   var·cmpVision·=·Engine.QueryInterface(this.entity,·IID_Vision);
|    | [NORMAL] JSHintBear:
|    | 'cmpVision' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5678| »   »   var·range·=·cmpVision.GetRange();
|    | [NORMAL] JSHintBear:
|    | 'range' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5683| »   »   var·cmpRanged·=·Engine.QueryInterface(this.entity,·iid);
|    | [NORMAL] JSHintBear:
|    | 'cmpRanged' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5686| »   »   var·range·=·iid·!==·IID_Attack·?·cmpRanged.GetRange()·:·cmpRanged.GetFullAttackRange();
|    | [NORMAL] JSHintBear:
|    | 'range' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5687| »   »   var·cmpVision·=·Engine.QueryInterface(this.entity,·IID_Vision);
|    | [NORMAL] JSHintBear:
|    | 'cmpVision' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5697| »   »   var·cmpVision·=·Engine.QueryInterface(this.entity,·IID_Vision);
|    | [NORMAL] JSHintBear:
|    | 'cmpVision' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5700| »   »   var·range·=·cmpVision.GetRange();
|    | [NORMAL] JSHintBear:
|    | 'range' is already defined.
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/861/display/redirect

Krinkle updated this revision to Diff 10480.Dec 5 2019, 11:51 PM
Krinkle marked an inline comment as not done.

Rebased to account for other changes in the repos since mid-September.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/685/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/components/UnitAI.js
| 331| »   »   »   return·true;
|    | [NORMAL] ESLintBear (consistent-return):
|    | Method 'Order.WalkToTarget' expected no return value.

binaries/data/mods/public/simulation/components/UnitAI.js
| 928| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
| 953| »   »   »   "enter":·function(msg)·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1003| »   »   »   »   »   return·true;
|    | [NORMAL] ESLintBear (consistent-return):
|    | Method 'enter' expected no return value.

binaries/data/mods/public/simulation/components/UnitAI.js
|1066| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1102| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1134| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1298| »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1355| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1530| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1552| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1584| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1738| »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1788| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|1866| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2045| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2161| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2439| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2472| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2578| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2644| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2683| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|2897| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|3078| »   »   »   »   "enter":·function()·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Expected to return a value at the end of method 'enter'.

binaries/data/mods/public/simulation/components/UnitAI.js
|3811| »   var·isWorkType·=·type·=>·type·==·"Gather"·||·type·==·"Trade"·||·type·==·"Repair"·||·type·==·"ReturnResource";
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'type' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4665| »   var·target·=·ents.find(target·=>·this.CanAttack(target));
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'target' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4680| »   var·target·=·ents.find(target·=>
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'target' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4726| »   var·ent·=·ents.find(ent·=>·this.CanHeal(ent));
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'ent' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4749| »   »   ····cmpAttack.GetAttackTypes().some(type·=>·cmpUnitAI.CheckTargetAttackRange(this.isGuardOf,·type)))
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'type' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|3773| »   »   var·order·=·{·"type":·type,·"data":·data·};
|    | [NORMAL] JSHintBear:
|    | 'order' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|3842| »   for·(var·i·=·0;·i·<·this.orderQueue.length;·++i)
|    | [NORMAL] JSHintBear:
|    | 'i' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5586| »   var·targets·=·this.GetTargetsFromUnit();
|    | [NORMAL] JSHintBear:
|    | 'targets' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5587| »   for·(var·targ·of·targets)
|    | [NORMAL] JSHintBear:
|    | 'targ' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5593| »   »   »   var·cmpIdentity·=·Engine.QueryInterface(targ,·IID_Identity);
|    | [NORMAL] JSHintBear:
|    | 'cmpIdentity' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5594| »   »   »   var·targetClasses·=·this.order.data.targetClasses;
|    | [NORMAL] JSHintBear:
|    | 'targetClasses' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5674| »   »   var·cmpVision·=·Engine.QueryInterface(this.entity,·IID_Vision);
|    | [NORMAL] JSHintBear:
|    | 'cmpVision' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5677| »   »   var·range·=·cmpVision.GetRange();
|    | [NORMAL] JSHintBear:
|    | 'range' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5682| »   »   var·cmpRanged·=·Engine.QueryInterface(this.entity,·iid);
|    | [NORMAL] JSHintBear:
|    | 'cmpRanged' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5685| »   »   var·range·=·iid·!==·IID_Attack·?·cmpRanged.GetRange()·:·cmpRanged.GetFullAttackRange();
|    | [NORMAL] JSHintBear:
|    | 'range' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5686| »   »   var·cmpVision·=·Engine.QueryInterface(this.entity,·IID_Vision);
|    | [NORMAL] JSHintBear:
|    | 'cmpVision' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5696| »   »   var·cmpVision·=·Engine.QueryInterface(this.entity,·IID_Vision);
|    | [NORMAL] JSHintBear:
|    | 'cmpVision' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5699| »   »   var·range·=·cmpVision.GetRange();
|    | [NORMAL] JSHintBear:
|    | 'range' is already defined.
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1201/display/redirect

Krinkle edited the summary of this revision. (Show Details)Feb 3 2020, 3:27 AM

Rebased to resolve merge conflicts.

Krinkle updated this revision to Diff 11265.Feb 3 2020, 10:54 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1706/display/redirect

Stan added a comment.Feb 4 2020, 9:30 AM

Ignore the failed build it randomly segfaults for some reason...