Page MenuHomeWildfire Games

Optimize GetIdentityClasses in Templates.js
ClosedPublic

Authored by Stan on Nov 4 2019, 10:52 PM.

Details

Reviewers
bb
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP23660: Optimize GetIdentityClasses in Templates.js
Summary

While looking for things to optimize I looked at the identity component, as apparently it's one of the slowest to be created because of the caching.

That component calls GetIdentityClasses which according to my profiling takes about 17-23µs to execute. This patch cuts this by half.

This function is called a lot on map generation (to create all entities) but also by the AI in entity.js, each time you open a detail window on a unit (some times more than 20 times) to put stuff in cache. So we might be able able to get back a few ms here and there to do other things

Test Plan

Make sure it's faster.
Check the output is the same, especially when using empty classes.
Notice that rank is passed as an array while it's always a single value.

Event Timeline

Stan created this revision.Nov 4 2019, 10:52 PM

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

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

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

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

Stan added a comment.Nov 4 2019, 11:08 PM

Running the file following file

with something like

node app.js

I get the following results:

0.0029000000000000002 ms.
0.002 ms.
0.0018 ms. <
0.0019 ms.
0.0028 ms.
0.0015 ms.
0.0015 ms. <
0.0013 ms.
0.0019 ms.
0.0014 ms.
0.0013 ms. <
0.0013 ms.

50000000 iterations

0.00194 ms.
0.0013599999999999999 ms.
0.00134 ms.
0.00138 ms.
Stan added a comment.Nov 4 2019, 11:40 PM

14000000 iterations

Original
0.001952441350057987 ms.
Push for rank
0.0013839924571470226 ms.
String
0.0014404163143730588 ms.
String no regex
0.0011586697571411995 ms.
Only push + For Loops
0.000828526207129471 ms.
Array.prototype.push.apply
0.0008673805714352056 ms.
Poor man's for
0.0008252551714124691 ms.
Poor man's while
0.0008407956429291516 ms.

200 iterations

Original
0.006944495253264904 ms.
Push for rank
0.010242999996989965 ms.
String
0.0030414946377277374 ms.
String no regex
0.003874499816447496 ms.
Only push + For Loops
0.0029770040418952703 ms.
Array.prototype.push.apply
0.004580500535666943 ms.
Poor man's for
0.004935504402965307 ms.
Poor man's while
0.0042970001231879 ms.

5000

Original
0.0032503800000995395 ms.
Push for rank
0.0017564602196216584 ms.
String
0.0021596400067210196 ms.
String no regex
0.0017216000240296125 ms.
Only push + For Loops
0.0020414799917489288 ms.
Array.prototype.push.apply
0.0014430799987167119 ms.
Poor man's for
0.0019078597892075778 ms.
Poor man's while
0.0015446002129465342 ms.

10000

Original
0.0028737799962982534 ms.
Push for rank
0.0019261399982497096 ms.
String
0.0016841801116243005 ms.
String no regex
0.0018637700006365774 ms.
Only push + For Loops
0.0015006100991740822 ms.
Array.prototype.push.apply
0.0011772299883887173 ms.
Poor man's for
0.0011110499966889619 ms.
Poor man's while
0.0012838701019063591 ms.
Stan updated this revision to Diff 10269.Nov 4 2019, 11:41 PM

For in seems to be faster in most cases and safer.

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

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

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

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

Stan updated this revision to Diff 10270.Nov 4 2019, 11:56 PM

Didn't know push returned the array size :D now I do.

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

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

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-extra-semi):
|    | Unnecessary semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|  45|  45| 		classList.push(template.Rank);
|  46|  46| 
|  47|  47| 	return classList;
|  48|    |-};
|    |  48|+}
|  49|  49| 
|  50|  50| /**
|  51|  51|  * Gets an array with all classes for this identity template
|    | [NORMAL] ESLintBear (operator-linebreak):
|    | '||' should be placed at the end of the line.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|  90|  90| 		// If the elements are still strings, split them by space or by '+'
|  91|  91| 		if (typeof sublist == "string")
|  92|  92| 			sublist = sublist.split(/[+\s]+/);
|  93|    |-		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1)
|  94|    |-		                    || (c[0] != "!" && classes.indexOf(c) != -1)))
|    |  93|+		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) ||
|    |  94|+		                    (c[0] != "!" && classes.indexOf(c) != -1)))
|  95|  95| 			return true;
|  96|  96| 	}
|  97|  97| 

binaries/data/mods/public/globalscripts/Templates.js
|  48| };
|    | [NORMAL] JSHintBear:
|    | Unnecessary semicolon.

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

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

Freagarach added inline comments.
binaries/data/mods/public/globalscripts/Templates.js
31

+.

bb added a subscriber: bb.Jan 13 2020, 7:48 PM
bb added inline comments.
binaries/data/mods/public/globalscripts/Templates.js
34–35

did you also try classList.push(...template.Classes._string.split(/\s+/))?

Stan added inline comments.Jan 13 2020, 7:50 PM
binaries/data/mods/public/globalscripts/Templates.js
34–35

No but I thought that the spread operator was bad for performance, and in my tests, for push was always faster than everything else.

bb added a comment.Jan 14 2020, 10:41 PM

Testing with SM gave that string without regex is fastest (about 20x), difference between other options is small.

As de class list is defined as a space-separated list, doing `split(" ") should work. Testing with adding more spaces tabs newlines and such doesn't make the front fall. So it seems to work, maybe I missed something?
The regex has been implemented by rP15195 (addition of visibleClasses).

Stan updated this revision to Diff 11834.May 11 2020, 12:07 PM
Stan marked 2 inline comments as done.
Stan edited the summary of this revision. (Show Details)
Stan edited the test plan for this revision. (Show Details)

I did some digging and it actually makes sense to use string.split(" ") because at lines 129-165 of ParamNode.cpp, we actually nuke everything and replace it by spaces.

You can also check using the structure tree there are never any other \s+ chars than space in the _string property.

if (template.Classes._string.indexOf("\t") != -1)
{
	warn("Classes._string contains tabs");
	warn(Object.prototype.toString.call(template.Classes._string)) // Was to check whether we used a custom string type
}

That also means there are a few other optimizations that can be made elsewhere (10 occurences) Profiling would be need to check how much they are called.

binaries\data\mods\public\globalscripts\Templates.js
  57,34: return template.VisibleClasses._string.split(/\s+/);

binaries\data\mods\public\simulation\components\Attack.js
  230,47: return this.template[type].PreferredClasses._string.split(/\s+/);
  239,48: return this.template[type].RestrictedClasses._string.split(/\s+/);

binaries\data\mods\public\simulation\components\Auras.js
  46,23: return this.template._string.split(/\s+/);

binaries\data\mods\public\simulation\components\EntityLimits.js
  91,75: this.removers[category][c] = this.template.LimitRemovers[category][c]._string.split(/\s+/);

binaries\data\mods\public\simulation\components\GuiInterface.js
  566,33: let auraNames = template.Auras._string.split(/\s+/);

binaries\data\mods\public\simulation\components\Identity.js
  128,33: return this.template.Phenotype._string.split(/\s+/);
  163,35: return this.template.Formations._string.split(/\s+/);

binaries\data\mods\public\simulation\components\StatisticsTracker.js
  29,48: this.unitsClasses = this.template.UnitClasses._string.split(/\s+/);
  30,57: this.buildingsClasses = this.template.StructureClasses._string.split(/\s+/);
Stan added a reviewer: bb.May 11 2020, 12:08 PM

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-extra-semi):
|    | Unnecessary semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|  45|  45| 		classList.push(template.Rank);
|  46|  46| 
|  47|  47| 	return classList;
|  48|    |-};
|    |  48|+}
|  49|  49| 
|  50|  50| /**
|  51|  51|  * Gets an array with all classes for this identity template
|    | [NORMAL] ESLintBear (operator-linebreak):
|    | '||' should be placed at the end of the line.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|  90|  90| 		// If the elements are still strings, split them by space or by '+'
|  91|  91| 		if (typeof sublist == "string")
|  92|  92| 			sublist = sublist.split(/[+\s]+/);
|  93|    |-		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1)
|  94|    |-		                    || (c[0] != "!" && classes.indexOf(c) != -1)))
|    |  93|+		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) ||
|    |  94|+		                    (c[0] != "!" && classes.indexOf(c) != -1)))
|  95|  95| 			return true;
|  96|  96| 	}
|  97|  97| 

binaries/data/mods/public/globalscripts/Templates.js
|  48| };
|    | [NORMAL] JSHintBear:
|    | Unnecessary semicolon.

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

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

Stan updated this revision to Diff 11835.May 11 2020, 12:23 PM

Remove unecessary semicolon

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (operator-linebreak):
|    | '||' should be placed at the end of the line.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|  90|  90| 		// If the elements are still strings, split them by space or by '+'
|  91|  91| 		if (typeof sublist == "string")
|  92|  92| 			sublist = sublist.split(/[+\s]+/);
|  93|    |-		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1)
|  94|    |-		                    || (c[0] != "!" && classes.indexOf(c) != -1)))
|    |  93|+		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) ||
|    |  94|+		                    (c[0] != "!" && classes.indexOf(c) != -1)))
|  95|  95| 			return true;
|  96|  96| 	}
|  97|  97| 

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

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

Stan updated this revision to Diff 11836.May 11 2020, 4:28 PM

Make linter happy, optimize further (Thanks @bb)

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

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

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

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

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

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/682/display/redirect

Stan updated this revision to Diff 11837.May 11 2020, 6:42 PM

Fix tests

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 4 spaces.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|  32|  32|  */
|  33|  33| function GetIdentityClasses(template)
|  34|  34| {
|  35|    |-    let classString = "";
|    |  35|+	let classString = "";
|  36|  36| 
|  37|  37|     if (template.Classes && template.Classes._string)
|  38|  38|         classString += " " + template.Classes._string;
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 4 spaces.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|  34|  34| {
|  35|  35|     let classString = "";
|  36|  36| 
|  37|    |-    if (template.Classes && template.Classes._string)
|    |  37|+	if (template.Classes && template.Classes._string)
|  38|  38|         classString += " " + template.Classes._string;
|  39|  39| 
|  40|  40|     if (template.VisibleClasses && template.VisibleClasses._string)
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 8 spaces.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|  35|  35|     let classString = "";
|  36|  36| 
|  37|  37|     if (template.Classes && template.Classes._string)
|  38|    |-        classString += " " + template.Classes._string;
|    |  38|+		classString += " " + template.Classes._string;
|  39|  39| 
|  40|  40|     if (template.VisibleClasses && template.VisibleClasses._string)
|  41|  41|         classString += " " + template.VisibleClasses._string;
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 4 spaces.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|  37|  37|     if (template.Classes && template.Classes._string)
|  38|  38|         classString += " " + template.Classes._string;
|  39|  39| 
|  40|    |-    if (template.VisibleClasses && template.VisibleClasses._string)
|    |  40|+	if (template.VisibleClasses && template.VisibleClasses._string)
|  41|  41|         classString += " " + template.VisibleClasses._string;
|  42|  42| 
|  43|  43|     if (template.Rank)
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 8 spaces.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|  38|  38|         classString += " " + template.Classes._string;
|  39|  39| 
|  40|  40|     if (template.VisibleClasses && template.VisibleClasses._string)
|  41|    |-        classString += " " + template.VisibleClasses._string;
|    |  41|+		classString += " " + template.VisibleClasses._string;
|  42|  42| 
|  43|  43|     if (template.Rank)
|  44|  44|         classString += " " + template.Rank;
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 4 spaces.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|  40|  40|     if (template.VisibleClasses && template.VisibleClasses._string)
|  41|  41|         classString += " " + template.VisibleClasses._string;
|  42|  42| 
|  43|    |-    if (template.Rank)
|    |  43|+	if (template.Rank)
|  44|  44|         classString += " " + template.Rank;
|  45|  45| 
|  46|  46|     return classString.length > 1 ? classString.substring(1).split(" ") : [];
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 8 spaces.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|  41|  41|         classString += " " + template.VisibleClasses._string;
|  42|  42| 
|  43|  43|     if (template.Rank)
|  44|    |-        classString += " " + template.Rank;
|    |  44|+		classString += " " + template.Rank;
|  45|  45| 
|  46|  46|     return classString.length > 1 ? classString.substring(1).split(" ") : [];
|  47|  47| }
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 4 spaces.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|  43|  43|     if (template.Rank)
|  44|  44|         classString += " " + template.Rank;
|  45|  45| 
|  46|    |-    return classString.length > 1 ? classString.substring(1).split(" ") : [];
|    |  46|+	return classString.length > 1 ? classString.substring(1).split(" ") : [];
|  47|  47| }
|  48|  48| 
|  49|  49| /**
Executing section cli...

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

Stan updated this revision to Diff 11838.May 11 2020, 6:49 PM

Fix tabs

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

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

Freagarach added inline comments.May 11 2020, 6:55 PM
binaries/data/mods/public/globalscripts/Templates.js
91

Spaces.

bb added a comment.May 11 2020, 11:00 PM

Extensive testing by stan and me showed this is the fastest syntax of all we considered, see http://irclogs.wildfiregames.com/2020-05/2020-05-11-QuakeNet-%230ad-dev.log

binaries/data/mods/public/globalscripts/Templates.js
31

is string[] proper JsDoc or should it be Array?

34–35

Wondered whether there should be a comment explaining why we add this space here, to just remove it later. Maybe instead of a comment add an unit test with an entity that has just 1 class of 1 character, this will blame anyone changing it too.

37

wondered if removing the > 1 would be different in performance => seems indifferent

91

spaces seem fine here

Stan planned changes to this revision.May 12 2020, 8:25 AM
Stan marked 2 inline comments as done.
Stan added inline comments.
binaries/data/mods/public/globalscripts/Templates.js
31
34–35

Okay will add a unit test.

37

Thanks for testimg.

Stan marked 4 inline comments as done.May 12 2020, 8:25 AM
Stan updated this revision to Diff 11839.May 12 2020, 10:18 AM

Add unit tests

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 4 spaces.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/tests/test_template.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/tests/test_template.js
|   1|   1| let identityTemplate = {
|   2|    |-    "Classes": { "@datatype": "tokens", "_string": "b a" },
|    |   2|+	"Classes": { "@datatype": "tokens", "_string": "b a" },
|   3|   3|     "VisibleClasses": { "@datatype": "tokens", "_string": "c f" }
|   4|   4| };
|   5|   5| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 4 spaces.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/tests/test_template.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/tests/test_template.js
|   1|   1| let identityTemplate = {
|   2|   2|     "Classes": { "@datatype": "tokens", "_string": "b a" },
|   3|    |-    "VisibleClasses": { "@datatype": "tokens", "_string": "c f" }
|    |   3|+	"VisibleClasses": { "@datatype": "tokens", "_string": "c f" }
|   4|   4| };
|   5|   5| 
|   6|   6| TS_ASSERT_UNEVAL_EQUALS(GetIdentityClasses(identityTemplate), ["b", "a", "c", "f"]);
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 4 spaces.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/tests/test_template.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/tests/test_template.js
|   7|   7| TS_ASSERT_UNEVAL_EQUALS(GetVisibleIdentityClasses(identityTemplate), ["c", "f"]);
|   8|   8| 
|   9|   9| identityTemplate = {
|  10|    |-    "Classes": { "@datatype": "tokens", "_string": "" },
|    |  10|+	"Classes": { "@datatype": "tokens", "_string": "" },
|  11|  11|     "VisibleClasses": { "@datatype": "tokens", "_string": "c f" }
|  12|  12| };
|  13|  13| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 4 spaces.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/tests/test_template.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/tests/test_template.js
|   8|   8| 
|   9|   9| identityTemplate = {
|  10|  10|     "Classes": { "@datatype": "tokens", "_string": "" },
|  11|    |-    "VisibleClasses": { "@datatype": "tokens", "_string": "c f" }
|    |  11|+	"VisibleClasses": { "@datatype": "tokens", "_string": "c f" }
|  12|  12| };
|  13|  13| 
|  14|  14| TS_ASSERT_UNEVAL_EQUALS(GetIdentityClasses(identityTemplate), ["c", "f"]);
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 4 spaces.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/tests/test_template.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/tests/test_template.js
|  15|  15| TS_ASSERT_UNEVAL_EQUALS(GetVisibleIdentityClasses(identityTemplate), ["c", "f"]);
|  16|  16| 
|  17|  17| identityTemplate = {
|  18|    |-    "Classes": { "@datatype": "tokens", "_string": "classA" },
|    |  18|+	"Classes": { "@datatype": "tokens", "_string": "classA" },
|  19|  19|     "VisibleClasses": { "@datatype": "tokens", "_string": "classC classF" },
|  20|  20|     "Rank": "testRank"
|  21|  21| };
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 4 spaces.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/tests/test_template.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/tests/test_template.js
|  16|  16| 
|  17|  17| identityTemplate = {
|  18|  18|     "Classes": { "@datatype": "tokens", "_string": "classA" },
|  19|    |-    "VisibleClasses": { "@datatype": "tokens", "_string": "classC classF" },
|    |  19|+	"VisibleClasses": { "@datatype": "tokens", "_string": "classC classF" },
|  20|  20|     "Rank": "testRank"
|  21|  21| };
|  22|  22| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 4 spaces.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/tests/test_template.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/tests/test_template.js
|  17|  17| identityTemplate = {
|  18|  18|     "Classes": { "@datatype": "tokens", "_string": "classA" },
|  19|  19|     "VisibleClasses": { "@datatype": "tokens", "_string": "classC classF" },
|  20|    |-    "Rank": "testRank"
|    |  20|+	"Rank": "testRank"
|  21|  21| };
|  22|  22| 
|  23|  23| TS_ASSERT_UNEVAL_EQUALS(GetIdentityClasses(identityTemplate), ["classA", "classC", "classF", "testRank"]);
Executing section cli...

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

Stan updated this revision to Diff 11840.May 12 2020, 10:28 AM

Use tabs

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

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

Stan updated this revision to Diff 11841.May 12 2020, 4:51 PM

More thorough tests.

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

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

bb accepted this revision as: bb.May 12 2020, 4:59 PM

New file for tests is appropriate since it is more than likely that more functions in the Templats.js file need tests.

Since we have the file Templates.js shouldn't we also have the test test_Templates.js (Cap)? Most tests in the sim also have the a cap

renaming is easy =>accept

binaries/data/mods/public/globalscripts/tests/test_template.js
6 ↗(On Diff #11841)

(one could try changing the f to a german b mole)

This revision is now accepted and ready to land.May 12 2020, 4:59 PM
Stan updated this revision to Diff 11842.May 12 2020, 5:14 PM

f toEszett rename with caps

Stan marked 2 inline comments as done.May 12 2020, 5:14 PM

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

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

bb accepted this revision.May 12 2020, 5:19 PM
This revision was automatically updated to reflect the committed changes.