Move unravelPhases function from structree to globalscripts.
ClosedPublic
Actions

Authored by s0600204 on Dec 28 2017, 9:38 PM.

Details

Reviewers

mimo

Commits

rP20750: Move unravelPhases function from structree to globalscripts for use in AI init.

Summary

So the AI can use it, hopefully improving its init code.

The changed AI code should generate what it did before, with the exception that it now supports phase-pairs. Or... at the very least recognises them.

Note: The technology files are present as an example of a phase-pair, and are not intended to be committed.

Depends on D1181

Test Plan

Test the AI's initialisation at game start. The AI should not throw any errors, nor have any phase-related problems in connection with this patch.

The structree should also load each civ correctly.

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

s0600204 created this revision.Dec 28 2017, 9:38 PM

s0600204 created this object with visibility "s0600204".

s0600204 created this object with edit policy "s0600204".

Owners added a subscriber: Restricted Owners Package.Dec 28 2017, 9:39 PM

s0600204 added a parent revision: D1181: Structure Tree - use phase techs to determine phase order..Dec 28 2017, 9:43 PM

Improve/simplify sorting.

(Changes are technically out-of-scope for a revision that relocates code, and could be backported to/included in the parent revision (1181) instead. Although they're likewise out-of-scope there.)

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

Harbormaster failed remote builds in B4247: Diff 4995!Dec 29 2017, 10:40 PM

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

binaries/data/mods/public/globalscripts/Technologies.js
| 226| »   »   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
| 249| »   »   »   »   »   return·false;
|    | [NORMAL] JSHintBear:
|    | Expected a 'break' statement before 'case'.

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

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

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

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.

Works as expected, both for ai and structree

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

52 ↗

(On Diff #4995)

To allow for possible tech disabling, and not always choose the bottom one, we could do

let pairTechName = this.playerData.disabledTechnologies[techData._template.top]) ?
                                  techData._template.bottom :
                                  this.playerData.disabledTechnologies[techData._template.bottom]) ?
                                  techData._template.top :
                                   pickRandom([techData._template.top, techData._template.bottom]);

instead of the loop

Use @mimo's suggestion to support selecting a phase at random from a pair of phases. (And to get rid of a loop.)

s0600204 added inline comments.Dec 30 2017, 6:48 PM

binaries/data/mods/public/simulation/ai/common-api/gamestate.js
52 ↗	(On Diff #4995)	I'm not a fan of nested ternary statements, but... yeah, it'd work.

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

Harbormaster failed remote builds in B4261: Diff 5003!Dec 30 2017, 7:36 PM

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

binaries/data/mods/public/globalscripts/Technologies.js
| 226| »   »   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
| 249| »   »   »   »   »   return·false;
|    | [NORMAL] JSHintBear:
|    | Expected a 'break' statement before 'case'.

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

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

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

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.

mimo added inline comments.Dec 30 2017, 8:14 PM

binaries/data/mods/public/simulation/ai/common-api/gamestate.js
52 ↗	(On Diff #4995)	In fact, i don't like them either :) but could not think of something else. In the meantime, i thought that name = pickRandom(new Array("techA", "techB").filter(tech => !this.playerData.disabledTechnologies[tech])) \|\| "techA"; would be more readable.