Page MenuHomeWildfire Games
Differential D2182

Support training of groups of units.
Needs ReviewPublic
Actions

Authored by Freagarach on Aug 16 2019, 3:57 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

This should allow training of groups of units (e.g. 3 archers, 2 spearmen) or a random selection of part of a group of units.
E.g. one can specify a composition with two entries Unit A and Unit B. On spawn both units will be spawned, unless MutuallyExclusive is true, then a random entry is chosen to spawn.

Yet to do:

  • Test.
  • PetraAI Support?
Test Plan

Start a game as Athens and click the button at the CC.

  • Play with the template to try and break functionality.
  • Verify that when there is not enough room to spawn entities, the correct number of remaining entities will be spawned when room has been made.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 9118
Build 14943: Vulcan BuildJenkins
Build 14942: arc lint + arc unit

Event Timeline

Freagarach created this revision.Aug 16 2019, 3:57 PM
Freagarach created this object with visibility "No One".
Owners added a subscriber: Restricted Owners Package.Aug 16 2019, 3:57 PM
Freagarach retitled this revision from [WIP] - All training of groups of units. to [WIP] - Support training of groups of units..Aug 16 2019, 3:58 PM
Freagarach edited the summary of this revision. (Show Details)Aug 19 2019, 9:49 PM
Freagarach changed the visibility from "No One" to "Subscribers".
Freagarach edited subscribers, added: Stan; removed: Restricted Owners Package.
Stan awarded a token.Aug 19 2019, 10:05 PM
Freagarach updated this revision to Diff 9432.Aug 21 2019, 6:13 PM
Freagarach edited the summary of this revision. (Show Details)
  • Moved group data from ProductionQueue.js to Identity.js to remove duplication.
  • Moved the training from seperate group entry to the entities entry.
  • Added Cost to the template so that one can choose its own cost for the group.
Owners added a subscriber: Restricted Owners Package.Aug 21 2019, 6:13 PM
Freagarach removed a subscriber: Restricted Owners Package.Aug 21 2019, 6:13 PM
Stan added inline comments.Aug 21 2019, 6:27 PM
binaries/data/mods/public/simulation/templates/structures/athen_civil_centre.xml
15

broken ident :)

Freagarach added inline comments.Aug 22 2019, 7:07 AM
binaries/data/mods/public/simulation/templates/structures/athen_civil_centre.xml
15

Yes, that way I can immediately see I need to remove that when done ;)

Freagarach updated this revision to Diff 9438.Aug 22 2019, 9:34 AM
Freagarach edited the summary of this revision. (Show Details)

Made stuff useful for training optional, so that the cost can also be calculated based on the individual entites to spawn. I think however that that is not viable, since one can deduce from the amount of resources subtracted which entities are to spawn. So I guess I'll have to revert this change when I find the time again to make Cost obligatory again.

Owners added a subscriber: Restricted Owners Package.Aug 22 2019, 9:34 AM
Freagarach removed a subscriber: Restricted Owners Package.Aug 22 2019, 9:35 AM
Freagarach updated this revision to Diff 9448.Aug 22 2019, 10:12 PM
Freagarach edited the summary of this revision. (Show Details)
Freagarach edited the test plan for this revision. (Show Details)
  • Reverted cost calculation.
  • Finish the spawning
    • Ugly but working. Feedback appreciated!
Freagarach changed the visibility from "Subscribers" to "Public (No Login Required)".Aug 22 2019, 10:12 PM
Vulcan added a comment.Aug 22 2019, 10:12 PM

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

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

Harbormaster failed remote builds in B9012: Diff 9448!Aug 22 2019, 10:12 PM
Freagarach added a reviewer: Restricted Owners Package.Aug 22 2019, 11:22 PM
Freagarach edited the summary of this revision. (Show Details)Aug 23 2019, 5:02 PM
Freagarach updated this revision to Diff 9537.Aug 30 2019, 5:28 PM

Split template into a parent and specific, to allow right-clicking on the icon.
For now only the cost is shown, I would like to elaborate with e.g. the individual units, their chance of creation and whether it is mutually exclusive (when true, show the chance) and perhaps some more information.

Vulcan added a comment.Aug 30 2019, 5:34 PM

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 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
| 120| 120| 	["units/athen_cavalry_javelinist_b", "units/iber_support_female_citizen"]
| 121| 121| );
| 122| 122| TS_ASSERT_UNEVAL_EQUALS(cmpProductionQueue.GetTechnologiesList(), ["phase_town_athen",
| 123|    |-                                                                   "phase_city_athen"]
|    | 123|+	"phase_city_athen"]
| 124| 124| );
| 125| 125| 
| 126| 126| AddMock(playerEntityID, IID_TechnologyManager, {

binaries/data/mods/public/simulation/components/tests/test_ProductionQueue.js
| 206| »   »   "GetTemplateData":·playerID·=>·({})
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'playerID' is already declared in the upper scope.
|    | [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
|  87|  87| 		// If the elements are still strings, split them by space or by '+'
|  88|  88| 		if (typeof sublist == "string")
|  89|  89| 			sublist = sublist.split(/[+\s]+/);
|  90|    |-		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1)
|  91|    |-		                    || (c[0] != "!" && classes.indexOf(c) != -1)))
|    |  90|+		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) ||
|    |  91|+		                    (c[0] != "!" && classes.indexOf(c) != -1)))
|  92|  92| 			return true;
|  93|  93| 	}
|  94|  94| 

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

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 825| »   »   »   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
| 194| »   for·(var·i·in·techList)
|    | [NORMAL] JSHintBear:
|    | 'i' is already defined.

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

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

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

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

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

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 737| »   »   »   template·=·cmpGuiInterface.GetTemplateData(item.player,·item.unitTemplate);
|    | [NORMAL] JSHintBear:
|    | 'template' used out of scope.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 739| »   »   »   if·(template.composition)
|    | [NORMAL] JSHintBear:
|    | 'template' used out of scope.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 743| »   »   »   »   »   this.compositionEntries·=·template.composition.Entries;
|    | [NORMAL] JSHintBear:
|    | 'template' used out of scope.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 744| »   »   »   »   »   if·(template.composition.MutuallyExclusive·!=·"false")
|    | [NORMAL] JSHintBear:
|    | 'template' used out of scope.
Executing section cli...

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

Harbormaster completed remote builds in B9115: Diff 9537.Aug 30 2019, 5:34 PM
Stan added inline comments.Aug 30 2019, 8:32 PM
binaries/data/mods/public/simulation/components/tests/test_ProductionQueue.js
206

Why the parenthesis ?

Freagarach marked an inline comment as done.Aug 30 2019, 9:22 PM
Freagarach added inline comments.
binaries/data/mods/public/simulation/components/tests/test_ProductionQueue.js
206

Trail and error says it is needed. I have no clue as to the reason. It occurs more often in tests.

Freagarach updated this revision to Diff 9540.Aug 30 2019, 9:22 PM
Freagarach marked an inline comment as done.
Freagarach edited the summary of this revision. (Show Details)

Some useful information.

Owners added a subscriber: Restricted Owners Package.Aug 30 2019, 9:22 PM
Vulcan added a comment.Aug 30 2019, 9:26 PM

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 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
| 120| 120| 	["units/athen_cavalry_javelinist_b", "units/iber_support_female_citizen"]
| 121| 121| );
| 122| 122| TS_ASSERT_UNEVAL_EQUALS(cmpProductionQueue.GetTechnologiesList(), ["phase_town_athen",
| 123|    |-                                                                   "phase_city_athen"]
|    | 123|+	"phase_city_athen"]
| 124| 124| );
| 125| 125| 
| 126| 126| AddMock(playerEntityID, IID_TechnologyManager, {

binaries/data/mods/public/simulation/components/tests/test_ProductionQueue.js
| 206| »   »   "GetTemplateData":·playerID·=>·({})
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'playerID' is already declared in the upper scope.
|    | [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
|  87|  87| 		// If the elements are still strings, split them by space or by '+'
|  88|  88| 		if (typeof sublist == "string")
|  89|  89| 			sublist = sublist.split(/[+\s]+/);
|  90|    |-		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1)
|  91|    |-		                    || (c[0] != "!" && classes.indexOf(c) != -1)))
|    |  90|+		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) ||
|    |  91|+		                    (c[0] != "!" && classes.indexOf(c) != -1)))
|  92|  92| 			return true;
|  93|  93| 	}
|  94|  94| 

binaries/data/mods/public/globalscripts/Templates.js
|  91| »   »   ····················||·(c[0]·!=·"!"·&&·classes.indexOf(c)·!=·-1)))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 389| 389| function getRepairTimeTooltip(entState)
| 390| 390| {
| 391| 391| 	return sprintf(translate("%(label)s %(details)s"), {
| 392|    |-			"label": headerFont(translate("Number of repairers:")),
|    | 392|+		"label": headerFont(translate("Number of repairers:")),
| 393| 393| 			"details": entState.repairable.numBuilders
| 394| 394| 		}) + "\n" + (entState.repairable.numBuilders ?
| 395| 395| 		sprintf(translatePlural(
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 390| 390| {
| 391| 391| 	return sprintf(translate("%(label)s %(details)s"), {
| 392| 392| 			"label": headerFont(translate("Number of repairers:")),
| 393|    |-			"details": entState.repairable.numBuilders
|    | 393|+		"details": entState.repairable.numBuilders
| 394| 394| 		}) + "\n" + (entState.repairable.numBuilders ?
| 395| 395| 		sprintf(translatePlural(
| 396| 396| 			"Add another worker to speed up the repairs by %(second)s second.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 2.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 391| 391| 	return sprintf(translate("%(label)s %(details)s"), {
| 392| 392| 			"label": headerFont(translate("Number of repairers:")),
| 393| 393| 			"details": entState.repairable.numBuilders
| 394|    |-		}) + "\n" + (entState.repairable.numBuilders ?
|    | 394|+	}) + "\n" + (entState.repairable.numBuilders ?
| 395| 395| 		sprintf(translatePlural(
| 396| 396| 			"Add another worker to speed up the repairs by %(second)s second.",
| 397| 397| 			"Add another worker to speed up the repairs by %(second)s seconds.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 411| 411| function getBuildTimeTooltip(entState)
| 412| 412| {
| 413| 413| 	return sprintf(translate("%(label)s %(details)s"), {
| 414|    |-			"label": headerFont(translate("Number of builders:")),
|    | 414|+		"label": headerFont(translate("Number of builders:")),
| 415| 415| 			"details": entState.foundation.numBuilders
| 416| 416| 		}) + "\n" + (entState.foundation.numBuilders ?
| 417| 417| 		sprintf(translatePlural(
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 412| 412| {
| 413| 413| 	return sprintf(translate("%(label)s %(details)s"), {
| 414| 414| 			"label": headerFont(translate("Number of builders:")),
| 415|    |-			"details": entState.foundation.numBuilders
|    | 415|+		"details": entState.foundation.numBuilders
| 416| 416| 		}) + "\n" + (entState.foundation.numBuilders ?
| 417| 417| 		sprintf(translatePlural(
| 418| 418| 			"Add another worker to speed up the construction by %(second)s second.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 2.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 413| 413| 	return sprintf(translate("%(label)s %(details)s"), {
| 414| 414| 			"label": headerFont(translate("Number of builders:")),
| 415| 415| 			"details": entState.foundation.numBuilders
| 416|    |-		}) + "\n" + (entState.foundation.numBuilders ?
|    | 416|+	}) + "\n" + (entState.foundation.numBuilders ?
| 417| 417| 		sprintf(translatePlural(
| 418| 418| 			"Add another worker to speed up the construction by %(second)s second.",
| 419| 419| 			"Add another worker to speed up the construction by %(second)s seconds.",

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 825| »   »   »   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
| 194| »   for·(var·i·in·techList)
|    | [NORMAL] JSHintBear:
|    | 'i' is already defined.

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

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

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

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

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

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 737| »   »   »   template·=·cmpGuiInterface.GetTemplateData(item.player,·item.unitTemplate);
|    | [NORMAL] JSHintBear:
|    | 'template' used out of scope.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 739| »   »   »   if·(template.composition)
|    | [NORMAL] JSHintBear:
|    | 'template' used out of scope.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 743| »   »   »   »   »   this.compositionEntries·=·template.composition.Entries;
|    | [NORMAL] JSHintBear:
|    | 'template' used out of scope.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 744| »   »   »   »   »   if·(template.composition.MutuallyExclusive·!=·"false")
|    | [NORMAL] JSHintBear:
|    | 'template' used out of scope.
Executing section cli...

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

Harbormaster completed remote builds in B9118: Diff 9540.Aug 30 2019, 9:26 PM
Freagarach edited the summary of this revision. (Show Details)Aug 30 2019, 9:31 PM
Freagarach edited the test plan for this revision. (Show Details)
Freagarach retitled this revision from [WIP] - Support training of groups of units. to Support training of groups of units..Aug 30 2019, 9:34 PM
wraitii edited reviewers, added: wraitii; removed: Restricted Owners Package.Sep 1 2019, 4:28 PM
wraitii added a subscriber: wraitii.

At a glance this looks interesting but too complex.

Freagarach mentioned this in D2175: [WIP] - Basic battalionish support..Sep 16 2019, 5:33 PM
wraitii removed a reviewer: wraitii.Jul 30 2020, 3:54 PM

Think I'm staying the mind that this might be worth for a mod but not the base game. I would vote towards abandoning.

wowgetoffyourcellphone added a subscriber: wowgetoffyourcellphone.Jan 24 2021, 11:10 PM

I could see this being very useful in differentiating some civs. Perhaps Romans, for example, only trains soldiers in these groups. Or maybe Champions train in groups. A group can include an officer (2x health, attack aura) and a bannerman (3x health, speed and health aura).

Revision Contents
Changeset List

PathSize
binaries/
data/
mods/
public/
globalscripts/
1 line
gui/
common/
34 lines
reference/
common/
1 line
simulation/
components/
27 lines
56 lines
tests/
3 lines
templates/
structures/
1 line
units/
compositions/
38 lines
24 lines

Diff 9540

View Options

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

Loading...
View Options

binaries/data/mods/public/gui/common/tooltips.js

Loading...
View Options

binaries/data/mods/public/gui/reference/common/draw.js

Loading...
View Options

binaries/data/mods/public/simulation/components/Identity.js

Loading...
View Options

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

Loading...
View Options

binaries/data/mods/public/simulation/components/tests/test_ProductionQueue.js

Loading...
View Options

binaries/data/mods/public/simulation/templates/structures/athen_civil_centre.xml

Loading...
View Options

binaries/data/mods/public/simulation/templates/units/compositions/

Loading...
View Options

binaries/data/mods/public/simulation/templates/units/compositions/athen_example.xml

Loading...
View Options

binaries/data/mods/public/simulation/templates/units/compositions/group.xml

Loading...
Wildfire Games · Phabricator