Page MenuHomeWildfire Games

Change comment for MatchesClassList
ClosedPublic

Authored by bb on Sep 4 2017, 10:35 PM.

Details

Reviewers
leper
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP20188: Change comment for the MatchesClassList function so we can use it for other…
Summary

This way we can use the function for requiredTechs attackTypes and others aswell and we don't need duplication for that.
On the fly also changed the commend style and var=> let

Test Plan

Agree on new comment and check english
Notice command has better style

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

bb created this revision.Sep 4 2017, 10:35 PM
Owners added subscribers: Restricted Owners Package, Restricted Owners Package, Restricted Owners Package.Sep 4 2017, 10:35 PM
elexis added a reviewer: Restricted Owners Package.Sep 4 2017, 11:16 PM
elexis added a subscriber: elexis.

Is the rename to string list correct?
This is a specific syntax, it's kind of a brand.
Would it be wrong to reuse MatchesClassList for attack types and requirements?
The simulation guys should be asked.

Vulcan added a subscriber: Vulcan.Sep 4 2017, 11:32 PM
Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/maps/scripts/Regicide.js
|  30| »   »   ····playersCivs.every(civ·=>·civ·!=·identity.Civ))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/maps/scripts/Regicide.js
|  63| »   »   let·spawnPoints·=·cmpRangeManager.GetEntitiesByPlayer(playerID).sort((entity1,·entity2)·=>
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/maps/scripts/Regicide.js
|  70| »   »   »   let·distanceToShip·=·entity·=>
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/maps/scripts/Regicide.js
|  72| »   »   »   spawnPoints·=·TriggerHelper.GetLandSpawnPoints().sort((entity1,·entity2)·=>
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'civPermitted' to undefined.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/globalscripts/Technologies.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/globalscripts/Technologies.js
| 223| 223| 
| 224| 224| 	case "all":
| 225| 225| 	{
| 226|    |-		let civPermitted = undefined; // tri-state (undefined, false, or true)
|    | 226|+		let civPermitted; // tri-state (undefined, false, or true)
| 227| 227| 		for (let subvalue of value)
| 228| 228| 		{
| 229| 229| 			let newOper = Object.keys(subvalue)[0];

binaries/data/mods/public/globalscripts/Technologies.js
| 233| »   »   »   switch·(newOper)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/globalscripts/Technologies.js
| 307| »   »   »   switch·(newOper)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/globalscripts/Technologies.js
| 226| »   »   let·civPermitted·=·undefined;·//·tri-state·(undefined,·false,·or·true)
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'civPermitted' to 'undefined'.

binaries/data/mods/public/globalscripts/Technologies.js
| 249| »   »   »   »   »   return·false;
|    | [NORMAL] JSHintBear:
|    | Expected a 'break' statement before 'case'.

binaries/data/mods/public/globalscripts/Technologies.js
| 259| »   »   »   »   }
|    | [NORMAL] JSHintBear:
|    | Expected a 'break' statement before 'case'.

binaries/data/mods/public/globalscripts/Technologies.js
| 328| »   »   »   »   }
|    | [NORMAL] JSHintBear:
|    | Expected a 'break' statement before 'case'.

binaries/data/mods/public/globalscripts/Technologies.js
| 334| »   »   »   »   civPermitted·=·true;
|    | [NORMAL] JSHintBear:
|    | Expected a 'break' statement before 'case'.

binaries/data/mods/public/simulation/components/Auras.js
| 241| »   »   »   »   »   let·playerEnts·=·affectedPlayers.map(player·=>·cmpPlayerManager.GetPlayerByID(player));
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'player' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/Auras.js
| 241| »   »   »   »   »   let·playerEnts·=·affectedPlayers.map(player·=>·cmpPlayerManager.GetPlayerByID(player));
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/maps/random/danubius_triggers.js
| 239| »   if·(remainder·!=·0)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/maps/random/danubius_triggers.js
| 456| »   if·(remainder·!=·0)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/maps/random/danubius_triggers.js
| 646| »   »   let·siegeEngines·=·attackers.filter(ent·=>·Engine.QueryInterface(ent,·IID_Identity).HasClass("Siege"));
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/maps/random/danubius_triggers.js
| 647| »   »   let·others·=·attackers.filter(ent·=>·siegeEngines.indexOf(ent)·==·-1);
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/maps/random/danubius_triggers.js
| 669| »   if·(data.from·!=·0)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/simulation/ai/common-api/gamestate.js
| 266| »   »   switch·(entity.check)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/simulation/ai/common-api/gamestate.js
| 283| »   »   »   switch·(type)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/simulation/ai/common-api/gamestate.js
| 776| »   »   »   if·(this.phases.every(phase·=>·template._templateName·!=·phase.name))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/GuiInterface.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/GuiInterface.js
| 209| 209| {
| 210| 210| 	if (this.miragedEntities[player])
| 211| 211| 		return this.renamedEntities.concat(this.miragedEntities[player]);
| 212|    |-	else
| 213|    |-		return this.renamedEntities;
|    | 212|+	return this.renamedEntities;
| 214| 213| };
| 215| 214| 
| 216| 215| GuiInterface.prototype.ClearRenamedEntities = function()
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/GuiInterface.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/GuiInterface.js
|1236|1236| 
|1237|1237| 		return false;
|1238|1238| 	}
|1239|    |-	else
|1240|    |-	{
|    |1239|+	
|1241|1240| 		// Move all existing cached entities outside of the world and reset their use count
|1242|1241| 		for (let tpl in this.placementWallEntities)
|1243|1242| 		{
|1271|1270| 				}
|1272|1271| 			}
|1273|1272| 		}
|1274|    |-	}
|    |1273|+	
|1275|1274| 
|1276|1275| 	// prevent division by zero errors further on if the start and end positions are the same
|1277|1276| 	if (end.pos && (start.pos.x === end.pos.x && start.pos.z === end.pos.z))
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/GuiInterface.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/GuiInterface.js
|2038|2038| {
|2039|2039| 	if (exposedFunctions[name])
|2040|2040| 		return this[name](player, args);
|2041|    |-	else
|2042|    |-		throw new Error("Invalid GuiInterface Call name \""+name+"\"");
|    |2041|+	throw new Error("Invalid GuiInterface Call name \""+name+"\"");
|2043|2042| };
|2044|2043| 
|2045|2044| Engine.RegisterSystemComponentType(IID_GuiInterface, "GuiInterface", GuiInterface);

binaries/data/mods/public/simulation/components/GuiInterface.js
| 664| »   for·(let·name·of·auraNames)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'name' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/GuiInterface.js
| 664| »   for·(let·name·of·auraNames)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'name' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/GuiInterface.js
| 764| »   if·(notification.players·==·undefined)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with 'undefined'.

binaries/data/mods/public/simulation/components/GuiInterface.js
|1032| »   »   »   »   if·(cmd.queued·==·true)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with 'true'.

binaries/data/mods/public/simulation/components/GuiInterface.js
|1094| »   »   if·(cmd.template·==·"")
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with ''.

binaries/data/mods/public/simulation/components/GuiInterface.js
|1515| »   »   »   »   if·(terrainGroundPrev·!=·null·||·terrainGroundNext·!=·null)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'null'.

binaries/data/mods/public/simulation/components/GuiInterface.js
|1515| »   »   »   »   if·(terrainGroundPrev·!=·null·||·terrainGroundNext·!=·null)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'null'.

binaries/data/mods/public/simulation/components/GuiInterface.js
|1637| »   if·(numRequiredPieces·>·0·&&·result.pieces.length·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/components/GuiInterface.js
|1707| »   »   if·(minDistEntitySnapData·!=·null)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'null'.

binaries/data/mods/public/simulation/components/GuiInterface.js
|1759| »   »   if(bucket·==·0·&&·data.prevUnit·&&·entity·<=·data.prevUnit)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/components/GuiInterface.js
|1767| »   »   if·(data.limit·&&·bucket·==·0·&&·idleUnits[0].length·==·data.limit)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/maps/random/polar_sea_triggers.js
|  58| »   »   »   for·(let·target·of·targets.sort((ent1,·ent2)·=>·getDistance(attacker,·ent1)·-·getDistance(attacker,·ent2)).slice(0,·targetCount))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/simulation/components/BuildingAI.js
| 125| »   if·(enemies.length·&&·enemies[0]·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/components/BuildingAI.js
| 295| »   if·(this.currentRound·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'm'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/ai/petra/defenseManager.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/ai/petra/defenseManager.js
| 627| 627| 					if (minEnt)
| 628| 628| 					{
| 629| 629| 						capturableTarget.ents.delete(minEnt.id());
| 630|    |-						minEnt.attack(attacker.id(),  m.allowCapture(gameState, minEnt, attacker));
|    | 630|+						minEnt.attack(attacker.id(), m.allowCapture(gameState, minEnt, attacker));
| 631| 631| 					}
| 632| 632| 				}
| 633| 633| 			}
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'm'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/ai/petra/defenseManager.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/ai/petra/defenseManager.js
| 631| 631| 					}
| 632| 632| 				}
| 633| 633| 			}
| 634|    |-			target.attack(attacker.id(),  m.allowCapture(gameState, target, attacker));
|    | 634|+			target.attack(attacker.id(), m.allowCapture(gameState, target, attacker));
| 635| 635| 		}
| 636| 636| 	}
| 637| 637| };

binaries/data/mods/public/simulation/components/Player.js
| 142| »   if·(num·!=·0·&&·num·>·(this.GetPopulationLimit()·-·this.GetPopulationCount()))
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/simulation/components/Player.js
| 274| »   »   if·(this.resourceCount[type]·!=·undefined·&&·amounts[type]·>·this.resourceCount[type])
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'undefined'.

binaries/data/mods/public/simulation/components/Player.js
| 277| »   if·(Object.keys(amountsNeeded).length·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/components/Player.js
| 331| »   for·(var·type·in·amounts)
|    | [NORMAL] JSHintBear:
|    | 'type' is already defined.

binaries/data/mods/public/simulation/components/Player.js
| 710| »   return·this.diplomacy[id]·==·0;
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 167| »   »   »   let·w·=·+right["@x"]·+·+right["@width"]/2·-·+left["@x"]·+·+left["@width"]/2;
|    | [NORMAL] JSHintBear:
|    | Confusing plusses.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 167| »   »   »   let·w·=·+right["@x"]·+·+right["@width"]/2·-·+left["@x"]·+·+left["@width"]/2;
|    | [NORMAL] JSHintBear:
|    | Confusing plusses.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 168| »   »   »   let·h·=·Math.max(+right["@z"]·+·+right["@depth"]/2,·+left["@z"]·+·+left["@depth"]/2)
|    | [NORMAL] JSHintBear:
|    | Confusing plusses.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 168| »   »   »   let·h·=·Math.max(+right["@z"]·+·+right["@depth"]/2,·+left["@z"]·+·+left["@depth"]/2)
|    | [NORMAL] JSHintBear:
|    | Confusing plusses.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 169| »   »   »   ······-·Math.min(+right["@z"]·-·+right["@depth"]/2,·+left["@z"]·-·+left["@depth"]/2);
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '-'; readers may interpret this as an expression boundary.
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'getActionInfo'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/unit_actions.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/unit_actions.js
| 691| 691| 			if (preSelectedAction != ACTION_GARRISON)
| 692| 692| 				return false;
| 693| 693| 
| 694|    |-			let actionInfo =  getActionInfo("garrison", target);
|    | 694|+			let actionInfo = getActionInfo("garrison", target);
| 695| 695| 			if (!actionInfo.possible)
| 696| 696| 				return {
| 697| 697| 					"type": "none",

binaries/data/mods/public/gui/session/unit_actions.js
| 589| »   »   »   switch·(tradingDetails.type)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/unit_actions.js
|1064| »   »   »   »   "tooltip":·colorizeHotkey("%(hotkey)s"·+·"·",·"session.unload")·+
|    | [NORMAL] ESLintBear (no-useless-concat):
|    | Unexpected string concatenation of literals.

binaries/data/mods/public/gui/session/unit_actions.js
|1082| »   »   »   »   »   »   colorizeHotkey("%(hotkey)s"·+·"·",·"session.kill")·+
|    | [NORMAL] ESLintBear (no-useless-concat):
|    | Unexpected string concatenation of literals.

binaries/data/mods/public/gui/session/unit_actions.js
|1121| »   »   »   »   "tooltip":·colorizeHotkey("%(hotkey)s"·+·"·",·"session.stop")·+
|    | [NORMAL] ESLintBear (no-useless-concat):
|    | Unexpected string concatenation of literals.

binaries/data/mods/public/gui/session/unit_actions.js
|1140| »   »   »   »   "tooltip":·colorizeHotkey("%(hotkey)s"·+·"·",·"session.garrison")·+
|    | [NORMAL] ESLintBear (no-useless-concat):
|    | Unexpected string concatenation of literals.

binaries/data/mods/public/gui/session/unit_actions.js
|1181| »   »   »   »   "tooltip":·colorizeHotkey("%(hotkey)s"·+·"·",·"session.repair")·+
|    | [NORMAL] ESLintBear (no-useless-concat):
|    | Unexpected string concatenation of literals.

binaries/data/mods/public/gui/session/unit_actions.js
|1200| »   »   »   »   "tooltip":·colorizeHotkey("%(hotkey)s"·+·"·",·"camera.rallypointfocus")·+
|    | [NORMAL] ESLintBear (no-useless-concat):
|    | Unexpected string concatenation of literals.

binaries/data/mods/public/gui/session/unit_actions.js
|1235| »   »   »   »   "tooltip":·colorizeHotkey("%(hotkey)s"·+·"·",·"session.backtowork")·+
|    | [NORMAL] ESLintBear (no-useless-concat):
|    | Unexpected string concatenation of literals.

binaries/data/mods/public/gui/session/unit_actions.js
|1254| »   »   »   »   "tooltip":·colorizeHotkey("%(hotkey)s"·+·"·",·"session.guard")·+
|    | [NORMAL] ESLintBear (no-useless-concat):
|    | Unexpected string concatenation of literals.

binaries/data/mods/public/gui/session/unit_actions.js
|1307| »   »   »   »   "tooltip":·colorizeHotkey("%(hotkey)s"·+·"·",·"session.patrol")·+
|    | [NORMAL] ESLintBear (no-useless-concat):
|    | Unexpected string concatenation of literals.

binaries/data/mods/public/gui/session/unit_actions.js
|1382| »   »   »   »   "tooltip":·colorizeHotkey("%(hotkey)s"·+·"·",·"session.unload")·+
|    | [NORMAL] ESLintBear (no-useless-concat):
|    | Unexpected string concatenation of literals.

binaries/data/mods/public/gui/session/unit_actions.js
| 613| »   »   »   »   if·(tradingDetails.gain.traderGain·==·0)···//·markets·too·close
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/gui/session/unit_actions.js
|1425| »   »   if·(player·==·"Gaia"·&&·targetState.player·==·0·||
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/ai/petra/garrisonManager.js
|  84| »   »   »   »   if·(ent.unitAIOrderData().some(order·=>·order.target·&&·order.target·==·id))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/simulation/components/AuraManager.js
| 155| »   »   if·(e.size·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/components/AuraManager.js
| 185| »   if·(this.templateModificationsCache.get(value).get(player).get(classes).size·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/components/AuraManager.js
| 190| »   if·(p.size·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/ai/petra/navalManager.js
| 309| »   »   »   plan.units.forEach(function·(ent)·{
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/simulation/ai/petra/navalManager.js
| 320| »   »   »   plan.units.forEach(function·(ent)·{
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/simulation/components/GarrisonHolder.js
| 284| »   if·((!cmpHealth·||·cmpHealth.GetHitpoints()·==·0)·&&·cmpIdentity·&&·cmpIdentity.HasClass("Ship"))
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/components/GarrisonHolder.js
| 563| »   »   if·(cmpHealth·&&·cmpHealth.GetHitpoints()·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/components/GarrisonHolder.js
| 693| »   if·(this.timer·&&·this.GetHealRate()·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/components/Attack.js
| 199| »   let·wantedTypesReal·=·wantedTypes.filter(wtype·=>·wtype.indexOf("!")·!=·0);
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/simulation/components/Attack.js
| 239| »   ···(!wantedTypes·||·!wantedTypes.filter(wType·=>·wType.indexOf("!")·!=·0).length))
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/simulation/components/Attack.js
| 534| »   »   if·(!cmpHealth·||·cmpHealth.GetHitpoints()·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/components/Attack.js
| 586| »   if·(c·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/globalscripts/Templates.js
|  60| »   »   ····················||·(c[0]·!=·"!"·&&·origin.indexOf(c)·!=·-1)))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.

binaries/data/mods/public/globalscripts/Templates.js
|  59| »   »   if·(sublist.every(c·=>·(c[0]·==·"!"·&&·origin.indexOf(c.substr(1))·==·-1)
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/globalscripts/Templates.js
| 145| »   »   »   let·getAttackStat·=·function(stat)·{
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'target' to undefined.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/input.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/input.js
| 271| 271| 	if (!g_DevSettings.controlAll && !allOwnedByPlayer)
| 272| 272| 		return undefined;
| 273| 273| 
| 274|    |-	var target = undefined;
|    | 274|+	var target;
| 275| 275| 	if (!fromMinimap)
| 276| 276| 	{
| 277| 277| 		var ent = Engine.PickEntityAtPoint(x, y);
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'actionInfo' to undefined.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/input.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/input.js
| 285| 285| 	var actions = Object.keys(unitActions).slice();
| 286| 286| 	actions.sort((a, b) => unitActions[a].specificness - unitActions[b].specificness);
| 287| 287| 
| 288|    |-	var actionInfo = undefined;
|    | 288|+	var actionInfo;
| 289| 289| 	if (preSelectedAction != ACTION_NONE)
| 290| 290| 	{
| 291| 291| 		for (var action of actions)

binaries/data/mods/public/gui/session/input.js
| 267| »   »   var·entState·=·GetEntityState(ent);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'entState' is already declared in the upper scope.

binaries/data/mods/public/gui/session/input.js
| 495| »   switch·(ev.type)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/input.js
| 522| »   switch·(inputState)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/input.js
| 526| »   »   switch·(ev.type)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/input.js
| 581| »   »   switch·(ev.type)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/input.js
| 588| »   »   »   var·maxDragDelta·=·16;
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'maxDragDelta' is already declared in the upper scope.

binaries/data/mods/public/gui/session/input.js
| 631| »   »   switch·(ev.type)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/input.js
| 660| »   »   switch·(ev.type)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/input.js
| 729| »   »   switch·(ev.type)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/input.js
| 849| »   switch·(inputState)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/input.js
| 852| »   »   switch·(ev.type)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/input.js
| 954| »   »   switch·(ev.type)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/input.js
|1046| »   »   switch·(ev.type)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/input.js
|1115| »   »   »   switch·(ev.hotkey)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/input.js
|1538| »   switch·(action)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/input.js
| 232| »   »   var·entState·=·GetExtendedEntityState(entityID);
|    | [NORMAL] JSHintBear:
|    | 'entState' is already defined.

binaries/data/mods/public/gui/session/input.js
| 274| »   var·target·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'target' to 'undefined'.

binaries/data/mods/public/gui/session/input.js
| 288| »   var·actionInfo·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'actionInfo' to 'undefined'.

binaries/data/mods/public/gui/session/input.js
| 302| »   for·(var·action·of·actions)
|    | [NORMAL] JSHintBear:
|    | 'action' is already defined.

binaries/data/mods/public/gui/session/input.js
| 305| »   »   »   var·r·=·unitActions[action].hotkeyActionCheck(target,·selection);
|    | [NORMAL] JSHintBear:
|    | 'r' is already defined.

binaries/data/mods/public/gui/session/input.js
| 310| »   for·(var·action·of·actions)
|    | [NORMAL] JSHintBear:
|    | 'action' is already defined.

binaries/data/mods/public/gui/session/input.js
| 313| »   »   »   var·r·=·unitActions[action].actionCheck(target,·selection);
|    | [NORMAL] JSHintBear:
|    | 'r' is already defined.

binaries/data/mods/public/gui/session/input.js
| 506| »   mouseIsOverObject·=·(hoveredObject·!=·null);
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'null'.

binaries/data/mods/public/gui/session/input.js
| 510| »   »   &&·(ev.button·==·SDL_BUTTON_LEFT·||·ev.button·==·SDL_BUTTON_RIGHT))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/gui/session/input.js
| 540| »   »   »   »   var·rect·=·updateBandbox(bandbox,·ev,·true);
|    | [NORMAL] JSHintBear:
|    | 'rect' is already defined.

binaries/data/mods/public/gui/session/input.js
| 543| »   »   »   »   var·ents·=·getPreferredEntities(Engine.PickPlayerEntitiesInRect(rect[0],·rect[1],·rect[2],·rect[3],·g_ViewedPlayer));
|    | [NORMAL] JSHintBear:
|    | 'ents' is already defined.

binaries/data/mods/public/gui/session/input.js
| 692| »   »   »   »   »   var·queued·=·Engine.HotkeyIsPressed("session.queue");
|    | [NORMAL] JSHintBear:
|    | 'queued' is already defined.

binaries/data/mods/public/gui/session/input.js
| 732| »   »   »   var·dragDeltaX·=·ev.x·-·dragStart[0];
|    | [NORMAL] JSHintBear:
|    | 'dragDeltaX' is already defined.

binaries/data/mods/public/gui/session/input.js
| 733| »   »   »   var·dragDeltaY·=·ev.y·-·dragStart[1];
|    | [NORMAL] JSHintBear:
|    | 'dragDeltaY' is already defined.

binaries/data/mods/public/gui/session/input.js
| 734| »   »   »   var·maxDragDelta·=·16;
|    | [NORMAL] JSHintBear:
|    | 'maxDragDelta' is already defined.

binaries/data/mods/public/gui/session/input.js
| 766| »   »   »   »   var·queued·=·Engine.HotkeyIsPressed("session.queue");
|    | [NORMAL] JSHintBear:
|    | 'queued' is already defined.

binaries/data/mods/public/gui/session/input.js
| 885| »   »   »   »   if·(ev.hotkey.indexOf("selection.group.")·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/gui/session/input.js
| 890| »   »   »   »   »   »   if·(ev.hotkey.indexOf("selection.group.select.")·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/gui/session/input.js
| 898| »   »   »   »   »   »   var·sptr·=·ev.hotkey.split(".");
|    | [NORMAL] JSHintBear:
|    | 'sptr' is already defined.

binaries/data/mods/public/gui/session/input.js
| 914| »   »   »   var·ent·=·Engine.PickEntityAtPoint(ev.x,·ev.y);
|    | [NORMAL] JSHintBear:
|    | 'ent' is already defined.

binaries/data/mods/public/gui/session/input.js
| 925| »   »   »   »   var·action·=·determineAction(ev.x,·ev.y);
|    | [NORMAL] JSHintBear:
|    | 'action' is already defined.

binaries/data/mods/public/gui/session/input.js
| 940| »   »   »   }
|    | [NORMAL] JSHintBear:
|    | Expected a 'break' statement before 'default'.

binaries/data/mods/public/gui/session/input.js
| 944| »   »   »   if·(g_Selection.toList().length·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/gui/session/input.js
| 967| »   »   »   var·ent·=·Engine.PickEntityAtPoint(ev.x,·ev.y);
|    | [NORMAL] JSHintBear:
|    | 'ent' is already defined.

binaries/data/mods/public/gui/session/input.js
|1179| »   if(getEntityLimitAndCount(playerState,·buildTemplate).canBeAddedCount·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/gui/session/input.js
|1357| »   var·batchTrainingPossible·=·limits.canBeAddedCount·==·undefined·||·limits.canBeAddedCount·>·1;
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with 'undefined'.

binaries/data/mods/public/gui/session/input.js
|1387| »   »   »   »   else·if·(limits.canBeAddedCount·==·undefined·||
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with 'undefined'.

binaries/data/mods/public/gui/session/input.js
|1391| »   »   »   »   »   »   multiplyEntityCosts(template,·g_BatchTrainingCount·+·batchIncrementSize)·}))
|    | [NORMAL] JSHintBear:
|    | 'template' used out of scope.

binaries/data/mods/public/gui/session/input.js
|1409| »   »   »   multiplyEntityCosts(template,·batchIncrementSize)·}))
|    | [NORMAL] JSHintBear:
|    | 'template' used out of scope.

binaries/data/mods/public/gui/session/input.js
|1464| »   if·((limits.canBeAddedCount·==·undefined·||
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with 'undefined'.

binaries/data/mods/public/simulation/ai/petra/headquarters.js
| 168| »   »   »   »   builders.forEach(function·(worker)·{
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/simulation/ai/petra/headquarters.js
|1441| »   »   if·(highLevel·==·0·||·lowLevel·>·1)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/ai/petra/headquarters.js
|2357| »   »   if·(gameState.ai.playedTurn·%·4·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
| 911| 911| 					this.FinishOrder();
| 912| 912| 					return;
| 913| 913| 				}
| 914|    |-				else
| 915|    |-				{
|    | 914|+				
| 916| 915| 					// Out of range; move there in formation
| 917| 916| 					if (this.MoveToGarrisonRange(msg.data.target))
| 918| 917| 					{
| 919| 918| 						this.SetNextState("GARRISON.APPROACHING");
| 920| 919| 						return;
| 921| 920| 					}
| 922|    |-				}
|    | 921|+				
| 923| 922| 			}
| 924| 923| 
| 925| 924| 			this.SetNextState("GARRISON.GARRISONING");
|    | [NORMAL] ESLintBear (no-unneeded-ternary):
|    | Unnecessary use of boolean literals in conditional expression.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|1958|1958| 					// TODO: we should probably only bother syncing projectile attacks, not melee
|1959|1959| 
|1960|1960| 					// If using a non-default prepare time, re-sync the animation when the timer runs.
|1961|    |-					this.resyncAnimation = (prepare != this.attackTimers.prepare) ? true : false;
|    |1961|+					this.resyncAnimation = (prepare != this.attackTimers.prepare);
|1962|1962| 
|1963|1963| 					this.FaceTowardsTarget(this.order.data.target);
|1964|1964|

http://jenkins-master:8080/job/phabricator_lint/480/ for more details.

The new function comment seems wrong, since we are checking whether the provided classes (or origin now), satisfy what is in match.

I'm not sure if the new name is an improvement, given that the old name at least tells someone reading the code using it what it roughly does. Why not provide an alias for other use cases? And if you are rewriting the comment it might be worth mentioning that it considers match to be in DNF.

binaries/data/mods/public/globalscripts/Templates.js
57–58 ↗(On Diff #3488)

Does it actually make sense to allow splitting on whitespace here?

That'd make [["A B"], ["D+C"]] equivalent to [["A", "B"], ["D", "C"]] which clearly isn't equivalent to "A B D+C". And we could only get the former case when someone passes that in directly, which I would consider misuse of the function and we should just disallow that behaviour, and possibly warn in case there is any whitespace encountered at this stage.

Vulcan added a comment.Sep 5 2017, 1:10 AM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jenkins-master:8080/job/phabricator/1965/ for more details.

bb added a comment.Sep 5 2017, 11:13 AM

What I want to do is using this function not only for class strings but for any list of strings. And duplicating the code for every use seems a waste and using the old name for many other things might be unclear for the reader too. However if someone provides a better name we can go with that.

binaries/data/mods/public/globalscripts/Templates.js
57–58 ↗(On Diff #3488)

didn't want the function behaviour to change in a rename but could be done

bb added a comment.Sep 5 2017, 11:53 AM

Another option would be to keep the MatchesClassList name and use it for eveything ((11:40:53) elexis: noone said its an Identity class), then we just need to change the comment so it doesn't specify the identity component. Opinions?

In D869#34064, @bb wrote:

Another option would be to keep the MatchesClassList name and use it for eveything ((11:40:53) elexis: noone said its an Identity class), then we just need to change the comment so it doesn't specify the identity component. Opinions?

Sounds better than MatchesStringList. I'd still be for mentioning that it can be used for e.g. identity classes, but apart from that that sounds good.

binaries/data/mods/public/globalscripts/Templates.js
57–58 ↗(On Diff #3488)

You can also split that into another change if you want.

bb updated this revision to Diff 3540.Sep 6 2017, 11:13 AM
bb retitled this revision from Rename MatchesClassList to MatchesStringList to Change comment for MatchesClassList.
bb edited the test plan for this revision. (Show Details)

change comment only, indeed mentioning it can be used for identity classes is no crime

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jenkins-master:8080/job/phabricator/1983/ for more details.

Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/globalscripts/Templates.js
|  62| »   »   ····················||·(c[0]·!=·"!"·&&·classes.indexOf(c)·!=·-1)))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.

binaries/data/mods/public/globalscripts/Templates.js
|  61| »   »   if·(sublist.every(c·=>·(c[0]·==·"!"·&&·classes.indexOf(c.substr(1))·==·-1)
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/globalscripts/Templates.js
| 147| »   »   »   let·getAttackStat·=·function(stat)·{
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jenkins-master:8080/job/phabricator_lint/498/ for more details.

leper accepted this revision.Sep 13 2017, 11:06 PM

Looks good, sorry for the delay.

This revision is now accepted and ready to land.Sep 13 2017, 11:06 PM
This revision was automatically updated to reflect the committed changes.