Changeset View
Standalone View
binaries/data/mods/public/globalscripts/Resources.js
/** | /** | ||||
* This class provides a cache to all resource names and properties defined by the JSON files. | * This class provides a cache to all resource names and properties defined by the JSON files. | ||||
*/ | */ | ||||
function Resources() | function Resources() | ||||
{ | { | ||||
this.resourceData = []; | this.resourceData = []; | ||||
this.resourceDataObj = {}; | this.resourceDataObj = {}; | ||||
this.resourceCodes = []; | this.resourceCodes = []; | ||||
this.resourceNames = {}; | this.resourceNames = {}; | ||||
this.resourceCodesByProperty = {}; | |||||
for (let filename of Engine.ListDirectoryFiles("simulation/data/resources/", "*.json", false)) | for (let filename of Engine.ListDirectoryFiles("simulation/data/resources/", "*.json", false)) | ||||
{ | { | ||||
let data = Engine.ReadJSONFile(filename); | let data = Engine.ReadJSONFile(filename); | ||||
if (!data) | if (!data) | ||||
elexis: Confusing naming. resourceTributable, resourceTributableCodes | |||||
continue; | continue; | ||||
if (data.code != data.code.toLowerCase()) | if (data.code != data.code.toLowerCase()) | ||||
warn("Resource codes should use lower case: " + data.code); | warn("Resource codes should use lower case: " + data.code); | ||||
// Treasures are supported for every specified resource | // Treasures are supported for every specified resource | ||||
if (data.code == "treasure") | if (data.code == "treasure") | ||||
{ | { | ||||
error("Encountered resource with reserved keyword: " + data.code); | error("Encountered resource with reserved keyword: " + data.code); | ||||
continue; | continue; | ||||
} | } | ||||
this.resourceData.push(data); | this.resourceData.push(data); | ||||
this.resourceDataObj[data.code] = data; | this.resourceDataObj[data.code] = data; | ||||
this.resourceCodes.push(data.code); | this.resourceCodes.push(data.code); | ||||
this.resourceNames[data.code] = data.name; | this.resourceNames[data.code] = data.name; | ||||
for (let subres in data.subtypes) | for (let subres in data.subtypes) | ||||
this.resourceNames[subres] = data.subtypes[subres]; | this.resourceNames[subres] = data.subtypes[subres]; | ||||
Done Inline ActionsI think it could be replaced by if (!data.canExchange || data.canExchange == true) same for the below statements. You can also probably find a way to inline the else with that. Stan: I think it could be replaced by if (!data.canExchange || data.canExchange == true) same for… | |||||
Done Inline ActionsI've tried that, but since data.canExchange is a bool, it will return true if "canExchange":false. That took me a looong while to circumvent. Any tips on how to make this cleaner is welcome ;) Freagarach: I've tried that, but since data.canExchange is a bool, it will return true if "canExchange"… | |||||
Done Inline Actions@Nescio is right, it's better to specify in the resource.json if the resource is tradeable, barterable, and tributable. This way if you forget you will get a nice error like you did on the forums. Then assume the value exists. Also can exchange is useless cause it's basically the combination of the three others. this.resourceTradable[data.code] = data.canTrade; if (data.canTrade == true) { this.resourceTradables.push(data.code); } this.resourceBarterable[data.code] = data.canBarter; if (data.canBarter == true) { this.canBarter.push(data.code); } this.resourceTributable[data.code] = data.canTribute; if (data.canTribute == true) { this.resourceTributables.push(data.code); } Stan: @Nescio is right, it's better to specify in the resource.json if the resource is tradeable… | |||||
Done Inline Actions
Sounds like missing parenthesis https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Operator_Precedence if (typeof(data.canExchange) == "undefined" ? true : data.canExchange == true) If the property was not defined for some resources, then one can use if (!!data.canExchange) to circumvent the undefined warning. elexis: > I've tried that, but since data.canExchange is a bool, it will return true if "canExchange"… | |||||
for (let property in data.properties) | |||||
{ | |||||
if (!this.resourceCodesByProperty[data.properties[property]]) | |||||
this.resourceCodesByProperty[data.properties[property]] = []; | |||||
this.resourceCodesByProperty[data.properties[property]].push(data.code); | |||||
} | |||||
} | } | ||||
Done Inline Actions-{} elexis: -{} | |||||
Done Inline ActionsI am not sure about that. Looks like this is for line 12. Silier: I am not sure about that. Looks like this is for line 12. | |||||
Done Inline Actions(cant read) elexis: (cant read) | |||||
// Sort arrays by specified order | // Sort arrays by specified order | ||||
let resSort = (a, b) => | let resDataSort = (a, b) => a.order < b.order ? -1 : a.order > b.order ? +1 : 0; | ||||
wraitiiUnsubmitted Done Inline Actionslinter complained if we indented. wraitii: linter complained if we indented. | |||||
a.order < b.order ? -1 : | let resSort = (a, b) => resDataSort( | ||||
a.order > b.order ? +1 : 0; | |||||
this.resourceData.sort(resSort); | |||||
this.resourceCodes.sort((a, b) => resSort( | |||||
this.resourceData.find(resource => resource.code == a), | this.resourceData.find(resource => resource.code == a), | ||||
this.resourceData.find(resource => resource.code == b) | this.resourceData.find(resource => resource.code == b) | ||||
)); | ); | ||||
this.resourceData.sort(resDataSort); | |||||
this.resourceCodes.sort(resSort); | |||||
for (let property in this.resourceCodesByProperty) | |||||
this.resourceCodesByProperty[property].sort(resSort); | |||||
deepfreeze(this.resourceData); | deepfreeze(this.resourceData); | ||||
deepfreeze(this.resourceDataObj); | deepfreeze(this.resourceDataObj); | ||||
deepfreeze(this.resourceCodes); | deepfreeze(this.resourceCodes); | ||||
deepfreeze(this.resourceNames); | deepfreeze(this.resourceNames); | ||||
Done Inline Actionsindent issue. wraitii: indent issue. | |||||
Done Inline ActionsYou should be able to just do this.resourceCodesByProperty[property].sort(resSort) if you move this.resourceData.find(resource => resource.code == X) to that variable, and then change also right above. wraitii: You should be able to just do `this.resourceCodesByProperty[property].sort(resSort)` if you… | |||||
Done Inline ActionsWell, I kind of got it working, is it okay like this? Freagarach: Well, I kind of got it working, is it okay like this?
I've done it like this because the… | |||||
deepfreeze(this.resourceCodesByProperty); | |||||
} | } | ||||
/** | /** | ||||
* Returns the objects defined in the JSON files for all availbale resources, | * Returns the objects defined in the JSON files for all available resources, | ||||
Done Inline Actions{ and } on a separate line if (x) is the same as as Stan pointed out elexis: { and } on a separate line
if (x)
y= true
else
y= false
is the same as
y = x
as Stan pointed… | |||||
* ordered as defined in these files. | * ordered as defined in these files. | ||||
*/ | */ | ||||
Done Inline ActionsCould not be deepfreezed (because it is no object nor an array?), it would be fun if the currency could be changed in game, so I don't mind ;) Freagarach: Could not be deepfreezed (because it is no object nor an array?), it would be fun if the… | |||||
Resources.prototype.GetResources = function() | Resources.prototype.GetResources = function() | ||||
{ | { | ||||
Done Inline ActionsStill triplicated. Stan: Still triplicated. | |||||
return this.resourceData; | return this.resourceData; | ||||
Done Inline ActionsShould be possible to remove the duplication elexis: Should be possible to remove the duplication | |||||
Done Inline ActionsYep could definitely be made in a function. Stan: Yep could definitely be made in a function. | |||||
}; | }; | ||||
/** | /** | ||||
* Returns the object defined in the JSON file for the given resource. | * Returns the object defined in the JSON file for the given resource. | ||||
*/ | */ | ||||
Resources.prototype.GetResource = function(type) | Resources.prototype.GetResource = function(type) | ||||
{ | { | ||||
return this.resourceDataObj[type]; | return this.resourceDataObj[type]; | ||||
}; | }; | ||||
/** | /** | ||||
* Returns an array containing all resource codes ordered as defined in the resource files. | * Returns an array containing all resource codes ordered as defined in the resource files. | ||||
* For example ["food", "wood", "stone", "metal"]. | * @param {string} property - the property e.g. ("tradable", "tributable" etc.) that the resource ought to have | ||||
* @return {string[]} - data of the form [ "food", "wood", ... ] | |||||
*/ | */ | ||||
Resources.prototype.GetCodes = function() | Resources.prototype.GetCodes = function(property) | ||||
{ | { | ||||
if (!property) | |||||
return this.resourceCodes; | return this.resourceCodes; | ||||
return this.resourceCodesByProperty[property] || []; | |||||
Done Inline ActionsCheck property ? X : Y rather, and then you don't need = undefined above wraitii: Check `property ? X : Y` rather, and then you don't need ` = undefined` above | |||||
Done Inline ActionsIt appeared that the = undefined was not necessary at all,,, Freagarach: It appeared that the = undefined was not necessary at all,,, | |||||
FreagarachAuthorUnsubmitted Done Inline ActionsIsn't the "|| [ ]" already covered by "if (!property)"? Freagarach: Isn't the "|| [ ]" already covered by "if (!property)"? | |||||
wraitiiUnsubmitted Done Inline ActionsNo, what this does is return empty if we pass a property that doesn't exist (see tests). If it wasn't there I think it would be a JS error. wraitii: No, what this does is return empty if we pass a property that doesn't exist (see tests). If it… | |||||
FreagarachAuthorUnsubmitted Done Inline ActionsAh, okay, then it is probably best to also alter the summary to reflect this ;) Freagarach: Ah, okay, then it is probably best to also alter the summary to reflect this ;) | |||||
wraitiiUnsubmitted Done Inline ActionsQuite right :p I think it's saner to return nothing than everything, since that seems more ilike with what one would expect. wraitii: Quite right :p
/me needs to read summaries more attentively.
I think it's saner to return… | |||||
FreagarachAuthorUnsubmitted Done Inline ActionsAgreed! Freagarach: Agreed! | |||||
}; | }; | ||||
/** | /** | ||||
* Returns an object mapping resource codes to translatable resource names. Includes subtypes. | * Returns an object mapping resource codes to translatable resource names. Includes subtypes. | ||||
* For example { "food": "Food", "fish": "Fish", "fruit": "Fruit", "metal": "Metal", ... } | * For example { "food": "Food", "fish": "Fish", "fruit": "Fruit", "metal": "Metal", ... } | ||||
*/ | */ | ||||
Resources.prototype.GetNames = function() | Resources.prototype.GetNames = function() | ||||
{ | { | ||||
return this.resourceNames; | return this.resourceNames; | ||||
}; | }; | ||||
Done Inline ActionsJSDOC here and for the functions below should use @return {type} Stan: JSDOC here and for the functions below should use @return {type} | |||||
Done Inline ActionsYep I forgot it again ;) Freagarach: Yep I forgot it again ;) | |||||
Done Inline ActionsIs the performance so bad that you need to cache them instead of returning this.GetTributable().filter(res => res).keys() ? Stan: Is the performance so bad that you need to cache them instead of returning
this.GetTributable… | |||||
Done Inline ActionsI guess not? It is even worse: I use the other function nowhere. I still fail to grasp how the => thing works,,, Freagarach: I guess not? It is even worse: I use the other function nowhere.
But would this.GetTributable(). | |||||
Done Inline Actions(res => res) is equals to this /** * @param {bool} a */ let f = function (a) { return a; } So in case of filter for each object of the array/object if the value is true, the object will be added in a new collection. Then keys will only get the food, wood metal strings. Stan: (res => res)
is equals to this
```lang=js
/**
* @param {bool} a
*/
let f = function (a)
{… | |||||
Done Inline Actions
Arrow functions is just a different syntax for functions as Stan mentioned. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/filter
(JS doesn't have collections afaik, only arrays and objects)
That's not legal JS, braces indicate an object, but content indicates an array, so [ "val1", "val2", ...]
When the resource-agnostic patch was done by s0600204 and me, we went for caching because there are only a handful of resources, so very few memory allocation and many references to resourceData, resourceDataObj, resourceCodes, resourceNames in GUI and simulation. Here it's only GUI, and only upon trade/barter dialog resizing only, and upon player Commands. Still not a problem, and code consistency isn't so bad. One must look at the actual callers to see if the caching makes any sense. (Actually doesn't look like it's really adding benefit, because the callers do tons of other stuff looping over resources already, but I can't go through these lines each now) elexis: > I still fail to grasp how the => thing works,,,
Arrow functions is just a different syntax… | |||||
Done Inline Actions
Doesn't the AI call these functions also? Freagarach: > Here it's only GUI, (...)
Doesn't the AI call these functions also? | |||||
Done Inline Actions[array] → {string[]} see http://usejsdoc.org/tags-param.html Stan: [array] → {string[]} see http://usejsdoc.org/tags-param.html |
Confusing naming. resourceTributable, resourceTributableCodes