Changeset View
Standalone View
binaries/data/mods/public/gui/reference/common/core.js
| var g_SelectedCiv = ""; | var g_SelectedCiv = "gaia"; | ||||
elexis: (I've always wondered about the filename `core.js`. Sounds more like C++ stuff. Perhaps it… | |||||
| var g_CallbackSet = false; | var g_CallbackSet = false; | ||||
| /** | /** | ||||
| * Initialises civ data. Not done automatically in case a page doesn't want it. | * Initialises civ data. Not done automatically in case a page doesn't want it. | ||||
| */ | */ | ||||
| function init_civs() | function init_civs() | ||||
| { | { | ||||
| g_CivData = loadCivData(true); | g_CivData = loadCivData(true); | ||||
| ▲ Show 20 Lines • Show All 153 Lines • ▼ Show 20 Lines | if (template.ProductionQueue.Entities && template.ProductionQueue.Entities._string) | ||||
| templateLists.units.push(build); | templateLists.units.push(build); | ||||
| } | } | ||||
| if (template.ProductionQueue.Technologies && template.ProductionQueue.Technologies._string) | if (template.ProductionQueue.Technologies && template.ProductionQueue.Technologies._string) | ||||
| for (let research of template.ProductionQueue.Technologies._string.split(" ")) | for (let research of template.ProductionQueue.Technologies._string.split(" ")) | ||||
| if (templateLists.techs.indexOf(research) === -1) | if (templateLists.techs.indexOf(research) === -1) | ||||
| templateLists.techs.push(research); | templateLists.techs.push(research); | ||||
| } | } | ||||
Done Inline ActionsSorry but we should nuke the quadruplication before one of the copies is extended, D1192 elexis: Sorry but we should nuke the quadruplication before one of the copies is extended, D1192 | |||||
Done Inline ActionsThis revision now requires D1192 s0600204: This revision now requires D1192 | |||||
| if (template.WallSet) | if (template.WallSet) | ||||
| { | { | ||||
| for (let wSegm in template.WallSet.Templates) | for (let wSegm in template.WallSet.Templates) | ||||
| { | { | ||||
| let wEntities = getTemplateListsFromStructure(template.WallSet.Templates[wSegm]); | let wEntities = getTemplateListsFromStructure(template.WallSet.Templates[wSegm]); | ||||
| for (let entity of wEntities.techs) | for (let entity of wEntities.techs) | ||||
| if (templateLists.techs.indexOf(entity) === -1) | if (templateLists.techs.indexOf(entity) === -1) | ||||
| templateLists.techs.push(entity); | templateLists.techs.push(entity); | ||||
| } | } | ||||
| } | } | ||||
| return templateLists; | return templateLists; | ||||
| } | } | ||||
| /** | |||||
| * @param {string} objectName | |||||
| * @param {string} templateName | |||||
Not Done Inline Actionsfor example foo returns bar elexis: for example foo returns bar | |||||
| */ | |||||
Done Inline ActionsIt' easier for the reader if the type of the argument is always the same. For example one could alwasy pass a string and add a new variable for that one occurrence where currently an object is passed. elexis: It' easier for the reader if the type of the argument is always the same. For example one could… | |||||
Done Inline ActionsI don't see the SelectionGroupName dictating any template name:
So we may not rely on selection group names being template names or paths. Ranks were only the first use case, but (1) the name of the property SelectionGroupName, (2) all code that we have for it (grouping of elexis: I don't see the SelectionGroupName dictating any template name:
> "<element… | |||||
Not Done Inline ActionsRewritten function to not use SelectionGroupName at all. See if you prefer this new version. s0600204: Rewritten function to not use `SelectionGroupName` at all. See if you prefer this new version. | |||||
Done Inline ActionsShould define in one sentence what a template base is, since it's a new term. (Better would be to avoid it however). I didn't understand the function. How can templateName be a mirage, by "mirage|templatename"? elexis: Should define in one sentence what a template base is, since it's a new term. (Better would be… | |||||
| function setViewerOnPress(objectName, templateName) | |||||
| { | |||||
| Engine.GetGUIObjectByName(objectName).onPressRight = () => { | |||||
| Engine.PushGuiPage("page_viewer.xml", { | |||||
| "templateName": templateName, | |||||
Done Inline Actions2x \n elexis: 2x \n | |||||
| "civ": g_SelectedCiv, | |||||
| }); | |||||
| }; | |||||
| } | |||||
| /** | |||||
| * @param {string} objectName | |||||
| */ | |||||
| function unsetViewerOnPress(objectName) | |||||
| { | |||||
Done Inline ActionsTLDR, KISS For example: Returns the selection group name, or rank or foo of the given template. elexis: TLDR, KISS
For example: Returns the selection group name, or rank or foo of the given template. | |||||
| Engine.GetGUIObjectByName(objectName).onPressRight = () => {}; | |||||
| } | |||||
Done Inline Actionsunused function? elexis: unused function? | |||||
Done Inline ActionsIt's a convention to have the operators on the previous line. (I recall sanderd17 saying its to make it clear that there isnt a missing semicolon. But mostly having a globally unique codestyle is a more important incentive IMO).
elexis: It's a convention to have the operators on the previous line. (I recall sanderd17 saying its to… | |||||
Not Done Inline ActionsWeird. I'd think we'd want || on a new line for much the same reason we put { on a new line - makes it easier to notice that this line is a continuation of the conditional statement. Particularly as brackets are skipped here. Either way, it's not mentioned on the coding conventions wiki page. s0600204: Weird. I'd think we'd want `||` on a new line for much the same reason we put `{` on a new line… | |||||
Not Done Inline Actions(Can we avoid the !template.Identity.SelectionGroupName check or does it possibly complain about undefined?) elexis: (Can we avoid the `!template.Identity.SelectionGroupName` check or does it possibly complain… | |||||
Not Done Inline ActionsHmm... if I remove that check, then lines 219-26 become, in essence: if ({string} == {undefined})
[ignored]
if (not {something_false_in_vast_majority_of_cases})
return undefinedYeah... no. s0600204: Hmm... if I remove that check, then lines 219-26 become, in essence:
```
if ({string} ==… | |||||
Done Inline ActionsguiObjectName, comments can go. elexis: guiObjectName, comments can go.
The importance of JSdoc comments lies in minimizing the… | |||||
Done Inline Actions-, elexis: -, | |||||
Done Inline Actions(> 0 could be omitted I suppose?) elexis: (`> 0` could be omitted I suppose?) | |||||
(I've always wondered about the filename core.js. Sounds more like C++ stuff. Perhaps it should be common/reference.js?