Page MenuHomeWildfire Games

Allow technologies to affect tokens.
Needs ReviewPublic

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

Details

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

This adds a new mode to technology modifications called "tokens" which allows clever token parsing. Potential use cases are:

-Swapping trainable units without having to show all potential units from the start (which can clutter the GUI massively if you have enough upgrades). Could potentially be extended for triggers (not sure if we plan on having triggers allowed to change the sim templates?)
-Likewise with researches and build able entities
-Adding hard/soft counters to units through technology.
-Adding/Removing auras to units by technology.
-Changing Garrisonable entities by technology
-Changing Healable classes by technology.

As it stands now, only ProductionQueue handles it, but it would be quite easy to extend on a per-component basis.

Changing garrison able tokens is on the list of desired technology effects, which this would help with (obviously there are other issues with changing garrisoned tokens that would need to be handled).

Test Plan

Test and review.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
temp
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 8115
Build 13210: Vulcan BuildJenkins
Build 13209: arc lint + arc unit

Unit TestsFailed

TimeTest
0 msJenkins > cxxtest_debug.xml::[failed-to-read]
Failed to read test report file /mnt/data/jenkins-phabricator/workspace/differential/cxxtest_debug.xml org.dom4j.DocumentException: Error on line 1 of document : Content is not allowed in prolog. Nested exception: Content is not allowed in prolog. at org.dom4j.io.SAXReader.read(SAXReader.java:482)
0 msJenkins > TestAllocators::test_da
0 msJenkins > TestAtlasObjectXML::test_parse_attributes1
0 msJenkins > TestAtlasObjectXML::test_parse_attributes2
0 msJenkins > TestAtlasObjectXML::test_parse_basic
View Full Test Results (1 Failed · 322 Passed)

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
127

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
127

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
32–34

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

127

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
136
binaries/data/mods/public/gui/session/messages.js
424

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
827

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
422

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
832

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.