Changeset View
Standalone View
binaries/data/mods/mod/gui/common/mod.js
- This file was added.
| /** | |||||
elexis: I don't know if the author of this page endorses this file, but there was sufficient… | |||||
| * Check the mod compatibility between the saved game to be loaded and the engine | |||||
| */ | |||||
| function hasSameMods(metadata, engineInfo) | |||||
| { | |||||
| if (!metadata.mods || !engineInfo.mods) | |||||
Done Inline Actionsfalse statement. also all 3 can be removed IMO since the reader has to read anyway elexis: false statement. also all 3 can be removed IMO since the reader has to read anyway | |||||
| return false; | |||||
| // Ignore the "user" mod which is loaded for releases but not working-copies | |||||
Done Inline ActionsThis is going to be reused by lobby, selecting+launching mods, possibly gamesetup and other places. So it should pass metadata.mods directly instead of metadata IMHO elexis: This is going to be reused by lobby, selecting+launching mods, possibly gamesetup and other… | |||||
Done Inline ActionsI see, one would have to change SavedGames::GetEngineInfo. Seems reasonable to me actually. The savegame version shouldn't exist, the mod version should come from JSInterface_Mods.cpp not SavedGame, and the engine version should come from Main or something, not SavedGame since its also going to be reused from things other than savedgames. Maybe it doesn't improve too much to nuke that, dunno. elexis: I see, one would have to change `SavedGames::GetEngineInfo`. Seems reasonable to me actually. | |||||
Done Inline ActionsSorry, what do you mean? Imarok: Sorry, what do you mean? | |||||
Not Done Inline ActionsD1131 for the first part. elexis: D1131 for the first part.
Secondly `SavedGames::GetEngineInfo` should be moved to… | |||||
| let modsA = metadata.mods.filter(mod => mod[0] != "user"); | |||||
| let modsB = engineInfo.mods.filter(mod => mod[0] != "user"); | |||||
| if (modsA.length != modsB.length) | |||||
| return false; | |||||
| // Mods must be loaded in the same order. 0: modname, 1: modversion | |||||
| return modsA.every((mod, index) => [0, 1].every(i => mod[i] == modsB[index][i])); | |||||
elexisUnsubmitted Done Inline ActionsReading only this function, I still think it could become ["modname", "modversion"].every(name, i => ). But since the next function can't do that too, it's ok. elexis: Reading only this function, I still think it could become `["modname", "modversion"].every(name… | |||||
| } | |||||
| /** | |||||
| * Converts a list of mods and their version into a human-readable string. | |||||
| */ | |||||
Done Inline Actionscould do ["mods", "version"].every((name, i) => ) to kill the 0, 1 and the comment elexis: could do ["mods", "version"].every((name, i) => ) to kill the 0, 1 and the comment
| |||||
Done Inline ActionsThat would look really strange imho Imarok: That would look really strange imho | |||||
Done Inline Actionsprobably yes. Unfortunately there is more symmetry in the logic (should have the same properties in the same order) than reflected by the code (the first object has a different role than the second obejct since it's the subject of the array function). But I don't see a nicer way to code it sadly. elexis: probably yes.
Unfortunately there is more symmetry in the logic (should have the same… | |||||
| function modsToString(mods) | |||||
| { | |||||
Done Inline Actionscomment seems pointless since it doesnt explain whats in it. The function is obvious enough IMO elexis: comment seems pointless since it doesnt explain whats in it. The function is obvious enough IMO | |||||
| return mods.filter(mod => mod[0] != "mod" && mod[0] != "user" ).map(mod => | |||||
elexisUnsubmitted Done Inline Actions1 excess space elexis: 1 excess space | |||||
| sprintf(translateWithContext("Mod comparison", "%(mod)s (%(version)s)"), { | |||||
| "mod": mod[0], | |||||
| "version": mod[1] | |||||
| })).join(translate(", ")); | |||||
| } | |||||
Not Done Inline ActionsLikely already a tumult on transifex about that comma. Someone should check elexis: Likely already a tumult on transifex about that comma. Someone should check | |||||
| /** | |||||
| * Convert the required and active mods and their version into a humanreadable translated string. | |||||
| */ | |||||
| function comparedModsString(required, active) | |||||
Done Inline Actionsx == "" could also be !x elexis: x == "" could also be !x
still sadface because of the array rather than object. I guess every… | |||||
Done Inline Actionswhat if it's version 0 ? Imarok: what if it's version 0 ? | |||||
Done Inline ActionsThen your code should use === instead of == so that we don't require the reader to know the subtle details of the ES Abstract Equality Comparison Algorithm? elexis: Then your code should use `===` instead of `==` so that we don't require the reader to know the… | |||||
| { | |||||
Done Inline ActionstranslateWithContext or translation comment elexis: translateWithContext or translation comment | |||||
Not Done Inline ActionsI assume there will be the one guy who has all the right mods but the wrong order and doesn't comprehend what's going on. Maybe we should write a ticket to catch that case. elexis: I assume there will be the one guy who has all the right mods but the wrong order and doesn't… | |||||
Not Done Inline ActionsI guess he'll see, that the only difference between the required and active strings is the order... Imarok: I guess he'll see, that the only difference between the required and active strings is the… | |||||
| return sprintf(translateWithContext("Mod comparison", "Required: %(mods)s"), | |||||
| { "mods": modsToString(required) }) + "\n" + | |||||
| sprintf(translateWithContext("Mod comparison", "Active: %(mods)s"), | |||||
Done Inline Actions) + "\n" + ` the line above. I believe JSHint was the one complaining about operators not being at the end of a line (http://jshint.com/docs/options/#laxbreak) elexis: `) + "\n"` + ` the line above. I believe JSHint was the one complaining about operators not… | |||||
Not Done Inline ActionsNow what's the difference between Active, Loaded and Enabled? :-) elexis: Now what's the difference between Active, Loaded and Enabled? :-) | |||||
| { "mods": modsToString(active) }); | |||||
| } | |||||
Not Done Inline Actions
elexis: * same translation context?
* \n is an operator and operators should be at the end of the line… | |||||
Not Done Inline Actions
y
y
L33 ? Imarok: > * same translation context?
y
> * \n is an operator and operators should be at the end of the… | |||||
Done Inline ActionsAh, reading can perform wonders for those exercising it. elexis: Ah, reading can perform wonders for those exercising it. | |||||
Done Inline Actions); after the } elexis: ); after the } | |||||
Not Done Inline ActionsDon't forget eol-style native and to end the file with exactly one newline elexis: Don't forget eol-style native and to end the file with exactly one newline | |||||
I don't know if the author of this page endorses this file, but there was sufficient possibility to give feedback.