Page MenuHomeWildfire Games

Don't use the Category in BuildRestrictions to check the hardcoded Dock special constraint.
ClosedPublic

Authored by fatherbushido on Apr 26 2017, 4:25 PM.

Details

Reviewers
leper
s0600204
mimo
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP19839: Use PlacementType instead of Category in BuildRestriction for the hardcoded…
Trac Tickets
#4290
Summary

Some entities like lighthouse use the dock Category in BuildRestrictions to check the special placement constraint but also have their own entity limits.

Test Plan

An in game test could be to try to build several lighthouse with or without the patch.

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

fatherbushido created this revision.Apr 26 2017, 4:25 PM

Edit the help.

Vulcan added a subscriber: Vulcan.Apr 26 2017, 5:35 PM

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!

http://jw:8080/job/phabricator/894/ for more details.

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!

http://jw:8080/job/phabricator/895/ for more details.

bb added a subscriber: bb.Apr 27 2017, 1:01 PM

Wouldn't it be better to reform the category element into a "list" so multiple categories can be added? and we check for MatchesClassList() on the "Dock" (and other) checks.

In D387#15785, @bb wrote:

Wouldn't it be better to reform the category element into a "list" so multiple categories can be added? and we check for MatchesClassList() on the "Dock" (and other) checks.

I had in mind to indeed move the Special entry to a token one. Easily extendable from that diff.
For the category one, that was what I had first in mind (see the ticket description) but it hardly match with the EntityLimit component imo.
So I was for a unique category but a list of special (hardcoded) constraints.

bb added a comment.Apr 27 2017, 1:17 PM

When we only allow a list of categories (no things with !Foo), the GetCategory function could then return an array, which could be looped over in the EntityLimits, doesn't seem to break anything there. We probably also need to change the trainingsRestriction.template.Category too in an array. Bet changing that component would be nice anyway for consistency.

In D387#15791, @bb wrote:

When we only allow a list of categories (no things with !Foo), the GetCategory function could then return an array, which could be looped over in the EntityLimits, doesn't seem to break anything there. We probably also need to change the trainingsRestriction.template.Category too in an array. Bet changing that component would be nice anyway for consistency.

Yes, I get what you say.
What leads me to split is that we should not mix the Category which is used for entity limits and the Special hardcoded constraints.
Else one should take care for example to not use one of those in Category to define an entity limit for example (and some other confusing things...)
Thanks for sharing thoughts!

leper requested changes to this revision.May 4 2017, 12:28 AM
leper added a subscriber: leper.
leper added inline comments.
binaries/data/mods/public/simulation/components/BuildRestrictions.js
40 ↗(On Diff #1494)

Could we use a choice for this one?

Since all available choices for this are likely to require code additions I don't think using text here is worth it. (Especially since one of those can be checked easily, while the other one cannot.)

Also if we make this some zeroOrMore thing we might make this non-optional (since optionals tend to make errors harder to decipher), but in this case optional doesn't seem that bad.

229 ↗(On Diff #1494)

if that was non-optional we wouldn't need this additional check. Also using text above doesn't work that well if we compare the whole thing here.

This revision now requires changes to proceed.May 4 2017, 12:28 AM

Thanks for the comments/advice. I'll do that.

fatherbushido added inline comments.May 5 2017, 8:55 PM
binaries/data/mods/public/simulation/components/BuildRestrictions.js
40 ↗(On Diff #1494)

I hesistate between make
"<zeroOrMore>"+

	  "<choice>"+
	     "<element name='Dock'>"+
	       "<empty/>"+
	    "</element>"+
	  "</choice>"+

"</zeroOrMore>"

and

"<element name='Special' a:help='Specifies special constraints.'>" +

		"<list>" +
			"<zeroOrMore>" +
				"<choice>" +
					"<value>Dock</value>" +
				"</choice>" +
			"</zeroOrMore>" +
		"</list>" +

"</element>"

fatherbushido edited edge metadata.

Use choice instead of text as adviced by leper. In that version elements are used but perhaps values are better (but would require to manipulate text/array). The special level remains mainly for readability but could be removed.

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!

http://jw:8080/job/phabricator/1121/ for more details.

leper requested changes to this revision.May 13 2017, 2:25 AM
leper added inline comments.
binaries/data/mods/public/simulation/components/BuildRestrictions.js
229 ↗(On Diff #1494)

This should most likely be this.template.Special.Dock.

Trying to place eg a house somewhere valid results in the following errors

ERROR: JavaScript error: simulation/components/BuildRestrictions.js line 233 TypeError: invalid 'in' operand this.template.Special BuildRestrictions.prototype.CheckPlacement@simulation/components/BuildRestrictions.js:233:1 GuiInterface.prototype.SetBuildingPlacementPreview@simulation/components/GuiInterface.js:1095:13 GuiInterface.prototype.ScriptCall@simulation/components/GuiInterface.js:2014:10 updateBuildingPlacementPreview@gui/session/input.js:101:17 handleInputAfterGui@gui/session/input.js:1072:4

ERROR: Error calling component script function ScriptCall

ERROR: JavaScript error: gui/session/input.js line 110 TypeError: result is undefined updateBuildingPlacementPreview@gui/session/input.js:110:36 handleInputAfterGui@gui/session/input.js:1072:4

Trying to place it somewhere invalid works though. Also placing something like a dock works.

This revision now requires changes to proceed.May 13 2017, 2:25 AM
fatherbushido edited edge metadata.

Element without sub element is parsed as undefined

fatherbushido added inline comments.May 13 2017, 8:23 AM
binaries/data/mods/public/simulation/components/BuildRestrictions.js
229 ↗(On Diff #1494)

Thanks!
The matter is that I work with element so as it's with </empty>, we need more an in check (or something like that).
Moreover with <Special/> we still need the this.template.Special check.
I wonder if we could just let it optional in that case or remove the Special level and move Dock at that level, or something else.

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!

http://jw:8080/job/phabricator/1178/ for more details.

s0600204 requested changes to this revision.May 15 2017, 10:00 PM
s0600204 added a subscriber: s0600204.

I note that with this applied, the lighthouse no longer rotates automatically to align itself with the shore. Neither do the brit crannog or the cart "super" dock,

This appears to be controlled in the GuiInterface component, line 1686.

binaries/data/mods/public/simulation/templates/template_structure_military_dock.xml
6 ↗(On Diff #1893)

As there's no limit on the number of docks players can build, is there any point in keeping the Dock category?

Alternatively, if you do wish to keep it, why remove it from the cart "super" dock, which is also a dock.

This revision now requires changes to proceed.May 15 2017, 10:00 PM

I note that with this applied, the lighthouse no longer rotates automatically to align itself with the shore. Neither do the brit crannog or the cart "super" dock,

This appears to be controlled in the GuiInterface component, line 1686.

thx for noticing

binaries/data/mods/public/simulation/templates/template_structure_military_dock.xml
6 ↗(On Diff #1893)

In case perhaps.
I remove it from the cart super dock as I guess it's not the same entity limit restriction.

fatherbushido edited edge metadata.

Add missing changes in GuiInterface noticed by s0...

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!

http://jw:8080/job/phabricator/1242/ for more details.

Giving it some thought, I notice there is one thing that the four templates have in common that no other template in 0ad has - they all have a BuildRestrictions/PlacementType value of shore. Would it perhaps be possible to check for that instead of adding a new bit to the entity definition? (Might be its not possible. or is a terrible idea, I'm just a-wondering.)

Otherwise, thanks for getting the auto-orientation working again.

Giving it some thought, I notice there is one thing that the four templates have in common that no other template in 0ad has - they all have a BuildRestrictions/PlacementType value of shore. Would it perhaps be possible to check for that instead of adding a new bit to the entity definition? (Might be its not possible. or is a terrible idea, I'm just a-wondering.)

Otherwise, thanks for getting the auto-orientation working again.

Indeed it would do the job. But I am not certain that those two things are strictly equals (looking at the TODO for example) and if some other special placement constraints could be added.

Indeed it would do the job. But I am not certain that those two things are strictly equals (looking at the TODO for example) and if some other special placement constraints could be added.

Fair enough, it was just a thought. Maybe the new restriction should be named something like "shoreline" as it doesn't just affect docks?

Giving it some thought, I notice there is one thing that the four templates have in common that no other template in 0ad has - they all have a BuildRestrictions/PlacementType value of shore. Would it perhaps be possible to check for that instead of adding a new bit to the entity definition? (Might be its not possible. or is a terrible idea, I'm just a-wondering.)

Indeed it would do the job. But I am not certain that those two things are strictly equals (looking at the TODO for example) and if some other special placement constraints could be added.

Well other special constraints will have to do some work later on anyway. I'm not sure if linking it to the PlacementType is a good idea, but it does seem quite tempting (especially given that everything with that placement type sort of needs that (unless I'm missing some obvious counter-example, which is quite possible)). Considering what would happen if we went for that could help with figuring out if that would come back to haunt us, or if we'll regret not doing this.

fatherbushido planned changes to this revision.May 19 2017, 8:27 PM

Yes, we need to imagine that.

Btw, I forgot to look at the AI.

(There is also such a Dock check in Commands.js)

Finally choose to use placement type.

It's really simpler and should handle many situtations.
@mimo had the same idea recently.
I let the Category 'Dock' for carth super dock (currently there are no entity limit).

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

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 149| »   »   switch·(ret)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 267| »   »   »   var·cmpIdentity·=·Engine.QueryInterface(id,·IID_Identity);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'cmpIdentity' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 127| »   case·"land":
|    | [NORMAL] JSHintBear:
|    | Expected a 'break' statement before 'default'.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 144| »   »   var·ret·=·cmpObstruction.CheckFoundation(passClassName,·false);
|    | [NORMAL] JSHintBear:
|    | 'ret' is already defined.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 167| »   var·cmpPlayer·=·QueryOwnerInterface(this.entity,·IID_Player);
|    | [NORMAL] JSHintBear:
|    | 'cmpPlayer' is already defined.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 177| »   var·isNeutral·=·tileOwner·==·0;
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 251| »   »   »   ||·(cmpWaterManager.GetWaterLevel(pos.x·-·sz,·pos.y·-·cz)·-·cmpTerrain.GetGroundLevel(pos.x·-·sz,·pos.y·-·cz))·>·2.0)·//·back
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 261| »   »   var·cmpRangeManager·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_RangeManager);
|    | [NORMAL] JSHintBear:
|    | 'cmpRangeManager' is already defined.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 262| »   »   var·cmpPlayer·=·QueryOwnerInterface(this.entity,·IID_Player);
|    | [NORMAL] JSHintBear:
|    | 'cmpPlayer' is already defined.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 277| »   »   »   »   var·result·=·markForPluralTranslation(
|    | [NORMAL] JSHintBear:
|    | 'result' is already defined.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 295| »   »   »   var·dist·=·+this.template.Distance.MaxDistance;
|    | [NORMAL] JSHintBear:
|    | 'dist' is already defined.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 296| »   »   »   var·nearEnts·=·cmpRangeManager.ExecuteQuery(this.entity,·0,·dist,·[cmpPlayer.GetPlayerID()],·IID_BuildRestrictions).filter(filter);
|    | [NORMAL] JSHintBear:
|    | 'nearEnts' is already defined.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 299| »   »   »   »   var·result·=·markForPluralTranslation(
|    | [NORMAL] JSHintBear:
|    | 'result' is already defined.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 147| »   if·(ret·!=·"success")
|    | [NORMAL] JSHintBear:
|    | 'ret' used out of scope.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 149| »   »   switch·(ret)
|    | [NORMAL] JSHintBear:
|    | 'ret' used out of scope.

binaries/data/mods/public/simulation/helpers/Commands.js
| 882| »   var·ids·=·[·id·for·(id·in·members)·];
|    | [MAJOR] ESLintBear:
|    | Parsing error: Unexpected token for

binaries/data/mods/public/simulation/helpers/Commands.js
|  53| var·g_Commands·=·{
|    | [NORMAL] JSHintBear:
|    | 'g_Commands' was used before it was defined.

binaries/data/mods/public/simulation/helpers/Commands.js
| 175| »   »   let·allowCapture·=·cmd.allowCapture·||·cmd.allowCapture·==·null;
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with 'null'.

binaries/data/mods/public/simulation/helpers/Commands.js
| 499| »   »   if·(notUngarrisoned·!=·0)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/simulation/helpers/Commands.js
| 513| »   »   »   »   ····&&·player·!=·+cmd.owner)
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/helpers/Commands.js
| 701| »   »   »   »   var·cmpGUIInterface·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_GuiInterface);
|    | [NORMAL] JSHintBear:
|    | 'cmpGUIInterface' is already defined.

binaries/data/mods/public/simulation/helpers/Commands.js
| 882| »   var·ids·=·[·id·for·(id·in·members)·];
|    | [NORMAL] JSHintBear:
|    | 'array comprehension' is only available in Mozilla JavaScript extensions (use moz option).

binaries/data/mods/public/simulation/helpers/Commands.js
| 882| »   var·ids·=·[·id·for·(id·in·members)·];
|    | [NORMAL] JSHintBear:
|    | Expected 'for' and instead saw 'id'.

binaries/data/mods/public/simulation/helpers/Commands.js
| 932| »   »   for·(var·i·=·0;·i·<·length;·++i)
|    | [NORMAL] JSHintBear:
|    | 'i' is already defined.

binaries/data/mods/public/simulation/helpers/Commands.js
| 945| »   »   var·count·=·0;
|    | [NORMAL] JSHintBear:
|    | 'count' is already defined.

binaries/data/mods/public/simulation/helpers/Commands.js
|1092| »   »   var·cmpGuiInterface·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_GuiInterface);
|    | [NORMAL] JSHintBear:
|    | 'cmpGuiInterface' is already defined.

binaries/data/mods/public/simulation/helpers/Commands.js
|1256| »   »   »   if·(!(i·==·0·&&·piece.template·==·cmd.wallSet.templates.tower·&&·!cmd.startSnappedEntity))
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/helpers/Commands.js
|1344| »   »   var·piece·=·pieces[j];
|    | [NORMAL] JSHintBear:
|    | 'piece' is already defined.

binaries/data/mods/public/simulation/helpers/Commands.js
|1373| »   »   »   »   if·(lastTowerControlGroup·!=·null·&&·lastTowerControlGroup·!=·INVALID_ENTITY)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'null'.

binaries/data/mods/public/simulation/helpers/Commands.js
|1428| »   »   var·cmpUnitAI·=·Engine.QueryInterface(ent,·IID_UnitAI);
|    | [NORMAL] JSHintBear:
|    | 'cmpUnitAI' is already defined.

binaries/data/mods/public/simulation/helpers/Commands.js
|1451| »   if·(formedEnts.length·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/helpers/Commands.js
|1468| »   »   »   &&·cmpFormation.GetMemberCount()·==·formation.entities.length)
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/helpers/Commands.js
|1483| »   »   for·(var·fid·in·formation.members)
|    | [NORMAL] JSHintBear:
|    | 'fid' is already defined.

binaries/data/mods/public/simulation/helpers/Commands.js
|1485| »   »   »   var·cmpFormation·=·Engine.QueryInterface(+fid,·IID_Formation);
|    | [NORMAL] JSHintBear:
|    | 'cmpFormation' is already defined.

binaries/data/mods/public/simulation/helpers/Commands.js
|1502| »   »   »   »   »   var·cmpUnitAI·=·Engine.QueryInterface(ent,·IID_UnitAI);
|    | [NORMAL] JSHintBear:
|    | 'cmpUnitAI' is already defined.

binaries/data/mods/public/simulation/helpers/Commands.js
|1525| »   »   »   var·cmpFormation·=·Engine.QueryInterface(formationEnt,·IID_Formation);
|    | [NORMAL] JSHintBear:
|    | 'cmpFormation' is already defined.
|    | [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[play

http://jw:8080/job/phabricator_lint/218/ for more details.

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://jw:8080/job/phabricator/1595/ for more details.

mimo added a comment.Jun 21 2017, 3:01 PM

Your latest patch (which looks good to me) should be completed by that one

Merge mimo's patch. Thanks and sorry for forgetting Petra.

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://jw:8080/job/phabricator/1603/ for more details.

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

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 149| »   »   switch·(ret)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 267| »   »   »   var·cmpIdentity·=·Engine.QueryInterface(id,·IID_Identity);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'cmpIdentity' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 127| »   case·"land":
|    | [NORMAL] JSHintBear:
|    | Expected a 'break' statement before 'default'.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 144| »   »   var·ret·=·cmpObstruction.CheckFoundation(passClassName,·false);
|    | [NORMAL] JSHintBear:
|    | 'ret' is already defined.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 167| »   var·cmpPlayer·=·QueryOwnerInterface(this.entity,·IID_Player);
|    | [NORMAL] JSHintBear:
|    | 'cmpPlayer' is already defined.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 177| »   var·isNeutral·=·tileOwner·==·0;
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 251| »   »   »   ||·(cmpWaterManager.GetWaterLevel(pos.x·-·sz,·pos.y·-·cz)·-·cmpTerrain.GetGroundLevel(pos.x·-·sz,·pos.y·-·cz))·>·2.0)·//·back
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 261| »   »   var·cmpRangeManager·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_RangeManager);
|    | [NORMAL] JSHintBear:
|    | 'cmpRangeManager' is already defined.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 262| »   »   var·cmpPlayer·=·QueryOwnerInterface(this.entity,·IID_Player);
|    | [NORMAL] JSHintBear:
|    | 'cmpPlayer' is already defined.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 277| »   »   »   »   var·result·=·markForPluralTranslation(
|    | [NORMAL] JSHintBear:
|    | 'result' is already defined.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 295| »   »   »   var·dist·=·+this.template.Distance.MaxDistance;
|    | [NORMAL] JSHintBear:
|    | 'dist' is already defined.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 296| »   »   »   var·nearEnts·=·cmpRangeManager.ExecuteQuery(this.entity,·0,·dist,·[cmpPlayer.GetPlayerID()],·IID_BuildRestrictions).filter(filter);
|    | [NORMAL] JSHintBear:
|    | 'nearEnts' is already defined.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 299| »   »   »   »   var·result·=·markForPluralTranslation(
|    | [NORMAL] JSHintBear:
|    | 'result' is already defined.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 147| »   if·(ret·!=·"success")
|    | [NORMAL] JSHintBear:
|    | 'ret' used out of scope.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
| 149| »   »   switch·(ret)
|    | [NORMAL] JSHintBear:
|    | 'ret' used out of scope.

binaries/data/mods/public/simulation/helpers/Commands.js
| 882| »   var·ids·=·[·id·for·(id·in·members)·];
|    | [MAJOR] ESLintBear:
|    | Parsing error: Unexpected token for

binaries/data/mods/public/simulation/helpers/Commands.js
|  53| var·g_Commands·=·{
|    | [NORMAL] JSHintBear:
|    | 'g_Commands' was used before it was defined.

binaries/data/mods/public/simulation/helpers/Commands.js
| 175| »   »   let·allowCapture·=·cmd.allowCapture·||·cmd.allowCapture·==·null;
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with 'null'.

binaries/data/mods/public/simulation/helpers/Commands.js
| 499| »   »   if·(notUngarrisoned·!=·0)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/simulation/helpers/Commands.js
| 513| »   »   »   »   ····&&·player·!=·+cmd.owner)
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/helpers/Commands.js
| 701| »   »   »   »   var·cmpGUIInterface·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_GuiInterface);
|    | [NORMAL] JSHintBear:
|    | 'cmpGUIInterface' is already defined.

binaries/data/mods/public/simulation/helpers/Commands.js
| 882| »   var·ids·=·[·id·for·(id·in·members)·];
|    | [NORMAL] JSHintBear:
|    | 'array comprehension' is only available in Mozilla JavaScript extensions (use moz option).

binaries/data/mods/public/simulation/helpers/Commands.js
| 882| »   var·ids·=·[·id·for·(id·in·members)·];
|    | [NORMAL] JSHintBear:
|    | Expected 'for' and instead saw 'id'.

binaries/data/mods/public/simulation/helpers/Commands.js
| 932| »   »   for·(var·i·=·0;·i·<·length;·++i)
|    | [NORMAL] JSHintBear:
|    | 'i' is already defined.

binaries/data/mods/public/simulation/helpers/Commands.js
| 945| »   »   var·count·=·0;
|    | [NORMAL] JSHintBear:
|    | 'count' is already defined.

binaries/data/mods/public/simulation/helpers/Commands.js
|1092| »   »   var·cmpGuiInterface·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_GuiInterface);
|    | [NORMAL] JSHintBear:
|    | 'cmpGuiInterface' is already defined.

binaries/data/mods/public/simulation/helpers/Commands.js
|1256| »   »   »   if·(!(i·==·0·&&·piece.template·==·cmd.wallSet.templates.tower·&&·!cmd.startSnappedEntity))
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/helpers/Commands.js
|1344| »   »   var·piece·=·pieces[j];
|    | [NORMAL] JSHintBear:
|    | 'piece' is already defined.

binaries/data/mods/public/simulation/helpers/Commands.js
|1373| »   »   »   »   if·(lastTowerControlGroup·!=·null·&&·lastTowerControlGroup·!=·INVALID_ENTITY)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'null'.

binaries/data/mods/public/simulation/helpers/Commands.js
|1428| »   »   var·cmpUnitAI·=·Engine.QueryInterface(ent,·IID_UnitAI);
|    | [NORMAL] JSHintBear:
|    | 'cmpUnitAI' is already defined.

binaries/data/mods/public/simulation/helpers/Commands.js
|1451| »   if·(formedEnts.length·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/helpers/Commands.js
|1468| »   »   »   &&·cmpFormation.GetMemberCount()·==·formation.entities.length)
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/helpers/Commands.js
|1483| »   »   for·(var·fid·in·formation.members)
|    | [NORMAL] JSHintBear:
|    | 'fid' is already defined.

binaries/data/mods/public/simulation/helpers/Commands.js
|1485| »   »   »   var·cmpFormation·=·Engine.QueryInterface(+fid,·IID_Formation);
|    | [NORMAL] JSHintBear:
|    | 'cmpFormation' is already defined.

binaries/data/mods/public/simulation/helpers/Commands.js
|1502| »   »   »   »   »   var·cmpUnitAI·=·Engine.QueryInterface(ent,·IID_UnitAI);
|    | [NORMAL] JSHintBear:
|    | 'cmpUnitAI' is already defined.

binaries/data/mods/public/simulation/helpers/Commands.js
|1525| »   »   »   var·cmpFormation·=·Engine.QueryInterface(formationEnt,·IID_Formation);
|    | [NORMAL] JSHintBear:
|    | 'cmpFormation' is already defined.
|    | [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[play

http://jw:8080/job/phabricator_lint/223/ for more details.

mimo accepted this revision.Jun 26 2017, 8:43 PM

patch is good, and testing on a naval ai game, it gives the same hash value.

This revision was automatically updated to reflect the committed changes.