Page MenuHomeWildfire Games

UnitAI: Fix ESLint coding style warnings
Needs ReviewPublic

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

Details

Reviewers
None
Trac Tickets
#5524
Summary

This commit only fixes the warnings from ESLint that are about coding style and were automatically fixable by 'eslint --fix'.

The more substantial issues that might require changing meaningful code will be in a separate commit for easier review.

All the changes in this commit were automated. In addition, I also manually made the pseudo-section comments a bit different.
They previously used four slashes (////) which violated the spaced-comment rule. The auto fix is to use // //, because
ESLint treated it like a normal inline comment. I don't know if these comments are useful long-term, seems to me like these
should be grouped in a more formal way (e.g. different namespace property or separate file loaded for those), but for now I went
with a Markdown-esque way, using // ## .

Before
✖ 77 problems (0 errors, 77 warnings)
  0 errors and 48 warnings potentially fixable with the `--fix` option.

/0ad-git/binaries/data/mods/public/simulation/components/UnitAI.js
   331:4    warning  Method 'Order.WalkToTarget' expected no return value           consistent-return
   766:5    warning  Unnecessary 'else' after 'return'                              no-else-return
   917:13   warning  Expected to return a value at the end of method 'enter'        consistent-return
   942:13   warning  Expected to return a value at the end of method 'enter'        consistent-return
   992:6    warning  Method 'enter' expected no return value                        consistent-return
  1033:14   warning  Missing space before value for key 'GARRISON'                  key-spacing
  1055:14   warning  Expected to return a value at the end of method 'enter'        consistent-return
  1091:13   warning  Expected to return a value at the end of method 'enter'        consistent-return
  1123:14   warning  Expected to return a value at the end of method 'enter'        consistent-return
  1283:12   warning  Expected to return a value at the end of method 'enter'        consistent-return
  1340:13   warning  Expected to return a value at the end of method 'enter'        consistent-return
  1515:13   warning  Expected to return a value at the end of method 'enter'        consistent-return
  1537:13   warning  Expected to return a value at the end of method 'enter'        consistent-return
  1569:13   warning  Expected to return a value at the end of method 'enter'        consistent-return
  1723:13   warning  Expected to return a value at the end of method 'enter'        consistent-return
  1773:14   warning  Expected to return a value at the end of method 'enter'        consistent-return
  1851:14   warning  Expected to return a value at the end of method 'enter'        consistent-return
  1983:9    warning  '&&' should be placed at the end of the line                   operator-linebreak
  2028:14   warning  Expected to return a value at the end of method 'enter'        consistent-return
  2144:14   warning  Expected to return a value at the end of method 'enter'        consistent-return
  2419:14   warning  Expected to return a value at the end of method 'enter'        consistent-return
  2452:14   warning  Expected to return a value at the end of method 'enter'        consistent-return
  2558:14   warning  Expected to return a value at the end of method 'enter'        consistent-return
  2624:14   warning  Expected to return a value at the end of method 'enter'        consistent-return
  2663:14   warning  Expected to return a value at the end of method 'enter'        consistent-return
  2710:41   warning  A space is required after '{'                                  object-curly-spacing
  2710:101  warning  A space is required before '}'                                 object-curly-spacing
  2874:14   warning  Expected to return a value at the end of method 'enter'        consistent-return
  3055:14   warning  Expected to return a value at the end of method 'enter'        consistent-return
  3223:1    warning  Expected indentation of 2 tabs but found 7                     indent
  3280:43   warning  Unnecessary use of boolean literals in conditional expression  no-unneeded-ternary
  3396:72   warning  A space is required after ','                                  comma-spacing
  3452:38   warning  A space is required after '{'                                  object-curly-spacing
  3452:76   warning  A space is required before '}'                                 object-curly-spacing
  3536:1    warning  Expected space or tab after '//' in comment                    spaced-comment
  3706:33   warning  A space is required after '{'                                  object-curly-spacing
  3706:60   warning  A space is required before '}'                                 object-curly-spacing
  3779:19   warning  'type' is already declared in the upper scope                  no-shadow
  3877:36   warning  A space is required after '{'                                  object-curly-spacing
  3877:88   warning  A space is required before '}'                                 object-curly-spacing
  3922:36   warning  A space is required after '{'                                  object-curly-spacing
  3922:80   warning  A space is required before '}'                                 object-curly-spacing
  3947:36   warning  A space is required after '{'                                  object-curly-spacing
  3947:68   warning  A space is required before '}'                                 object-curly-spacing
  3952:36   warning  A space is required after '{'                                  object-curly-spacing
  3952:80   warning  A space is required before '}'                                 object-curly-spacing
  3957:36   warning  A space is required after '{'                                  object-curly-spacing
  3957:92   warning  A space is required before '}'                                 object-curly-spacing
  3963:37   warning  A space is required after '{'                                  object-curly-spacing
  3963:75   warning  A space is required before '}'                                 object-curly-spacing
  3965:37   warning  A space is required after '{'                                  object-curly-spacing
  3965:79   warning  A space is required before '}'                                 object-curly-spacing
  3970:36   warning  A space is required after '{'                                  object-curly-spacing
  3970:81   warning  A space is required before '}'                                 object-curly-spacing
  3973:1    warning  Expected space or tab after '//' in comment                    spaced-comment
  4626:25   warning  'target' is already declared in the upper scope                no-shadow
  4641:25   warning  'target' is already declared in the upper scope                no-shadow
  4643:5    warning  '&&' should be placed at the end of the line                   operator-linebreak
  4644:5    warning  '&&' should be placed at the end of the line                   operator-linebreak
  4687:22   warning  'ent' is already declared in the upper scope                   no-shadow
  4707:20   warning  Multiple spaces found before 'Engine'                          no-multi-spaces
  4710:39   warning  'type' is already declared in the upper scope                  no-shadow
  4711:1    warning  Expected indentation of 3 tabs but found 4                     indent
  4749:20   warning  Multiple spaces found before 'Engine'                          no-multi-spaces
  4762:1    warning  Expected space or tab after '//' in comment                    spaced-comment
  4919:4    warning  Unnecessary 'else' after 'return'                              no-else-return
  5250:36   warning  Trailing spaces not allowed                                    no-trailing-spaces
  5532:9    warning  '&&' should be placed at the end of the line                   operator-linebreak
  5535:9    warning  '&&' should be placed at the end of the line                   operator-linebreak
  5558:7    warning  '&&' should be placed at the end of the line                   operator-linebreak
  5561:7    warning  '&&' should be placed at the end of the line                   operator-linebreak
  5717:22   warning  A space is required after '{'                                  object-curly-spacing
  5717:37   warning  A space is required before '}'                                 object-curly-spacing
  5744:1    warning  Expected space or tab after '//' in comment                    spaced-comment
  5948:1    warning  Expected space or tab after '//' in comment                    spaced-comment
  5954:5    warning  '&&' should be placed at the end of the line                   operator-linebreak
  5957:1    warning  Expected space or tab after '//' in comment                    spaced-comment
After
✖ 29 problems (0 errors, 29 warnings)

/0ad-git/binaries/data/mods/public/simulation/components/UnitAI.js
   331:4   warning  Method 'Order.WalkToTarget' expected no return value     consistent-return
   916:13  warning  Expected to return a value at the end of method 'enter'  consistent-return
   941:13  warning  Expected to return a value at the end of method 'enter'  consistent-return
   991:6   warning  Method 'enter' expected no return value                  consistent-return
  1054:14  warning  Expected to return a value at the end of method 'enter'  consistent-return
  1090:13  warning  Expected to return a value at the end of method 'enter'  consistent-return
  1122:14  warning  Expected to return a value at the end of method 'enter'  consistent-return
  1282:12  warning  Expected to return a value at the end of method 'enter'  consistent-return
  1339:13  warning  Expected to return a value at the end of method 'enter'  consistent-return
  1514:13  warning  Expected to return a value at the end of method 'enter'  consistent-return
  1536:13  warning  Expected to return a value at the end of method 'enter'  consistent-return
  1568:13  warning  Expected to return a value at the end of method 'enter'  consistent-return
  1722:13  warning  Expected to return a value at the end of method 'enter'  consistent-return
  1772:14  warning  Expected to return a value at the end of method 'enter'  consistent-return
  1850:14  warning  Expected to return a value at the end of method 'enter'  consistent-return
  2027:14  warning  Expected to return a value at the end of method 'enter'  consistent-return
  2143:14  warning  Expected to return a value at the end of method 'enter'  consistent-return
  2418:14  warning  Expected to return a value at the end of method 'enter'  consistent-return
  2451:14  warning  Expected to return a value at the end of method 'enter'  consistent-return
  2557:14  warning  Expected to return a value at the end of method 'enter'  consistent-return
  2623:14  warning  Expected to return a value at the end of method 'enter'  consistent-return
  2662:14  warning  Expected to return a value at the end of method 'enter'  consistent-return
  2873:14  warning  Expected to return a value at the end of method 'enter'  consistent-return
  3054:14  warning  Expected to return a value at the end of method 'enter'  consistent-return
  3778:19  warning  'type' is already declared in the upper scope            no-shadow
  4625:25  warning  'target' is already declared in the upper scope          no-shadow
  4640:25  warning  'target' is already declared in the upper scope          no-shadow
  4686:22  warning  'ent' is already declared in the upper scope             no-shadow
  4709:39  warning  'type' is already declared in the upper scope            no-shadow
Test Plan

Slightly less unhappy Coala bears.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
arcpatch_D2279 (branched from master)
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 9373
Build 15558: Vulcan BuildJenkins
Build 15557: Vulcan Build (Windows)Jenkins
Build 15556: arc lint + arc unit

Event Timeline

Krinkle created this revision.Tue, Sep 10, 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.Thu, Sep 12, 11:59 PM
elexis added inline comments.
binaries/data/mods/public/simulation/components/UnitAI.js
774

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.

2002

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")
3307

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

3565

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.Fri, Sep 13, 12:08 AM
Krinkle added inline comments.
binaries/data/mods/public/simulation/components/UnitAI.js
3307

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).
3565

Sounds good to me.

Krinkle planned changes to this revision.Fri, Sep 13, 12:08 AM
Krinkle updated this revision to Diff 9749.Sat, Sep 14, 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.Sat, Sep 14, 6:57 AM
Krinkle added inline comments.
binaries/data/mods/public/simulation/components/UnitAI.js
2002

(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.Sat, Sep 14, 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.Sat, Sep 14, 12:12 PM
Stan added inline comments.
binaries/data/mods/public/simulation/components/UnitAI.js
3307

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.Sat, Sep 14, 2:21 PM
binaries/data/mods/public/simulation/components/UnitAI.js
2002

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.

2730–2733

(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.)

3307

!== 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.

3565

ping

3909–3913

(same)

3958

(same)

5756

(I suppose this should be a vector some time)

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

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.Sat, Sep 14, 7:02 PM
Krinkle added inline comments.
binaries/data/mods/public/simulation/components/UnitAI.js
3307

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.Sat, Sep 14, 7:57 PM
Krinkle added inline comments.
binaries/data/mods/public/simulation/components/UnitAI.js
2730–2733

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.Sat, Sep 14, 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