Page MenuHomeWildfire Games

Fix an issue with Upgrade and training restriction.
ClosedPublic

Authored by fatherbushido on Oct 13 2017, 6:19 PM.

Details

Reviewers
wraitii
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP20372: Fix an issue with upgrade and entity limits.
Summary

rP18467 introduced the Upgrade component and a Transform helper.
rP18754 uses it for a structure upgrade.
rP19095 uses it for a unit upgrade.
rP19103 revert the unit upgrade as it didn't work.
rP19195 adress the bug of the upgrade component related to those limit restriction (noticeable for the structure upgrade)

There is still a Command check (for unit, not for structure) which is adressed here.

I am not really proud of the patch as it leaves me the feeling to stack things over things. But it adresses the bug.

Test Plan

You can test it against D961

Event Timeline

fatherbushido created this revision.Oct 13 2017, 6:19 PM
fatherbushido added inline comments.
binaries/data/mods/public/simulation/helpers/Commands.js
727

Note:
For the origin entity, we can also use the template to get the category.

Vulcan added a subscriber: Vulcan.Oct 13 2017, 7:07 PM

Build is green

Updating workspaces...
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimation]’:
FCollada/FCDocument/FCDLibrary.cpp:149:30:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
  const T* cptr = ((const FCDLibrary<T>*)l1)->GetEntity(0);
           ^
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimationClip]’:
FCollada/FCDocument/FCDLibrary.cpp:150:34:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDCamera]’:
FCollada/FCDocument/FCDLibrary.cpp:151:27:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDController]’:
FCollada/FCDocument/FCDLibrary.cpp:152:31:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEffect]’:
FCollada/FCDocument/FCDLibrary.cpp:153:27:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEmitter]’:
FCollada/FCDocument/FCDLibrary.cpp:154:28:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDForceField]’:
FCollada/FCDocument/FCDLibrary.cpp:155:31:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDGeometry]’:
FCollada/FCDocument/FCDLibrary.cpp:156:29:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDImage]’:
FCollada/FCDocument/FCDLibrary.cpp:157:26:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDLight]’:
FCollada/FCDocument/FCDLibrary.cpp:158:26:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDMaterial]’:
FCollada/FCDocument/FCDLibrary.cpp:159:29:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDSceneNode]’:
FCollada/FCDocument/FCDLibrary.cpp:160:30:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDPhysicsModel]’:
FCollada/FCDocument/FCDLibrary.cpp:161:33:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDPhysicsMaterial]’:
FCollada/FCDocument/FCDLibrary.cpp:162:36:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDPhysicsScene]’:
FCollada/FCDocument/FCDLibrary.cpp:163:33:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
In file included from FCollada/FUtils/FUSemaphore.cpp:10:0:
FCollada/FUtils/FUSemaphore.h:36:2: warning: #warning "FUSemaphore: Semaphore not implemented for non Windows" [-Wcpp]
 #warning "FUSemaphore: Semaphore not implemented for non Windows"
  ^
FCollada/FUtils/FUStringConversion.cpp: In function ‘void TrickLinkerFUStringConversion()’:
FCollada/FUtils/FUStringConversion.cpp:281:8: warning: variable ‘f’ set but not used [-Wunused-but-set-variable]
  float f = FUStringConversion::ToFloat(&c);
        ^
FCollada/FUtils/FUStringConversion.cpp:283:7: warning: variable ‘b’ set but not used [-Wunused-but-set-variable]
  bool b = FUStringConversion::ToBoolean(c);
       ^
FCollada/FUtils/FUStringConversion.cpp:285:8: warning: variable ‘i32’ set but not used [-Wunused-but-set-variable]
  int32 i32 = FUStringConversion::ToInt32(&c);
        ^
FCollada/FUtils/FUStringConversion.cpp:287:9: warning: variable ‘u32’ set but not used [-Wunused-but-set-variable]
  uint32 u32 = FUStringConversion::ToUInt32(&c);
         ^
In file included from FCollada/FUtils/FUThread.cpp:10:0:
FCollada/FUtils/FUThread.h:30:2: warning: #warning "Threads not yet implemented for non Windows." [-Wcpp]
 #warning "Threads not yet implemented for non Windows."
  ^
FCollada/FCDocument/FCDAnimationCurve.cpp: In member function ‘float FCDAnimationCurve::Evaluate(float) const’:
FCollada/FCDocument/FCDAnimationCurve.cpp:396:13: warning: ‘inTangent.FMVector2::<anonymous>.FMVector2::<anonymous union>::x’ may be used uninitialized in this function [-Wmaybe-uninitialized]
   FMVector2 inTangent;
             ^
FCollada/FCDocument/FCDAnimationCurve.cpp:396:13: warning: ‘inTangent.FMVector2::<anonymous>.FMVector2::<anonymous union>::y’ may be used uninitialized in this function [-Wmaybe-uninitialized]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimation]’:
FCollada/FCDocument/FCDLibrary.cpp:149:30:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
  const T* cptr = ((const FCDLibrary<T>*)l1)->GetEntity(0);
           ^
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimationClip]’:
FCollada/FCDocument/FCDLibrary.cpp:150:34:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDCamera]’:
FCollada/FCDocument/FCDLibrary.cpp:151:27:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDController]’:
FCollada/FCDocument/FCDLibrary.cpp:152:31:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEffect]’:
FCollada/FCDocument/FCDLibrary.cpp:153:27:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEmitter]’:
FCollada/FCDocument/FCDLibrary.cpp:154:28:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDForceField]’:
FCollada/FCDocument/FC

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

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

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

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

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

binaries/data/mods/public/simulation/helpers/Commands.js
| 528| »   »   »   »   ····&&·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
| 716| »   »   »   »   var·cmpGUIInterface·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_GuiInterface);
|    | [NORMAL] JSHintBear:
|    | 'cmpGUIInterface' is already defined.

binaries/data/mods/public/simulation/helpers/Commands.js
| 730| »   »   »   »   &&·(!cmpTrainingRestrictions·||·!template.TrainingRestrictions.Category·==·cmpTrainingRestrictions.GetCategory())
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/helpers/Commands.js
| 730| »   »   »   »   &&·(!cmpTrainingRestrictions·||·!template.TrainingRestrictions.Category·==·cmpTrainingRestrictions.GetCategory())
|    | [NORMAL] JSHintBear:
|    | Confusing use of '!'.

binaries/data/mods/public/simulation/helpers/Commands.js
| 731| »   »   »   »   &&·!cmpEntityLimits.AllowedToTrain(template.TrainingRestrictions.Category,·1)
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/helpers/Commands.js
| 732| »   »   »   »   ||·template.BuildRestrictions·&&·!cmpEntityLimits.AllowedToBuild(template.BuildRestrictions.Category))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/helpers/Commands.js
| 907| »   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
| 907| »   var·ids·=·[·id·for·(id·in·members)·];
|    | [NORMAL] JSHintBear:
|    | Expected 'for' and instead saw 'id'.

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

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

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

binaries/data/mods/public/simulation/helpers/Commands.js
|1281| »   »   »   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
|1369| »   »   var·piece·=·pieces[j];
|    | [NORMAL] JSHintBear:
|    | 'piece' is already defined.

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

binaries/data/mods/public/simulation/helpers/Commands.js
|1453| »   »   var·cmpUnitAI·=·Engine.QueryInterface(ent,·IID_UnitAI);
|    |

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

Wouldn't it end up being easier to add an "AllowedToReplace" function to the entity limits component?

Your idea sounds ok.
(Basically move that part in the EntityLimits component)

fatherbushido added a comment.EditedOct 19 2017, 6:16 PM

@wraitii :
I just noticed that the check for BuildRestriction seems not being strong enough (exact opposite of training restriction).

fatherbushido added inline comments.Oct 19 2017, 6:43 PM
binaries/data/mods/public/simulation/helpers/Commands.js
727

but there is a gui check before (and iirc a sim check after)

bb added a subscriber: bb.Oct 24 2017, 9:20 PM
bb added inline comments.
binaries/data/mods/public/simulation/helpers/Commands.js
727

The class can (in future) change by a tech or so, so rather get the sim value

728

!foo == baz equals foo != baz

730

why don't we do the same here. If you understand the problem correctly we have a bug when we upgrade a unit to another with the same restrictionclass, and that could happen with buildings too (f.e. wonder upgrade)

leper added inline comments.
binaries/data/mods/public/simulation/helpers/Commands.js
728

No it does not. ! has higher precedence than == or !=.

This however looks broken regardless of that.

fatherbushido added inline comments.Oct 24 2017, 10:28 PM
binaries/data/mods/public/simulation/helpers/Commands.js
727

Same preference but I hope that would never happen ("The class can (in future) change by a tech" -> messy) :p

728

indeed :p

730

'If you understand the problem correctly'

me or you?

fatherbushido planned changes to this revision.Oct 24 2017, 10:31 PM

(don't review that, I will edit that thing)

bb added inline comments.Oct 24 2017, 10:33 PM
binaries/data/mods/public/simulation/helpers/Commands.js
727

true but never disallow when it that easy to allow here

730

you => I (so refering to bb)

fatherbushido edited the summary of this revision. (Show Details)

Fix mistakes. Use a function.

Update

  • use a function in EntityLimits component. We could let all that in Commands too.
  • When we have training restriction:
    • check with +1 count, if we have a different category (same without the patch)
    • check with +0 count, if we have the same category (not the same as without the patch) (*)
  • When we have building restriction:
    • check with +1 count, if we have a different category (not the same as without the patch) (**)
    • check with +0 count, if we have the same category (same as without the patch)

I guess (**) wasn't noticed as the gui did a previous check.

Build is green

Updating workspaces...
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimation]’:
FCollada/FCDocument/FCDLibrary.cpp:149:30:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
  const T* cptr = ((const FCDLibrary<T>*)l1)->GetEntity(0);
           ^
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimationClip]’:
FCollada/FCDocument/FCDLibrary.cpp:150:34:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDCamera]’:
FCollada/FCDocument/FCDLibrary.cpp:151:27:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDController]’:
FCollada/FCDocument/FCDLibrary.cpp:152:31:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEffect]’:
FCollada/FCDocument/FCDLibrary.cpp:153:27:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEmitter]’:
FCollada/FCDocument/FCDLibrary.cpp:154:28:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDForceField]’:
FCollada/FCDocument/FCDLibrary.cpp:155:31:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDGeometry]’:
FCollada/FCDocument/FCDLibrary.cpp:156:29:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDImage]’:
FCollada/FCDocument/FCDLibrary.cpp:157:26:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDLight]’:
FCollada/FCDocument/FCDLibrary.cpp:158:26:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDMaterial]’:
FCollada/FCDocument/FCDLibrary.cpp:159:29:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDSceneNode]’:
FCollada/FCDocument/FCDLibrary.cpp:160:30:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDPhysicsModel]’:
FCollada/FCDocument/FCDLibrary.cpp:161:33:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDPhysicsMaterial]’:
FCollada/FCDocument/FCDLibrary.cpp:162:36:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ s

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

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

binaries/data/mods/public/simulation/components/EntityLimits.js
|  89| »   »   »   for·(var·c·in·this.template.LimitRemovers[category])
|    | [NORMAL] JSHintBear:
|    | 'c' is already defined.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 124| »   »   if·("RequiredTechs"·in·this.removers[category]·&&·this.removers[category]["RequiredTechs"].indexOf(tech)·!==·-1)
|    | [NORMAL] JSHintBear:
|    | ['RequiredTechs'] is better written in dot notation.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 125| »   »   »   this.removers[category]["RequiredTechs"].splice(this.removers[category]["RequiredTechs"].indexOf(tech),·1);
|    | [NORMAL] JSHintBear:
|    | ['RequiredTechs'] is better written in dot notation.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 125| »   »   »   this.removers[category]["RequiredTechs"].splice(this.removers[category]["RequiredTechs"].indexOf(tech),·1);
|    | [NORMAL] JSHintBear:
|    | ['RequiredTechs'] is better written in dot notation.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 136| »   »   »   nolimit·=·!this.removers[category]["RequiredTechs"].length;
|    | [NORMAL] JSHintBear:
|    | ['RequiredTechs'] is better written in dot notation.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 138| »   »   »   for·(var·cls·of·this.removers[category]["RequiredClasses"])
|    | [NORMAL] JSHintBear:
|    | ['RequiredClasses'] is better written in dot notation.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 235| »   »   var·modifier·=·1;
|    | [NORMAL] JSHintBear:
|    | 'modifier' is already defined.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 260| »   for·(var·category·in·this.changers)
|    | [NORMAL] JSHintBear:
|    | 'category' is already defined.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 264| »   »   »   »   if·(this.limit[category]·!=·undefined)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'undefined'.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 266| »   »   »   »   if·(this.removedLimit[category]·!=·undefined)»   //·update·removed·limits·in·case·we·want·to·restore·it
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'undefined'.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 270| »   for·(var·category·in·this.removers)
|    | [NORMAL] JSHintBear:
|    | 'category' is already defined.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 272| »   »   »   for·(var·cls·of·this.removers[category]["RequiredClasses"])
|    | [NORMAL] JSHintBear:
|    | ['RequiredClasses'] is better written in dot notation.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 248| »   »   this.ChangeCount(category,·modifier);
|    | [NORMAL] JSHintBear:
|    | 'modifier' used out of scope.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 265| »   »   »   »   »   this.limit[category]·+=·modifier·*·this.changers[category][c];
|    | [NORMAL] JSHintBear:
|    | 'modifier' used out of scope.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 267| »   »   »   »   »   this.removedLimit[category]·+=·modifier·*·this.changers[category][c];
|    | [NORMAL] JSHintBear:
|    | 'modifier' used out of scope.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 274| »   »   »   »   »   this.classCount[cls]·+=·modifier;
|    | [NORMAL] JSHintBear:
|    | 'modifier' used out of scope.

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

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

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

Build is green

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

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

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

binaries/data/mods/public/simulation/components/EntityLimits.js
|  89| »   »   »   for·(var·c·in·this.template.LimitRemovers[category])
|    | [NORMAL] JSHintBear:
|    | 'c' is already defined.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 124| »   »   if·("RequiredTechs"·in·this.removers[category]·&&·this.removers[category]["RequiredTechs"].indexOf(tech)·!==·-1)
|    | [NORMAL] JSHintBear:
|    | ['RequiredTechs'] is better written in dot notation.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 125| »   »   »   this.removers[category]["RequiredTechs"].splice(this.removers[category]["RequiredTechs"].indexOf(tech),·1);
|    | [NORMAL] JSHintBear:
|    | ['RequiredTechs'] is better written in dot notation.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 125| »   »   »   this.removers[category]["RequiredTechs"].splice(this.removers[category]["RequiredTechs"].indexOf(tech),·1);
|    | [NORMAL] JSHintBear:
|    | ['RequiredTechs'] is better written in dot notation.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 136| »   »   »   nolimit·=·!this.removers[category]["RequiredTechs"].length;
|    | [NORMAL] JSHintBear:
|    | ['RequiredTechs'] is better written in dot notation.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 138| »   »   »   for·(var·cls·of·this.removers[category]["RequiredClasses"])
|    | [NORMAL] JSHintBear:
|    | ['RequiredClasses'] is better written in dot notation.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 235| »   »   var·modifier·=·1;
|    | [NORMAL] JSHintBear:
|    | 'modifier' is already defined.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 260| »   for·(var·category·in·this.changers)
|    | [NORMAL] JSHintBear:
|    | 'category' is already defined.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 264| »   »   »   »   if·(this.limit[category]·!=·undefined)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'undefined'.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 266| »   »   »   »   if·(this.removedLimit[category]·!=·undefined)»   //·update·removed·limits·in·case·we·want·to·restore·it
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'undefined'.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 270| »   for·(var·category·in·this.removers)
|    | [NORMAL] JSHintBear:
|    | 'category' is already defined.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 272| »   »   »   for·(var·cls·of·this.removers[category]["RequiredClasses"])
|    | [NORMAL] JSHintBear:
|    | ['RequiredClasses'] is better written in dot notation.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 248| »   »   this.ChangeCount(category,·modifier);
|    | [NORMAL] JSHintBear:
|    | 'modifier' used out of scope.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 265| »   »   »   »   »   this.limit[category]·+=·modifier·*·this.changers[category][c];
|    | [NORMAL] JSHintBear:
|    | 'modifier' used out of scope.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 267| »   »   »   »   »   this.removedLimit[category]·+=·modifier·*·this.changers[category][c];
|    | [NORMAL] JSHintBear:
|    | 'modifier' used out of scope.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 274| »   »   »   »   »   this.classCount[cls]·+=·modifier;
|    | [NORMAL] JSHintBear:
|    | 'modifier' used out of scope.

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

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

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

Much neater this way for me. See questions above (could be I just don't know the code), otherwise good to go

binaries/data/mods/public/simulation/components/EntityLimits.js
210

Why different functions?

217

Why different functions?

fatherbushido added inline comments.Oct 27 2017, 6:59 PM
binaries/data/mods/public/simulation/components/EntityLimits.js
210

typo

217

typo (the same :p)

wraitii's comments

leper added inline comments.Oct 27 2017, 7:24 PM
binaries/data/mods/public/simulation/components/EntityLimits.js
200

Pointless blank line.

203

I think I commented about it maybe being useful to provide one function that does this, but IIRC last time I looked at it this only occured once or twice. Might be worth considering in the future.

210

Why not push the ternary into the function call? (Same below.)

leper's remarks

fatherbushido added inline comments.Oct 27 2017, 7:44 PM
binaries/data/mods/public/simulation/components/EntityLimits.js
200

thx

203

I remembered that when writing it ;-)

210

nice!

Build is green

Updating workspaces...
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimation]’:
FCollada/FCDocument/FCDLibrary.cpp:149:30:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
  const T* cptr = ((const FCDLibrary<T>*)l1)->GetEntity(0);
           ^
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimationClip]’:
FCollada/FCDocument/FCDLibrary.cpp:150:34:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDCamera]’:
FCollada/FCDocument/FCDLibrary.cpp:151:27:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDController]’:
FCollada/FCDocument/FCDLibrary.cpp:152:31:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEffect]’:
FCollada/FCDocument/FCDLibrary.cpp:153:27:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEmitter]’:
FCollada/FCDocument/FCDLibrary.cpp:154:28:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDForceField]’:
FCollada/FCDocument/FCDLibrary.cpp:155:31:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDGeometry]’:
FCollada/FCDocument/FCDLibrary.cpp:156:29:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDImage]’:
FCollada/FCDocument/FCDLibrary.cpp:157:26:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDLight]’:
FCollada/FCDocument/FCDLibrary.cpp:158:26:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDMaterial]’:
FCollada/FCDocument/FCDLibrary.cpp:159:29:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDSceneNode]’:
FCollada/FCDocument/FCDLibrary.cpp:160:30:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDPhysicsModel]’:
FCollada/FCDocument/FCDLibrary.cpp:161:33:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDPhysicsMaterial]’:
FCollada/FCDocument/FCDLibrary.cpp:162:36:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ s

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

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

binaries/data/mods/public/simulation/components/EntityLimits.js
|  89| »   »   »   for·(var·c·in·this.template.LimitRemovers[category])
|    | [NORMAL] JSHintBear:
|    | 'c' is already defined.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 124| »   »   if·("RequiredTechs"·in·this.removers[category]·&&·this.removers[category]["RequiredTechs"].indexOf(tech)·!==·-1)
|    | [NORMAL] JSHintBear:
|    | ['RequiredTechs'] is better written in dot notation.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 125| »   »   »   this.removers[category]["RequiredTechs"].splice(this.removers[category]["RequiredTechs"].indexOf(tech),·1);
|    | [NORMAL] JSHintBear:
|    | ['RequiredTechs'] is better written in dot notation.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 125| »   »   »   this.removers[category]["RequiredTechs"].splice(this.removers[category]["RequiredTechs"].indexOf(tech),·1);
|    | [NORMAL] JSHintBear:
|    | ['RequiredTechs'] is better written in dot notation.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 136| »   »   »   nolimit·=·!this.removers[category]["RequiredTechs"].length;
|    | [NORMAL] JSHintBear:
|    | ['RequiredTechs'] is better written in dot notation.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 138| »   »   »   for·(var·cls·of·this.removers[category]["RequiredClasses"])
|    | [NORMAL] JSHintBear:
|    | ['RequiredClasses'] is better written in dot notation.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 235| »   »   var·modifier·=·1;
|    | [NORMAL] JSHintBear:
|    | 'modifier' is already defined.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 260| »   for·(var·category·in·this.changers)
|    | [NORMAL] JSHintBear:
|    | 'category' is already defined.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 264| »   »   »   »   if·(this.limit[category]·!=·undefined)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'undefined'.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 266| »   »   »   »   if·(this.removedLimit[category]·!=·undefined)»   //·update·removed·limits·in·case·we·want·to·restore·it
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'undefined'.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 270| »   for·(var·category·in·this.removers)
|    | [NORMAL] JSHintBear:
|    | 'category' is already defined.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 272| »   »   »   for·(var·cls·of·this.removers[category]["RequiredClasses"])
|    | [NORMAL] JSHintBear:
|    | ['RequiredClasses'] is better written in dot notation.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 248| »   »   this.ChangeCount(category,·modifier);
|    | [NORMAL] JSHintBear:
|    | 'modifier' used out of scope.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 265| »   »   »   »   »   this.limit[category]·+=·modifier·*·this.changers[category][c];
|    | [NORMAL] JSHintBear:
|    | 'modifier' used out of scope.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 267| »   »   »   »   »   this.removedLimit[category]·+=·modifier·*·this.changers[category][c];
|    | [NORMAL] JSHintBear:
|    | 'modifier' used out of scope.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 274| »   »   »   »   »   this.classCount[cls]·+=·modifier;
|    | [NORMAL] JSHintBear:
|    | 'modifier' used out of scope.

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

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

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

Build is green

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

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

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

binaries/data/mods/public/simulation/components/EntityLimits.js
|  89| »   »   »   for·(var·c·in·this.template.LimitRemovers[category])
|    | [NORMAL] JSHintBear:
|    | 'c' is already defined.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 124| »   »   if·("RequiredTechs"·in·this.removers[category]·&&·this.removers[category]["RequiredTechs"].indexOf(tech)·!==·-1)
|    | [NORMAL] JSHintBear:
|    | ['RequiredTechs'] is better written in dot notation.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 125| »   »   »   this.removers[category]["RequiredTechs"].splice(this.removers[category]["RequiredTechs"].indexOf(tech),·1);
|    | [NORMAL] JSHintBear:
|    | ['RequiredTechs'] is better written in dot notation.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 125| »   »   »   this.removers[category]["RequiredTechs"].splice(this.removers[category]["RequiredTechs"].indexOf(tech),·1);
|    | [NORMAL] JSHintBear:
|    | ['RequiredTechs'] is better written in dot notation.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 136| »   »   »   nolimit·=·!this.removers[category]["RequiredTechs"].length;
|    | [NORMAL] JSHintBear:
|    | ['RequiredTechs'] is better written in dot notation.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 138| »   »   »   for·(var·cls·of·this.removers[category]["RequiredClasses"])
|    | [NORMAL] JSHintBear:
|    | ['RequiredClasses'] is better written in dot notation.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 232| »   »   var·modifier·=·1;
|    | [NORMAL] JSHintBear:
|    | 'modifier' is already defined.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 257| »   for·(var·category·in·this.changers)
|    | [NORMAL] JSHintBear:
|    | 'category' is already defined.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 261| »   »   »   »   if·(this.limit[category]·!=·undefined)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'undefined'.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 263| »   »   »   »   if·(this.removedLimit[category]·!=·undefined)»   //·update·removed·limits·in·case·we·want·to·restore·it
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'undefined'.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 267| »   for·(var·category·in·this.removers)
|    | [NORMAL] JSHintBear:
|    | 'category' is already defined.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 269| »   »   »   for·(var·cls·of·this.removers[category]["RequiredClasses"])
|    | [NORMAL] JSHintBear:
|    | ['RequiredClasses'] is better written in dot notation.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 245| »   »   this.ChangeCount(category,·modifier);
|    | [NORMAL] JSHintBear:
|    | 'modifier' used out of scope.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 262| »   »   »   »   »   this.limit[category]·+=·modifier·*·this.changers[category][c];
|    | [NORMAL] JSHintBear:
|    | 'modifier' used out of scope.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 264| »   »   »   »   »   this.removedLimit[category]·+=·modifier·*·this.changers[category][c];
|    | [NORMAL] JSHintBear:
|    | 'modifier' used out of scope.

binaries/data/mods/public/simulation/components/EntityLimits.js
| 271| »   »   »   »   »   this.classCount[cls]·+=·modifier;
|    | [NORMAL] JSHintBear:
|    | 'modifier' used out of scope.

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

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

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

wraitii accepted this revision.Oct 28 2017, 9:45 AM
This revision is now accepted and ready to land.Oct 28 2017, 9:45 AM

(Waiting this night for potential leper's comment.)

elexis added a subscriber: elexis.Oct 28 2017, 11:52 PM

(The reverted upgrade was Boudicca in: https://code.wildfiregames.com/rP19103#change-mIqRvLTEYgy.)
(Thanks for having moved the code from Commands.js to the component!)

binaries/data/mods/public/simulation/components/EntityLimits.js
203

(let while at it)

fatherbushido added inline comments.Oct 29 2017, 9:00 AM
binaries/data/mods/public/simulation/components/EntityLimits.js
203

will be done when commiting

This revision was automatically updated to reflect the committed changes.
elexis added inline comments.Oct 29 2017, 2:48 PM
ps/trunk/binaries/data/mods/public/simulation/components/EntityLimits.js
65 ↗(On Diff #4008)

(Perhaps these two would be better off as prototype constants as they are available in the entire simulation context. Also the name sounds improveable, something like EntityLimitTypeTraining or so.)

fatherbushido added inline comments.Oct 29 2017, 5:54 PM
ps/trunk/binaries/data/mods/public/simulation/components/EntityLimits.js
65 ↗(On Diff #4008)

Personally I just don't see why they exist.