Details

Reviewers

Commits

rP20551: Allow civ specific techs with {civ}

Trac Tickets

Summary

Currently, for civ specific techs like phasing, we have to add all of them in the ProductionQueue templates. That is quite heavy and not mod friendly: two mods adding new civs with specific phasing would not work simultaneously as expected.
This patch proposes another approach: templates contain only "tech_{civ}", and the code will replace {civ} by the corresponding civ (as is done for units) or fallback to "tech" if the civ one is not found in the data/technologies folder.
An existing use case is phasing (phase_town and phase_city are done), but this would also work for any tech.

Changes for the structree and the ai are also done in the patch.

Test Plan

Check that everything works as before

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mimo created this revision.Nov 8 2017, 6:58 PM

Owners added subscribers: Restricted Owners Package, Restricted Owners Package.Nov 8 2017, 6:58 PM

#4589 and #4588 seem related. I actually thought there was a diff for exactly this somewhere around, but I can't seem to find it, so maybe I'm just thinking of D707.

As for replacing _{civ} instead of just replacing {civ} that seems unfortunate. Is there any issue with having both tech_a and tech_a_{civ} in templates? Or just naming the generic one tech_a_ (which is admittedly ugly).

source/simulation2/system/ComponentManager.cpp
86 ↗	(On Diff #4126)	Since this is limited to files in simulation/data, I guess having that in the name would be nice.

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

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

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

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

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

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

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

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 224| »   »   var·template·=·cmpTechnologyManager.GetTechnologyTemplate(tech);
|    | [NORMAL] JSHintBear:
|    | 'template' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 253| »   »   »   ||·cmpTechnologyManager.IsTechnologyResearched(template.bottom)·||·cmpTechnologyManager.IsInProgress(template.bottom));
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 349| »   »   »   var·template·=·cmpDataTemplateManager.GetTechnologyTemplate(templateName);
|    | [NORMAL] JSHintBear:
|    | 'template' is already defined.

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

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

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

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

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 808| »   »   »   var·template·=·cmpTechnologyManager.GetTechnologyTemplate(item.technologyTemplate);
|    | [NORMAL] JSHintBear:
|    | 'template' is already defined.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 167| »   »   »   let·w·=·+right["@x"]·+·+right["@width"]/2·-·+left["@x"]·+·+left["@width"]/2;
|    | [NORMAL] JSHintBear:
|    | Confusing plusses.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 167| »   »   »   let·w·=·+right["@x"]·+·+right["@width"]/2·-·+left["@x"]·+·+left["@width"]/2;
|    | [NORMAL] JSHintBear:
|    | Confusing plusses.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 168| »   »   »   let·h·=·Math.max(+right["@z"]·+·+right["@depth"]/2,·+left["@z"]·+·+left["@depth"]/2)
|    | [NORMAL] JSHintBear:
|    | Confusing plusses.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 168| »   »   »   let·h·=·Math.max(+right["@z"]·+·+right["@depth"]/2,·+left["@z"]·+·+left["@depth"]/2)
|    | [NORMAL] JSHintBear:
|    | Confusing plusses.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 169| »   »   »   ······-·Math.min(+right["@z"]·-·+right["@depth"]/2,·+left["@z"]·-·+left["@depth"]/2);
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '-'; readers may interpret this as an expression boundary.

Harbormaster completed remote builds in B3638: Diff 4126.Nov 8 2017, 7:48 PM

In D1024#40093, @leper wrote:

#4589 and #4588 seem related. I actually thought there was a diff for exactly this somewhere around, but I can't seem to find it, so maybe I'm just thinking of D707.

Yes #4589 is the one! thanks. But there was no code linked as far as i can remember.

As for replacing _{civ} instead of just replacing {civ} that seems unfortunate. Is there any issue with having both tech_a and tech_a_{civ} in templates? Or just naming the generic one tech_a_ (which is admittedly ugly).

I don't really get your point: we either replace {civ} by its value or remove _{civ}. Do you see a problem with removing _{civ} ?

source/simulation2/system/ComponentManager.cpp
86 ↗	(On Diff #4126)	DataFileExists ?

mimo updated the Trac tickets for this revision.Nov 8 2017, 7:49 PM

I don't really get your point: we either replace {civ} by its value or remove _{civ}. Do you see a problem with removing _{civ} ?

We only ever changed {civ} to something else, that the current template naming scheme uses _ is not connected to that in any way. So it is the same as having foo{civ}bar and if we don't have that for the current value of the civ, we end up with fobar. Replacing {civ} by an empty string, or if that is considered somewhat ugly by eg default or generic that would most likely be easier to understand than changing anything apart from {civ}.

In D1024#40127, @leper wrote:

I don't really get your point: we either replace {civ} by its value or remove _{civ}. Do you see a problem with removing _{civ} ?

We only ever changed {civ} to something else, that the current template naming scheme uses _ is not connected to that in any way. So it is the same as having foo{civ}bar and if we don't have that for the current value of the civ, we end up with fobar. Replacing {civ} by an empty string, or if that is considered somewhat ugly by eg default or generic that would most likely be easier to understand than changing anything apart from {civ}.

OK agreed, i'll go with _generic

It would also be nice if we would have the same kind of replacement in all cases where we have {civ}.

updated version

• leper added inline comments.Nov 9 2017, 10:24 PM

binaries/data/mods/public/simulation/ai/petra/headquarters.js
2368 ↗	(On Diff #4132)	Looks like that shouldn't be in the diff.
binaries/data/mods/public/simulation/components/DataTemplateManager.js
65 ↗	(On Diff #4132)	Given that we are passing ".json" explicitly that name seems wrong. Why not DataFileExists?
binaries/data/mods/public/simulation/components/ProductionQueue.js
26 ↗	(On Diff #4132)	We should have a help text here that describes how `{civ}` is handled (and the fallback to generic). I guess doing something similar (the fallback that is) for Entities can be done later.
binaries/data/mods/public/simulation/data/technologies/phase_city_athen.json
11 ↗	(On Diff #4132)	We could just replace phase_city_generic, then the phase_city file wouldn't be needed anymore.
binaries/data/mods/public/simulation/data/technologies/phase_town_athen.json
11 ↗	(On Diff #4132)	Same here.
source/simulation2/system/ComponentManager.cpp
86 ↗	(On Diff #4126)	Yes.

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

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

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

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

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

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

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

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 224| »   »   var·template·=·cmpTechnologyManager.GetTechnologyTemplate(tech);
|    | [NORMAL] JSHintBear:
|    | 'template' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 253| »   »   »   ||·cmpTechnologyManager.IsTechnologyResearched(template.bottom)·||·cmpTechnologyManager.IsInProgress(template.bottom));
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 349| »   »   »   var·template·=·cmpDataTemplateManager.GetTechnologyTemplate(templateName);
|    | [NORMAL] JSHintBear:
|    | 'template' is already defined.

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

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

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

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

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 808| »   »   »   var·template·=·cmpTechnologyManager.GetTechnologyTemplate(item.technologyTemplate);
|    | [NORMAL] JSHintBear:
|    | 'template' is already defined.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 167| »   »   »   let·w·=·+right["@x"]·+·+right["@width"]/2·-·+left["@x"]·+·+left["@width"]/2;
|    | [NORMAL] JSHintBear:
|    | Confusing plusses.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 167| »   »   »   let·w·=·+right["@x"]·+·+right["@width"]/2·-·+left["@x"]·+·+left["@width"]/2;
|    | [NORMAL] JSHintBear:
|    | Confusing plusses.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 168| »   »   »   let·h·=·Math.max(+right["@z"]·+·+right["@depth"]/2,·+left["@z"]·+·+left["@depth"]/2)
|    | [NORMAL] JSHintBear:
|    | Confusing plusses.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 168| »   »   »   let·h·=·Math.max(+right["@z"]·+·+right["@depth"]/2,·+left["@z"]·+·+left["@depth"]/2)
|    | [NORMAL] JSHintBear:
|    | Confusing plusses.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 169| »   »   »   ······-·Math.min(+right["@z"]·-·+right["@depth"]/2,·+left["@z"]·-·+left["@depth"]/2);
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '-'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/ai/petra/headquarters.js
| 399| »   »   »   let·ratioMax·=·0.70·+·randFloat(0.,·0.1);
|    | [NORMAL] JSHintBear:
|

Harbormaster completed remote builds in B3647: Diff 4132.Nov 9 2017, 10:41 PM

mimo added inline comments.Nov 9 2017, 10:47 PM

binaries/data/mods/public/simulation/ai/petra/headquarters.js
2368 ↗	(On Diff #4132)	yep, a forgotten debug! but in fact could be useful for people testing the patch with ais :)
binaries/data/mods/public/simulation/components/DataTemplateManager.js
65 ↗	(On Diff #4132)	ok
binaries/data/mods/public/simulation/components/ProductionQueue.js
26 ↗	(On Diff #4132)	yes
binaries/data/mods/public/simulation/data/technologies/phase_city_athen.json
11 ↗	(On Diff #4132)	But then, we would have to replace all the requirements in all templates to phase_city_generic instead of phase_city, quite ugly.

update following leper's comments

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

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

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

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

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

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 224| »   »   var·template·=·cmpTechnologyManager.GetTechnologyTemplate(tech);
|    | [NORMAL] JSHintBear:
|    | 'template' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 253| »   »   »   ||·cmpTechnologyManager.IsTechnologyResearched(template.bottom)·||·cmpTechnologyManager.IsInProgress(template.bottom));
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 349| »   »   »   var·template·=·cmpDataTemplateManager.GetTechnologyTemplate(templateName);
|    | [NORMAL] JSHintBear:
|    | 'template' is already defined.

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

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

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

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

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 808| »   »   »   var·template·=·cmpTechnologyManager.GetTechnologyTemplate(item.technologyTemplate);
|    | [NORMAL] JSHintBear:
|    | 'template' is already defined.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 167| »   »   »   let·w·=·+right["@x"]·+·+right["@width"]/2·-·+left["@x"]·+·+left["@width"]/2;
|    | [NORMAL] JSHintBear:
|    | Confusing plusses.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 167| »   »   »   let·w·=·+right["@x"]·+·+right["@width"]/2·-·+left["@x"]·+·+left["@width"]/2;
|    | [NORMAL] JSHintBear:
|    | Confusing plusses.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 168| »   »   »   let·h·=·Math.max(+right["@z"]·+·+right["@depth"]/2,·+left["@z"]·+·+left["@depth"]/2)
|    | [NORMAL] JSHintBear:
|    | Confusing plusses.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 168| »   »   »   let·h·=·Math.max(+right["@z"]·+·+right["@depth"]/2,·+left["@z"]·+·+left["@depth"]/2)
|    | [NORMAL] JSHintBear:
|    | Confusing plusses.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 169| »   »   »   ······-·Math.min(+right["@z"]·-·+right["@depth"]/2,·+left["@z"]·-·+left["@depth"]/2);
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '-'; readers may interpret this as an expression boundary.

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

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
Relax-NG validity error : Extra element props in interleave
/public/art/actors/structures/carthaginians/stable_elephant.xml:0: Relax-NG validity error : Element variant failed to validate content
/public/art/actors/structures/carthaginians/stable_elephant.xml:0: Relax-NG validity error : Element group has extra content: variant
Relax-NG validity error : Extra element group in interleave
/public/art/actors/structures/carthaginians/stable_elephant.xml:0: Relax-NG validity error : Element actor failed to validate content

Harbormaster completed remote builds in B3654: Diff 4137.Nov 10 2017, 7:32 PM

add a missing word in a comment

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

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
Relax-NG validity error : Extra element props in interleave
/public/art/actors/structures/carthaginians/stable_elephant.xml:0: Relax-NG validity error : Element variant failed to validate content
/public/art/actors/structures/carthaginians/stable_elephant.xml:0: Relax-NG validity error : Element group has extra content: variant
Relax-NG validity error : Extra element group in interleave
/public/art/actors/structures/carthaginians/stable_elephant.xml:0: Relax-NG validity error : Element actor failed to validate content

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

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

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

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

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

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 224| »   »   var·template·=·cmpTechnologyManager.GetTechnologyTemplate(tech);
|    | [NORMAL] JSHintBear:
|    | 'template' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 253| »   »   »   ||·cmpTechnologyManager.IsTechnologyResearched(template.bottom)·||·cmpTechnologyManager.IsInProgress(template.bottom));
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 349| »   »   »   var·template·=·cmpDataTemplateManager.GetTechnologyTemplate(templateName);
|    | [NORMAL] JSHintBear:
|    | 'template' is already defined.

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

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

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

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

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 808| »   »   »   var·template·=·cmpTechnologyManager.GetTechnologyTemplate(item.technologyTemplate);
|    | [NORMAL] JSHintBear:
|    | 'template' is already defined.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 167| »   »   »   let·w·=·+right["@x"]·+·+right["@width"]/2·-·+left["@x"]·+·+left["@width"]/2;
|    | [NORMAL] JSHintBear:
|    | Confusing plusses.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 167| »   »   »   let·w·=·+right["@x"]·+·+right["@width"]/2·-·+left["@x"]·+·+left["@width"]/2;
|    | [NORMAL] JSHintBear:
|    | Confusing plusses.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 168| »   »   »   let·h·=·Math.max(+right["@z"]·+·+right["@depth"]/2,·+left["@z"]·+·+left["@depth"]/2)
|    | [NORMAL] JSHintBear:
|    | Confusing plusses.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 168| »   »   »   let·h·=·Math.max(+right["@z"]·+·+right["@depth"]/2,·+left["@z"]·+·+left["@depth"]/2)
|    | [NORMAL] JSHintBear:
|    | Confusing plusses.

binaries/data/mods/public/simulation/ai/common-api/entity.js
| 169| »   »   »   ······-·Math.min(+right["@z"]·-·+right["@depth"]/2,·+left["@z"]·-·+left["@depth"]/2);
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '-'; readers may interpret this as an expression boundary.

Harbormaster completed remote builds in B3660: Diff 4141.Nov 11 2017, 12:25 PM

bb added a subscriber: bb.Nov 27 2017, 9:33 PM

bb added inline comments.

binaries/data/mods/public/gui/reference/common/core.js
125 ↗	(On Diff #4141)	check only good for performance I guess?

mimo added inline comments.Nov 27 2017, 11:50 PM

binaries/data/mods/public/gui/reference/common/core.js
125 ↗	(On Diff #4141)	yep, no need to check for the techDataExists in that case.

bb accepted this revision.Nov 28 2017, 5:32 PM

bb added inline comments.

binaries/data/mods/public/simulation/components/ProductionQueue.js
26 ↗	(On Diff #4141)	As replacing also happens in the structree, mentioning it is the owners civ is not correct, thus keeping it general seems ok

This revision is now accepted and ready to land.Nov 28 2017, 5:32 PM

elexis added a subscriber: elexis.Nov 28 2017, 5:40 PM

elexis added inline comments.

source/simulation2/system/ComponentManager.cpp
1192 ↗	(On Diff #4141)	Lines could be merged afaics

mimo added inline comments.Nov 28 2017, 6:50 PM

binaries/data/mods/public/simulation/components/ProductionQueue.js
26 ↗	(On Diff #4141)	In fact, now that you say that, both are equivalent for the tech tree and it may be clearer for modders to say explicitely here that it is the civ of the building's owner? I'll change it when commiting later (if nobody objects before)
source/simulation2/system/ComponentManager.cpp
1192 ↗	(On Diff #4141)	ok

Closed by commit rP20551: Allow civ specific techs with {civ} (authored by mimo). · Explain WhyNov 28 2017, 9:43 PM

This revision was automatically updated to reflect the committed changes.

elexis mentioned this in rP20587: Check for Technology existence by looking up loaded technologies rather than….Dec 5 2017, 12:58 AM

elexis mentioned this in D11: Implement Single-Player (SP) campaign interface.Dec 5 2017, 2:58 PM

s0600204 mentioned this in D1181: Structure Tree - use phase techs to determine phase order..Dec 28 2017, 6:25 PM

Nescio mentioned this in D1472: Grouped civ bonuses into a single folder.Feb 16 2019, 10:51 AM

elexis mentioned this in rP23342: Fix maps with StartingTechnologies (Carthaginian Demo map) following….Jan 7 2020, 3:41 PM

civ specific techs
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 4427

ps/trunk/binaries/data/mods/public/gui/reference/common/core.js

ps/trunk/binaries/data/mods/public/gui/reference/common/load.js

ps/trunk/binaries/data/mods/public/simulation/ai/common-api/entity.js

ps/trunk/binaries/data/mods/public/simulation/ai/common-api/filters.js

ps/trunk/binaries/data/mods/public/simulation/ai/common-api/gamestate.js

ps/trunk/binaries/data/mods/public/simulation/components/DataTemplateManager.js

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

ps/trunk/binaries/data/mods/public/simulation/data/technologies/phase_city.json

ps/trunk/binaries/data/mods/public/simulation/data/technologies/phase_city_athen.json

ps/trunk/binaries/data/mods/public/simulation/data/technologies/phase_city_generic.json

ps/trunk/binaries/data/mods/public/simulation/data/technologies/phase_town.json

ps/trunk/binaries/data/mods/public/simulation/data/technologies/phase_town_athen.json

ps/trunk/binaries/data/mods/public/simulation/data/technologies/phase_town_generic.json

ps/trunk/binaries/data/mods/public/simulation/templates/structures/brit_crannog.xml

ps/trunk/binaries/data/mods/public/simulation/templates/template_structure_civic_civil_centre.xml

ps/trunk/binaries/data/mods/public/simulation/templates/template_structure_civic_civil_centre_military_colony.xml

ps/trunk/source/simulation2/system/ComponentManager.h

ps/trunk/source/simulation2/system/ComponentManager.cpp

civ specific techsClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 4427

ps/trunk/binaries/data/mods/public/gui/reference/common/core.js

ps/trunk/binaries/data/mods/public/gui/reference/common/load.js

ps/trunk/binaries/data/mods/public/simulation/ai/common-api/entity.js

ps/trunk/binaries/data/mods/public/simulation/ai/common-api/filters.js

ps/trunk/binaries/data/mods/public/simulation/ai/common-api/gamestate.js

ps/trunk/binaries/data/mods/public/simulation/components/DataTemplateManager.js

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

ps/trunk/binaries/data/mods/public/simulation/data/technologies/phase_city.json

ps/trunk/binaries/data/mods/public/simulation/data/technologies/phase_city_athen.json

ps/trunk/binaries/data/mods/public/simulation/data/technologies/phase_city_generic.json

ps/trunk/binaries/data/mods/public/simulation/data/technologies/phase_town.json

ps/trunk/binaries/data/mods/public/simulation/data/technologies/phase_town_athen.json

ps/trunk/binaries/data/mods/public/simulation/data/technologies/phase_town_generic.json

ps/trunk/binaries/data/mods/public/simulation/templates/structures/brit_crannog.xml

ps/trunk/binaries/data/mods/public/simulation/templates/template_structure_civic_civil_centre.xml

ps/trunk/binaries/data/mods/public/simulation/templates/template_structure_civic_civil_centre_military_colony.xml

ps/trunk/source/simulation2/system/ComponentManager.h

ps/trunk/source/simulation2/system/ComponentManager.cpp

civ specific techs
ClosedPublic
Actions

Revision Contents
Changeset List