Page MenuHomeWildfire Games

Allow Modifiers to affect tokens.
Needs ReviewPublic

Authored by wraitii on Mar 28 2017, 4:47 PM.

Details

Reviewers
Freagarach
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Summary

This adds a new mode to modifications called "tokens" which allows clever token parsing.

Technologies, auras and modifiers in general can use this to switch out, add or delete tokens dynamically.


Implemented use cases:

  • Changing production queue and builder entities at runtime.
    • Could be used for phase upgrades / unit upgrades.

Future use cases (will be implemented later, as they are trickier):

  • Changing garrisonable entities.
  • Changing healable classes.
  • Changing attackable classes.
  • Changing auras dynamically.
  • Changing Identity stuff
    • Should be extendable to do fancy-pants props switching on tech upgrades, ultimately.
    • That might make it possible to simulate "promotion" without actually promoting, too.
Test Plan

Test and review.

Event Timeline

wraitii created this revision.Mar 28 2017, 4:47 PM
Owners added a subscriber: Restricted Owners Package.Mar 28 2017, 4:47 PM
Vulcan added a subscriber: Vulcan.Mar 28 2017, 5:32 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!

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

mimo added a subscriber: mimo.Mar 28 2017, 10:19 PM

I don't know if we should have that (seems to me like looking for troubles), but anyway this patch will currently break the ai which would need some changes.

Mh, is it because the AI only gets the templates once at the start?

Re the feature itself, this could also be used to implement the spartan team bonus, which grants its teammates access to a specific unit.

mimo added a comment.Mar 28 2017, 11:26 PM
In D270#10404, @wraitii wrote:

Mh, is it because the AI only gets the templates once at the start?

Yes, it would need at least to update the buildableEntities and trainableEntities functions in common-api/entity.js
or directly the get function in the same file.

I think there are better way to upgrade Hypaspists to Silvershields, for instance use the Upgrade feature, and a tech unlocking this.

But regardless of that detail, the feature wraitii propose would make an exacellent addition to the game and open many possibilities for mods to extend the gameplay. Specifically, I like the idea of adding hard and soft counters via techs. I have designed a pair of techs for the Mauryans: poison arrows, which I have a TODO to make it give archers additional bonus vs. infantry, paired with a steel arrows tech which would give archers a bonus vs. cavalry and elephants. Though, I can see using the Upgrade feature to swap between steel arrows and poison arrows using templates, which means you can swap back to the other arrow type -- see my Mauryan Maiden Archers in Delenda Est. Not sure what way would be preferred or if a whole new "secondary attack" feature would be more desirable. Using the Upgrade feature is admittedly hacky, but works.

I can see a tech that allows ships to transport cavalry, which was a big deal in ancient times. Done by adding the 'Cavalry' token to the garrisonable classes. There are many possibilities. Thanks wraitii for pushing the gameplay.

wraitii planned changes to this revision.Mar 29 2017, 11:39 AM

@mimo: ok, that sounds easy enough to change I guess.

Re this feature, I now share the opinion that techs are a little dangerous to change, because their implementation is poorer than I expected tbh. For example "replace" is completely buggy, since it returns on the first modification, which I had not realised was a problem (turns out we don't have tests for technology effects, only for technology triggers).

I think I'm just going to rewrite this all into a "ModifiersManager" class ideally called by both auras and technologies and anything else, that would handle temporaries as well as permanent modifiers in a centralised manner, supporting orders and a few other things correctly. Should not actually be too difficult, though obviously it might introduce new bugs. So stand-by.

wraitii updated this revision to Diff 8141.EditedMay 25 2019, 8:48 PM

I think this is worth having.

Still doesn't work with the AI, need to fix that.

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Technologies.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Technologies.js
|  48|  48| 
|  49|  49| /**
|  50|  50|  * Derives modifications (to be applied to entities) from a given technology.
|  51|    |- * 
|    |  51|+ *
|  52|  52|  * @param {Object} techTemplate - The technology template to derive the modifications from.
|  53|  53|  * @return {Object} containing the relevant modifications.
|  54|  54|  */
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'for-of'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Technologies.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Technologies.js
| 132| 132| 	let newTokens = modification.split(/\s+/);
| 133| 133| 
| 134| 134| 	for (let token of newTokens)
| 135|    |-	{
|    | 135|+	
| 136| 136| 		if (token.search(">") !== -1)
| 137| 137| 		{
| 138| 138| 			let [oldToken, newToken] = token.split(">");
| 148| 148| 		}
| 149| 149| 		else
| 150| 150| 			tokens.push(token);
| 151|    |-	}
|    | 151|+	
| 152| 152| 
| 153| 153| 	return tokens.join(" ");
| 154| 154| }
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Technologies.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Technologies.js
| 144| 144| 		{
| 145| 145| 			let index = tokens.indexOf(token.substr(1));
| 146| 146| 			if (index !== -1)
| 147|    |-				tokens.splice(index,1);
|    | 147|+				tokens.splice(index, 1);
| 148| 148| 		}
| 149| 149| 		else
| 150| 150| 			tokens.push(token);
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'civPermitted' to undefined.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Technologies.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Technologies.js
| 257| 257| 
| 258| 258| 	case "all":
| 259| 259| 	{
| 260|    |-		let civPermitted = undefined; // tri-state (undefined, false, or true)
|    | 260|+		let civPermitted; // tri-state (undefined, false, or true)
| 261| 261| 		for (let subvalue of value)
| 262| 262| 		{
| 263| 263| 			let newOper = Object.keys(subvalue)[0];

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

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

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

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

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

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

binaries/data/mods/public/globalscripts/Technologies.js
| 368| »   »   »   »   civPermitted·=·true;
|    | [NORMAL] JSHintBear:
|    | Expected a 'break' statement before 'case'.
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'for-of'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/ProductionQueue.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/ProductionQueue.js
| 474| 474| {
| 475| 475| 	var out = [];
| 476| 476| 	for (var item of this.queue)
| 477|    |-	{
|    | 477|+	
| 478| 478| 		out.push({
| 479| 479| 			"id": item.id,
| 480| 480| 			"unitTemplate": item.unitTemplate,
| 485| 485| 			"timeRemaining": item.timeRemaining,
| 486| 486| 			"metadata": item.metadata,
| 487| 487| 		});
| 488|    |-	}
|    | 488|+	
| 489| 489| 	return out;
| 490| 490| };
| 491| 491| 
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/ProductionQueue.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/ProductionQueue.js
| 567| 567| 	var spawnedEnts = [];
| 568| 568| 
| 569| 569| 	if (this.entityCache.length == 0)
| 570|    |-	{
|    | 570|+	
| 571| 571| 		// We need entities to test spawning, but we don't want to waste resources,
| 572| 572| 		//	so only create them once and use as needed
| 573| 573| 		for (var i = 0; i < count; ++i)
| 586| 586| 				cmpPlayerEntityLimits.ChangeCount(unitCategory, -1);
| 587| 587| 			}
| 588| 588| 		}
| 589|    |-	}
|    | 589|+	
| 590| 590| 
| 591| 591| 	let cmpAutoGarrison;
| 592| 592| 	if (cmpRallyPoint)
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/ProductionQueue.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/ProductionQueue.js
| 647| 647| 	}
| 648| 648| 
| 649| 649| 	if (spawnedEnts.length > 0 && !cmpAutoGarrison)
| 650|    |-	{
|    | 650|+	
| 651| 651| 		// If a rally point is set, walk towards it (in formation) using a suitable command based on where the
| 652| 652| 		// rally point is placed.
| 653| 653| 		if (cmpRallyPoint)
| 660| 660| 					ProcessCommand(cmpOwnership.GetOwner(), com);
| 661| 661| 			}
| 662| 662| 		}
| 663|    |-	}
|    | 663|+	
| 664| 664| 
| 665| 665| 	if (createdEnts.length > 0)
| 666| 666| 		Engine.PostMessage(this.entity, MT_TrainingFinished, {

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 345| »   »   »   let·template·=·TechnologyTemplates.Get(templateName);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'template' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 347| »   »   »   let·time·=·techCostMultiplier.time·*·template.researchTime·*·cmpPlayer.GetTimeMultiplier();
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'time' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 599| »   for·(let·i·=·0;·i·<·count;·++i)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 601| »   »   let·ent·=·this.entityCache[0];
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'ent' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 784| »   »   »   let·template·=·TechnologyTemplates.Get(item.technologyTemplate);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'template' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 192| »   for·(var·i·in·techList)
|    | [NORMAL] JSHintBear:
|    | 'i' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 194| »   »   var·tech·=·techList[i];
|    | [NORMAL] JSHintBear:
|    | 'tech' is already defined.

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

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 206| »   »   var·tech·=·techList[i];
|    | [NORMAL] JSHintBear:
|    | 'tech' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 376| »   »   »   var·cmpTrigger·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_Trigger);
|    | [NORMAL] JSHintBear:
|    | 'cmpTrigger' is already defined.

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

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 781| »   »   »   var·cmpTechnologyManager·=·QueryOwnerInterface(this.entity,·IID_TechnologyManager);
|    | [NORMAL] JSHintBear:
|    | 'cmpTechnologyManager' is already defined.
|    | [NORMAL] ESLintBear (space-before-function-paren):
|    | Unexpected space before function parentheses.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/messages.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/messages.js
| 473| 473| 		g_Selection.reset();
| 474| 474| 		g_Selection.addList(selection, false, cmd.type == "gather");
| 475| 475| 	},
| 476|    |-	"play-tracks": function (notification, player)
|    | 476|+	"play-tracks": function(notification, player)
| 477| 477| 	{
| 478| 478| 		if (notification.lock)
| 479| 479| 		{
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/messages.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/messages.js
| 580| 580| 	let notificationText =
| 581| 581| 		notification.instructions.reduce((instructions, item) =>
| 582| 582| 			instructions + (typeof item == "string" ? translate(item) : colorizeHotkey(translate(item.text), item.hotkey)),
| 583|    |-			"");
|    | 583|+		"");
| 584| 584| 
| 585| 585| 	Engine.GetGUIObjectByName("tutorialText").caption = g_TutorialMessages.concat(setStringTags(notificationText, g_TutorialNewMessageTags)).join("\n");
| 586| 586| 	g_TutorialMessages.push(notificationText);
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/messages.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/messages.js
|1088|1088| 
|1089|1089| 	let message = "";
|1090|1090| 	if (notifyPhase == "all")
|1091|    |-	{
|    |1091|+	
|1092|1092| 		if (msg.phaseState == "started")
|1093|1093| 			message = translate("%(player)s is advancing to the %(phaseName)s.");
|1094|1094| 		else if (msg.phaseState == "aborted")
|1095|1095| 			message = translate("The %(phaseName)s of %(player)s has been aborted.");
|1096|    |-	}
|    |1096|+	
|1097|1097| 	if (msg.phaseState == "completed")
|1098|1098| 		message = translate("%(player)s has reached the %(phaseName)s.");
|1099|1099| 
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/differential/1497/display/redirect

wraitii added a reviewer: Restricted Owners Package.Jun 12 2019, 10:33 PM
wraitii updated this revision to Diff 8647.Jun 29 2019, 6:52 PM
wraitii edited the summary of this revision. (Show Details)

Tokens > _string for convenience.

This includes AI support (and a tech demo that won't get committed).

Owners added a subscriber: Restricted Owners Package.Jun 29 2019, 6:52 PM

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Technologies.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Technologies.js
|  48|  48| 
|  49|  49| /**
|  50|  50|  * Derives modifications (to be applied to entities) from a given technology.
|  51|    |- * 
|    |  51|+ *
|  52|  52|  * @param {Object} techTemplate - The technology template to derive the modifications from.
|  53|  53|  * @return {Object} containing the relevant modifications.
|  54|  54|  */
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'for-of'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Technologies.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Technologies.js
| 132| 132| 	let newTokens = modification.split(/\s+/);
| 133| 133| 
| 134| 134| 	for (let token of newTokens)
| 135|    |-	{
|    | 135|+	
| 136| 136| 		if (token.search(">") !== -1)
| 137| 137| 		{
| 138| 138| 			let [oldToken, newToken] = token.split(">");
| 148| 148| 		}
| 149| 149| 		else
| 150| 150| 			tokens.push(token);
| 151|    |-	}
|    | 151|+	
| 152| 152| 
| 153| 153| 	return tokens.join(" ");
| 154| 154| }
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Technologies.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Technologies.js
| 144| 144| 		{
| 145| 145| 			let index = tokens.indexOf(token.substr(1));
| 146| 146| 			if (index !== -1)
| 147|    |-				tokens.splice(index,1);
|    | 147|+				tokens.splice(index, 1);
| 148| 148| 		}
| 149| 149| 		else
| 150| 150| 			tokens.push(token);
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'civPermitted' to undefined.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Technologies.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Technologies.js
| 257| 257| 
| 258| 258| 	case "all":
| 259| 259| 	{
| 260|    |-		let civPermitted = undefined; // tri-state (undefined, false, or true)
|    | 260|+		let civPermitted; // tri-state (undefined, false, or true)
| 261| 261| 		for (let subvalue of value)
| 262| 262| 		{
| 263| 263| 			let newOper = Object.keys(subvalue)[0];

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

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

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

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

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

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

binaries/data/mods/public/globalscripts/Technologies.js
| 368| »   »   »   »   civPermitted·=·true;
|    | [NORMAL] JSHintBear:
|    | Expected a 'break' statement before 'case'.
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'for-of'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/AIInterface.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/AIInterface.js
| 242| 242| 			let item = template;
| 243| 243| 			let ended = true;
| 244| 244| 			for (let str of strings)
| 245|    |-			{
|    | 245|+			
| 246| 246| 				if (item !== undefined && item[str] !== undefined)
| 247| 247| 					item = item[str];
| 248| 248| 				else
| 249| 249| 					ended = false;
| 250|    |-			}
|    | 250|+			
| 251| 251| 			if (!ended)
| 252| 252| 				continue;
| 253| 253| 			// item now contains the template value for this.
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'for-of'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/AIInterface.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/AIInterface.js
| 290| 290| 			let item = template;
| 291| 291| 			let ended = true;
| 292| 292| 			for (let str of strings)
| 293|    |-			{
|    | 293|+			
| 294| 294| 				if (item !== undefined && item[str] !== undefined)
| 295| 295| 					item = item[str];
| 296| 296| 				else
| 297| 297| 					ended = false;
| 298|    |-			}
|    | 298|+			
| 299| 299| 			if (!ended)
| 300| 300| 				continue;
| 301| 301| 			// "item" now contains the unmodified template value for this.
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'for-of'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/ProductionQueue.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/ProductionQueue.js
| 467| 467| {
| 468| 468| 	var out = [];
| 469| 469| 	for (var item of this.queue)
| 470|    |-	{
|    | 470|+	
| 471| 471| 		out.push({
| 472| 472| 			"id": item.id,
| 473| 473| 			"unitTemplate": item.unitTemplate,
| 478| 478| 			"timeRemaining": item.timeRemaining,
| 479| 479| 			"metadata": item.metadata,
| 480| 480| 		});
| 481|    |-	}
|    | 481|+	
| 482| 482| 	return out;
| 483| 483| };
| 484| 484| 
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/ProductionQueue.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/ProductionQueue.js
| 636| 636| 	}
| 637| 637| 
| 638| 638| 	if (spawnedEnts.length > 0 && !cmpAutoGarrison)
| 639|    |-	{
|    | 639|+	
| 640| 640| 		// If a rally point is set, walk towards it (in formation) using a suitable command based on where the
| 641| 641| 		// rally point is placed.
| 642| 642| 		if (cmpRallyPoint)
| 649| 649| 					ProcessCommand(cmpOwnership.GetOwner(), com);
| 650| 650| 			}
| 651| 651| 		}
| 652|    |-	}
|    | 652|+	
| 653| 653| 
| 654| 654| 	if (createdEnts.length > 0)
| 655| 655| 		Engine.PostMessage(this.entity, MT_TrainingFinished, {

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 773| »   »   »   let·template·=·TechnologyTemplates.Get(item.technologyTemplate);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'template' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 192| »   for·(var·i·in·techList)
|    | [NORMAL] JSHintBear:
|    | 'i' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 194| »   »   var·tech·=·techList[i];
|    | [NORMAL] JSHintBear:
|    | 'tech' is already defined.

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

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 206| »   »   var·tech·=·techList[i];
|    | [NORMAL] JSHintBear:
|    | 'tech' is already defined.

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

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 770| »   »   »   var·cmpTechnologyManager·=·QueryOwnerInterface(this.entity,·IID_TechnologyManager);
|    | [NORMAL] JSHintBear:
|    | 'cmpTechnologyManager' is already defined.
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'for-of'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/ai/common-api/entity.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/ai/common-api/entity.js
|  32|  32| 		{
|  33|  33| 			let args = string.split("/");
|  34|  34| 			for (let arg of args)
|  35|    |-			{
|    |  35|+			
|  36|  36| 				if (value[arg])
|  37|  37| 					value = value[arg];
|  38|  38| 				else
|  40|  40| 					value = undefined;
|  41|  41| 					break;
|  42|  42| 				}
|  43|    |-			}
|    |  43|+			
|  44|  44| 			this._tpCache.set(string, value);
|  45|  45| 		}
|  46|  46| 		return this._tpCache.get(string);
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/ai/common-api/entity.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/ai/common-api/entity.js
| 307| 307| 			return undefined;
| 308| 308| 
| 309| 309| 		if (this.get("Attack/" + type + "/Bonuses"))
| 310|    |-		{
|    | 310|+		
| 311| 311| 			for (let b in this.get("Attack/" + type + "/Bonuses"))
| 312| 312| 			{
| 313| 313| 				let bonusClasses = this.get("Attack/" + type + "/Bonuses/" + b + "/Classes");
| 317| 317| 					if (bcl == againstClass)
| 318| 318| 						return +this.get("Attack/" + type + "/Bonuses/" + b + "/Multiplier");
| 319| 319| 			}
| 320|    |-		}
|    | 320|+		
| 321| 321| 		return 1;
| 322| 322| 	},
| 323| 323| 

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 797| »   »   if·(this.position()·!==·undefined)·{
|    | [NORMAL] ESLintBear (brace-rules/brace-on-same-line):
|    | Opening curly brace appears on the same line as controlling statement.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 814| »   »   if·(this.position()·!==·undefined·&&·unitToFleeFrom.position()·!==·undefined)·{
|    | [NORMAL] ESLintBear (brace-rules/brace-on-same-line):
|    | Opening curly brace appears on the same line as controlling statement.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 944| }(API3);
|    | [MAJOR] ESLintBear (no-use-before-define):
|    | 'API3' was used before it was defined.
|    | [NORMAL] ESLintBear (space-before-function-paren):
|    | Unexpected space before function parentheses.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/messages.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/messages.js
| 473| 473| 		g_Selection.reset();
| 474| 474| 		g_Selection.addList(selection, false, cmd.type == "gather");
| 475| 475| 	},
| 476|    |-	"play-tracks": function (notification, player)
|    | 476|+	"play-tracks": function(notification, player)
| 477| 477| 	{
| 478| 478| 		if (notification.lock)
| 479| 479| 		{
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/messages.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/messages.js
| 580| 580| 	let notificationText =
| 581| 581| 		notification.instructions.reduce((instructions, item) =>
| 582| 582| 			instructions + (typeof item == "string" ? translate(item) : colorizeHotkey(translate(item.text), item.hotkey)),
| 583|    |-			"");
|    | 583|+		"");
| 584| 584| 
| 585| 585| 	Engine.GetGUIObjectByName("tutorialText").caption = g_TutorialMessages.concat(setStringTags(notificationText, g_TutorialNewMessageTags)).join("\n");
| 586| 586| 	g_TutorialMessages.push(notificationText);
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/messages.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/messages.js
|1088|1088| 
|1089|1089| 	let message = "";
|1090|1090| 	if (notifyPhase == "all")
|1091|    |-	{
|    |1091|+	
|1092|1092| 		if (msg.phaseState == "started")
|1093|1093| 			message = translate("%(player)s is advancing to the %(phaseName)s.");
|1094|1094| 		else if (msg.phaseState == "aborted")
|1095|1095| 			message = translate("The %(phaseName)s of %(player)s has been aborted.");
|1096|    |-	}
|    |1096|+	
|1097|1097| 	if (msg.phaseState == "completed")
|1098|1098| 		message = translate("%(player)s has reached the %(phaseName)s.");
|1099|1099| 
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/differential/1845/display/redirect

Freagarach added inline comments.
binaries/data/mods/public/globalscripts/Technologies.js
53

Could it be like:
"A>B" to replace A by B, "-A" to remove A, "+A" to add A and rest will replace all.?

wraitii added inline comments.Sep 20 2019, 3:21 PM
binaries/data/mods/public/globalscripts/Technologies.js
53

Modifiers already support replace to replace all, which I think is sufficient, so I do't see much the point.

I guess it could still be A>B -A and +A though.

Freagarach added inline comments.Sep 20 2019, 3:27 PM
binaries/data/mods/public/globalscripts/Technologies.js
30–33

Do these returns mean that if one has multiple modifications which replace/handle tokens, only the first one is applied?

53

Ah yes of course.

Stan added a subscriber: Stan.Sep 20 2019, 3:29 PM
Stan added inline comments.
binaries/data/mods/public/globalscripts/Technologies.js
62
binaries/data/mods/public/gui/session/messages.js
219

Could invert it and remove a line ?

binaries/data/mods/public/simulation/components/AIInterface.js
254

Is that necessary ?

binaries/data/mods/public/simulation/components/ProductionQueue.js
878

check ?

elexis added a subscriber: elexis.Sep 20 2019, 3:48 PM

I might agree with the "asking for trouble" (as in bugs), and if only one of the use cases describes is implemented, it leaves a WIP impression.
The use cases described however don't seem easily refutable however if one wants to consider the simulation engine to become more extensible with regards to features, unit interaction etc.

binaries/data/mods/public/gui/session/messages.js
217

dont name this according to what it should do, consider this to be an event that describes what happened.
then the GUI can decide what to do on that event, which may be more than updating the selection.

So it should be something like "entitytokenschanged" or so.

Also one only needs to rebuild the selection if an enity was affected.

binaries/data/mods/public/simulation/components/ProductionQueue.js
883

This sounds very expensive if this is done for every OnValueModification of every component that ends up with a tooltip.

I suppose we dont want a live-stream of events but only an update each turn?

Consider the current game with 1600 units on the map in a 4v4 with 200 pop per player which is the current lobby mainland status quo.
And those are only units, there can also be hundreds (thousands?) of structures affected by value modifications, with multiple components per entity that can have tokens.

Thanks for the look everyone, I will take this as confirmation that despite the relative bug-possibilities (tbh this seems safe enough to me), there is some good from digging deeper in this project.

binaries/data/mods/public/simulation/components/AIInterface.js
254

As I recall it was. There'd probably be other ways to do this cleanly.

Stan added inline comments.Mar 9 2020, 8:20 AM
binaries/data/mods/public/simulation/components/ProductionQueue.js
878

According to the undying nephalim when an unit with an aura promotes this error is dispmayed

a small problem it causes though.... units that promote that have an aura it now shows a huge error at the top of the screen
<p class="error">ERROR: Script message handler OnValueModification failed</p>
<p class="error">ERROR: Script message handler OnValueModification failed</p>
<p class="error">ERROR: Script message handler OnGlobalValueModification failed</p>
<p class="error">ERROR: JavaScript error: simulation/components/ProductionQueue.js line 843
TypeError: cmpPlayer is null
  ProductionQueue.prototype.OnValueModification@simulation/components/ProductionQueue.js:843:15
  AuraManager.prototype.RemoveBonus@simulation/components/AuraManager.js:168:1
  Auras.prototype.RemoveBonus@simulation/components/Auras.js:436:1
  Auras.prototype.RegisterGlobalOwnershipChanged@simulation/components/Auras.js:326:1
  AuraManager.prototype.OnGlobalOwnershipChanged@simulation/components/AuraManager.js:271:4</p>
<p class="error">ERROR: Script message handler OnGlobalOwnershipChanged failed</p>
Stan added inline comments.Mar 9 2020, 8:34 AM
binaries/data/mods/public/simulation/components/ProductionQueue.js
878

Only global auras not range ones.

wraitii updated this revision to Diff 11978.Fri, May 22, 7:01 PM
wraitii marked 5 inline comments as done.
wraitii retitled this revision from Allow technologies to affect tokens. to Allow Modifiers to affect tokens..
wraitii edited the summary of this revision. (Show Details)

Rework the selection update implementation to not be braindead.

Implement the following modifications:

  • ProductionQueue/Entities/_string
  • ProductionQueue/Technologies/_string
  • Builder/Entities/_string

For convenience, the "tech demo" can be researched at the TC, and you'll see it works as expected. You may also test the Briton hero Cunobelin.

(It didn't occur to me, but this works with auras as well obviously, which can be cool for global auras and special buildings perhaps).

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

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

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

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

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

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

Freagarach added inline comments.Mon, May 25, 6:09 PM
binaries/data/mods/public/globalscripts/Technologies.js
15

(numeric)

Freagarach accepted this revision.Mon, May 25, 7:48 PM

Looking very nice :)
I couldn't break it. I tested:

  • Provided tech-demo.
  • Removing a production queue item whilst it is being executed or is scheduled.
  • Changing ownership several ways.
  • Previewing a building that is changed.

(You do need to fix the test obviously.)

binaries/data/mods/public/globalscripts/Technologies.js
39–40

(Not true anymore.)

This revision is now accepted and ready to land.Mon, May 25, 7:48 PM
Stan added a comment.Mon, May 25, 9:03 PM

Are most of your breaking attempts coveres by tests ?

wraitii updated this revision to Diff 12027.Tue, May 26, 1:39 PM
wraitii marked an inline comment as done.
In D270#116956, @Stan wrote:

Are most of your breaking attempts coveres by tests ?

These are more integration tests that we can't easily do in our unit tests, sadly. I need to get around to adding integration-test facilities, but it's not so trivial atm.


Fix the tests, broken by needing a mock now.

binaries/data/mods/public/globalscripts/Technologies.js
62

yup.

binaries/data/mods/public/gui/session/messages.js
217

Good point, though I've simply removed this event and resetselectionpannel below in the latest diff.

binaries/data/mods/public/simulation/components/ProductionQueue.js
883

You're right, a much better solution is to poll Gui Interface to recreate the selection.
I've also done some changes to try and avoid spurious reconstruction of the selection.

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

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

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

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

Stan added inline comments.Tue, May 26, 2:21 PM
binaries/data/mods/public/globalscripts/Technologies.js
57

Do we get tabs here? I know we don't for datatype tokens as it is handled by cpp but I don't know there
else can be split(" ") which is faster

binaries/data/mods/public/simulation/components/AIInterface.js
254

typeof number maybe? dunno

binaries/data/mods/public/simulation/components/ProductionQueue.js
91

Check is after below, not sure if that has any influence.

148–150

inline?

binaries/data/mods/public/simulation/data/technologies/tech_demo.json
12

Maybe one with multiple changes at the same time, I had to experiment a bit to find one had to put them all in the string :)

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'civPermitted' to undefined.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/Technologies.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Technologies.js
| 183| 183| 
| 184| 184| 	case "all":
| 185| 185| 	{
| 186|    |-		let civPermitted = undefined; // tri-state (undefined, false, or true)
|    | 186|+		let civPermitted; // tri-state (undefined, false, or true)
| 187| 187| 		for (let subvalue of value)
| 188| 188| 		{
| 189| 189| 			let newOper = Object.keys(subvalue)[0];

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

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

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

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

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

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

binaries/data/mods/public/globalscripts/Technologies.js
| 294| »   »   »   »   civPermitted·=·true;
|    | [NORMAL] JSHintBear:
|    | Expected a 'break' statement before 'case'.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/session/messages.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/session/messages.js
| 339| 339| 	let notificationText =
| 340| 340| 		notification.instructions.reduce((instructions, item) =>
| 341| 341| 			instructions + (typeof item == "string" ? translate(item) : colorizeHotkey(translate(item.text), item.hotkey)),
| 342|    |-			"");
|    | 342|+		"");
| 343| 343| 
| 344| 344| 	Engine.GetGUIObjectByName("tutorialText").caption = g_TutorialMessages.concat(setStringTags(notificationText, g_TutorialNewMessageTags)).join("\n");
| 345| 345| 	g_TutorialMessages.push(notificationText);

binaries/data/mods/public/gui/session/session.js
| 698| »   »   button.onPress·=·(function(i)·{·return·function()·{·performGroup((Engine.HotkeyIsPressed("selection.add")·?·"add"·:·"select"),·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
| 699| »   »   button.onDoublePress·=·(function(i)·{·return·function()·{·performGroup("snap",·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
| 700| »   »   button.onPressRight·=·(function(i)·{·return·function()·{·performGroup("breakUp",·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 8 spaces.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_ProductionQueue.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_ProductionQueue.js
|   1|   1| Resources = {
|   2|    |-        "BuildSchema": (a, b) => {}
|    |   2|+	"BuildSchema": (a, b) => {}
|   3|   3| };
|   4|   4| 
|   5|   5| Engine.LoadHelperScript("Player.js");
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 67 spaces.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_ProductionQueue.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_ProductionQueue.js
| 123| 123| 	["units/athen_cavalry_javelinist_b", "units/iber_support_female_citizen"]
| 124| 124| );
| 125| 125| TS_ASSERT_UNEVAL_EQUALS(cmpProductionQueue.GetTechnologiesList(), ["phase_town_athen",
| 126|    |-                                                                   "phase_city_athen"]
|    | 126|+	"phase_city_athen"]
| 127| 127| );
| 128| 128| 
| 129| 129| AddMock(playerEntityID, IID_TechnologyManager, {

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

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 800| »   »   if·(this.position()·!==·undefined)·{
|    | [NORMAL] ESLintBear (brace-rules/brace-on-same-line):
|    | Opening curly brace appears on the same line as controlling statement.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 817| »   »   if·(this.position()·!==·undefined·&&·unitToFleeFrom.position()·!==·undefined)·{
|    | [NORMAL] ESLintBear (brace-rules/brace-on-same-line):
|    | Opening curly brace appears on the same line as controlling statement.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 947| }(API3);
|    | [MAJOR] ESLintBear (no-use-before-define):
|    | 'API3' was used before it was defined.
Executing section cli...

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

wraitii updated this revision to Diff 12056.Sat, May 30, 11:09 AM

Had the rather good idea of testing this further, and I noticed that I didn't handle the case where a template being produced was changed.

...

This proved to be impossible to fix cleanly. At the moment, tokens are processed and {civ}/{native} strings replaced very early on. I don't have a way to recover the original token.
I should look into refactoring that, but in the meantime this works.

Also added a test case for this, which ended up being rather useful in debugging this annoying issue.

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'civPermitted' to undefined.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/Technologies.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Technologies.js
| 181| 181| 
| 182| 182| 	case "all":
| 183| 183| 	{
| 184|    |-		let civPermitted = undefined; // tri-state (undefined, false, or true)
|    | 184|+		let civPermitted; // tri-state (undefined, false, or true)
| 185| 185| 		for (let subvalue of value)
| 186| 186| 		{
| 187| 187| 			let newOper = Object.keys(subvalue)[0];

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

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

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

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

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

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

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

binaries/data/mods/public/gui/session/session.js
| 698| »   »   button.onPress·=·(function(i)·{·return·function()·{·performGroup((Engine.HotkeyIsPressed("selection.add")·?·"add"·:·"select"),·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
| 699| »   »   button.onDoublePress·=·(function(i)·{·return·function()·{·performGroup("snap",·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
| 700| »   »   button.onPressRight·=·(function(i)·{·return·function()·{·performGroup("breakUp",·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/session/messages.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/session/messages.js
| 339| 339| 	let notificationText =
| 340| 340| 		notification.instructions.reduce((instructions, item) =>
| 341| 341| 			instructions + (typeof item == "string" ? translate(item) : colorizeHotkey(translate(item.text), item.hotkey)),
| 342|    |-			"");
|    | 342|+		"");
| 343| 343| 
| 344| 344| 	Engine.GetGUIObjectByName("tutorialText").caption = g_TutorialMessages.concat(setStringTags(notificationText, g_TutorialNewMessageTags)).join("\n");
| 345| 345| 	g_TutorialMessages.push(notificationText);

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

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 800| »   »   if·(this.position()·!==·undefined)·{
|    | [NORMAL] ESLintBear (brace-rules/brace-on-same-line):
|    | Opening curly brace appears on the same line as controlling statement.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 817| »   »   if·(this.position()·!==·undefined·&&·unitToFleeFrom.position()·!==·undefined)·{
|    | [NORMAL] ESLintBear (brace-rules/brace-on-same-line):
|    | Opening curly brace appears on the same line as controlling statement.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 947| }(API3);
|    | [MAJOR] ESLintBear (no-use-before-define):
|    | 'API3' was used before it was defined.
Executing section cli...

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

wraitii requested review of this revision.Mon, Jun 1, 7:15 AM

For the record, given the hack I'm now introducing in ProductionQueue.js, I won't commit this until I either:

  • Get some other dev to look at this and agree that it's painful but we should go along with it
  • Refactor so I can remove the hack.

This might or might not entail removing the "auto-Promotion" business, I am so far unclear on that.

binaries/data/mods/public/globalscripts/Technologies.js
57

There is a bunch of other code using /\s+/ and some using ' '. I'm leaving this as is, for consistency one should do an actual check someday.

binaries/data/mods/public/simulation/components/AIInterface.js
254

wouldn't work, since if it was a number we wouldn't need to '+' it.

binaries/data/mods/public/simulation/components/ProductionQueue.js
91

👍