From 2af020bab1f162adb9df12bbfd6b1036f8f46bb3 Mon Sep 17 00:00:00 2001 From: bb Date: Sun, 6 Mar 2022 21:01:36 +0000 Subject: [PATCH] Generalize fromInitAttributes loop Add deepCompare function to utilities to easy and reliably compare js variables. Comments By: Langbart, Stan, asterix Differential Revision: D4521 This was SVN commit r26583. --- .../globalscripts/tests/test_deepCompare.js | 39 ++++++++++++ .../data/mods/public/globalscripts/utility.js | 63 +++++++++++++++++++ .../public/gui/gamesettings/GameSettings.js | 40 ++++++++---- .../gui/gamesettings/attributes/PlayerAI.js | 6 +- .../gui/gamesettings/attributes/PlayerCiv.js | 6 +- .../gamesettings/attributes/PlayerColor.js | 6 +- .../gamesettings/attributes/PlayerCount.js | 10 ++- .../gui/gamesettings/attributes/PlayerName.js | 4 +- .../gui/gamesettings/attributes/PlayerTeam.js | 6 +- .../gamesettings/attributes/StartingCamera.js | 4 +- 10 files changed, 143 insertions(+), 41 deletions(-) create mode 100644 binaries/data/mods/public/globalscripts/tests/test_deepCompare.js diff --git a/binaries/data/mods/public/globalscripts/tests/test_deepCompare.js b/binaries/data/mods/public/globalscripts/tests/test_deepCompare.js new file mode 100644 index 0000000000..621d6c5c2c --- /dev/null +++ b/binaries/data/mods/public/globalscripts/tests/test_deepCompare.js @@ -0,0 +1,39 @@ +function test_deepCompare() +{ + TS_ASSERT(deepCompare({}, {})); + TS_ASSERT(deepCompare([], [])); + TS_ASSERT(deepCompare({ "foo": NaN }, { "foo": NaN })); + TS_ASSERT(!deepCompare({ "foo": Infinity }, { "foo": NaN })); + TS_ASSERT(!deepCompare({ "foo": NaN }, { "foo": Infinity })); + TS_ASSERT(!deepCompare({ "foo": NaN }, { "bar": NaN })); + TS_ASSERT(!deepCompare({ "foo": NaN }, { "foo": NaN, "bar": NaN })); + TS_ASSERT(!deepCompare({ "foo": NaN, "bar": NaN }, { "foo": NaN })); + + TS_ASSERT(deepCompare(undefined, undefined)); + TS_ASSERT(deepCompare([undefined], [undefined])); + TS_ASSERT(deepCompare({ "foo": undefined }, { "foo": undefined })); + TS_ASSERT(!deepCompare({ "foo": undefined }, {})); + + // Ordering in objects does not matter. + TS_ASSERT(deepCompare({ "foo": NaN, "bar": NaN }, { "foo": NaN, "bar": NaN })); + TS_ASSERT(deepCompare({ "foo": NaN, "bar": NaN }, { "bar": NaN, "foo": NaN })); + + // Test some other JS structures. + TS_ASSERT(deepCompare(new Set(), new Set())); + TS_ASSERT(deepCompare(new Map(), new Map())); + TS_ASSERT(!deepCompare(new Set(), new Map())); + TS_ASSERT(!deepCompare(new Set([0]), new Set([1]))); + TS_ASSERT(!deepCompare(new Set([undefined]), new Set([null]))); + TS_ASSERT(deepCompare(new Set([NaN]), new Set([NaN]))); + TS_ASSERT(deepCompare(new Set([0, 0, 0]), new Set([0]))); + + // Ordering in arrays is relevant. + TS_ASSERT(deepCompare([1, 2, 3], [1, 2, 3])); + TS_ASSERT(!deepCompare([1, 2, 3], [3, 1, 2])); + + // Some nestling. + TS_ASSERT(deepCompare({ "foo": new Set([1, 2, { "baz": Infinity }]), "bar": [new Set([9]), { "foo": [0, 1, 2] }] }, + { "foo": new Set([1, 2, { "baz": Infinity }]), "bar": [new Set([9]), { "foo": [0, 1, 2] }] })); +} + +test_deepCompare(); diff --git a/binaries/data/mods/public/globalscripts/utility.js b/binaries/data/mods/public/globalscripts/utility.js index 7751237446..bf8f6725ee 100644 --- a/binaries/data/mods/public/globalscripts/utility.js +++ b/binaries/data/mods/public/globalscripts/utility.js @@ -51,6 +51,69 @@ function heapsPermute(array, cloneFunc, callback) } } +/** + * Compare two variables recursively. This compares better than a quick + * JSON.stringify check since we also check undefineds, Sets and the like. + * + * @param first - Any javascript instance. + * @param second - Any javascript instance. + * @return {boolean} Whether first and second are equal. + */ +function deepCompare(first, second) +{ + // If the value of either variable is empty we can instantly compare + // them and check for equality. + if (first === null || first === undefined || + second === null || second === undefined) + return first === second; + + // Make sure both variables have the same type. + if (first.constructor !== second.constructor) + return false; + + // We know that the variables are of the same type so all we need to do is + // check what type one of the objects is, and then compare them. + + // Check numbers seperately. Make sure this works with NaN, Infinity etc. + if (typeof first == "number") + return uneval(first) === uneval(second); + + // Functions and RegExps must have the same reference to be equal. + if (first instanceof Function || first instanceof RegExp) + return first === second; + + // If we are comparing simple objects, we can just compare them. + if (first === second || first.valueOf() === second.valueOf()) + return true; + + // Dates would have equal valueOf if they are equal. + if (first instanceof Date) + return false; + + // From now we will assume we have some kind of objects so that + // we can do a recursive check of the keys and values. + if (!(first instanceof Object) || !(second instanceof Object)) + return false; + + // We cannot iterate over Sets, so collapse them to Arrays. + if (first instanceof Set) + { + first = Array.from(first); + second = Array.from(second); + } + + // Objects need the same number of keys in order to be equal. + const firstKeys = Object.keys(first); + if (firstKeys.length != Object.keys(second).length) + return false; + + // Make sure that all the object keys on this level of the object are the same. + // Finally, we pass all the values of our of each object recursively to + // make sure everything matches. + return Object.keys(second).every(i => firstKeys.includes(i)) && + firstKeys.every(i => deepCompare(first[i], second[i])); +} + /** * Removes prefixing path from a path or filename, leaving just the file's name (with extension) * diff --git a/binaries/data/mods/public/gui/gamesettings/GameSettings.js b/binaries/data/mods/public/gui/gamesettings/GameSettings.js index a482cf6fd7..f31cd35bd3 100644 --- a/binaries/data/mods/public/gui/gamesettings/GameSettings.js +++ b/binaries/data/mods/public/gui/gamesettings/GameSettings.js @@ -68,19 +68,33 @@ class GameSettings */ fromInitAttributes(attribs) { - // Settings depend on the map, but some settings - // may also be illegal for a given map. - // It would be good to validate, but just bulk-accept at the moment. - // There is some light order-dependency between settings. - // First deserialize the map, then the player #, then victory conditions, then the rest. - // TODO: there's a DAG in there. - this.map.fromInitAttributes(attribs); - this.playerCount.fromInitAttributes(attribs); - this.victoryConditions.fromInitAttributes(attribs); - for (let comp in this) - if (this[comp].fromInitAttributes && - comp !== "map" && comp !== "playerCount" && comp !== "victoryConditions") - this[comp].fromInitAttributes(attribs); + // Settings may depend on eachother. Some selections of settings + // might be illegal. So keep looping through all settings until + // we find something stable. + const components = Object.keys(this); + + // To check in the loop below if something change we just compare + // the entire component. However, we must ignore the "settings" + // keyword to avoid cyclic objects. + const getComponentData = comp => Object.keys(this[comp]).map(key => key == "settings" ? undefined : this[comp][key]); + + // When we have looped components.length + 1 times, we are considered stuck. + for (let i = 0; i <= components.length; ++i) + { + // Re-init if any setting was changed, to make sure dependencies are cleared. + let reInit = false; + for (const comp in this) + { + const oldSettings = clone(getComponentData(comp)); + if (this[comp].fromInitAttributes) + this[comp].fromInitAttributes(attribs); + reInit = reInit || !deepCompare(oldSettings, getComponentData(comp)); + } + if (!reInit) + return; + } + + throw new Error("Infinite loop initializing attributes detected, components: " + uneval(components)); } /** diff --git a/binaries/data/mods/public/gui/gamesettings/attributes/PlayerAI.js b/binaries/data/mods/public/gui/gamesettings/attributes/PlayerAI.js index 1e4e722d61..feefa200f4 100644 --- a/binaries/data/mods/public/gui/gamesettings/attributes/PlayerAI.js +++ b/binaries/data/mods/public/gui/gamesettings/attributes/PlayerAI.js @@ -37,10 +37,8 @@ GameSettings.prototype.Attributes.PlayerAI = class PlayerAI extends GameSetting { if (!this.getLegacySetting(attribs, "PlayerData")) return; - let pData = this.getLegacySetting(attribs, "PlayerData"); - if (this.values.length < pData.length) - this._resize(pData.length); - for (let i in pData) + const pData = this.getLegacySetting(attribs, "PlayerData"); + for (let i = 0; i < this.values.length; ++i) { // Also covers the "" case. if (!pData[i] || !pData[i].AI) diff --git a/binaries/data/mods/public/gui/gamesettings/attributes/PlayerCiv.js b/binaries/data/mods/public/gui/gamesettings/attributes/PlayerCiv.js index d8e4e536d8..b493c42afd 100644 --- a/binaries/data/mods/public/gui/gamesettings/attributes/PlayerCiv.js +++ b/binaries/data/mods/public/gui/gamesettings/attributes/PlayerCiv.js @@ -27,10 +27,8 @@ GameSettings.prototype.Attributes.PlayerCiv = class PlayerCiv extends GameSettin { if (!this.getLegacySetting(attribs, "PlayerData")) return; - let pData = this.getLegacySetting(attribs, "PlayerData"); - if (this.values.length < pData.length) - this._resize(pData.length); - for (let i in pData) + const pData = this.getLegacySetting(attribs, "PlayerData"); + for (let i = 0; i < this.values.length; ++i) if (pData[i] && pData[i].Civ) this.setValue(i, pData[i].Civ); } diff --git a/binaries/data/mods/public/gui/gamesettings/attributes/PlayerColor.js b/binaries/data/mods/public/gui/gamesettings/attributes/PlayerColor.js index c84d7691bd..8f0785edca 100644 --- a/binaries/data/mods/public/gui/gamesettings/attributes/PlayerColor.js +++ b/binaries/data/mods/public/gui/gamesettings/attributes/PlayerColor.js @@ -32,10 +32,8 @@ GameSettings.prototype.Attributes.PlayerColor = class PlayerColor extends GameSe { if (!this.getLegacySetting(attribs, "PlayerData")) return; - let pData = this.getLegacySetting(attribs, "PlayerData"); - if (this.values.length < pData.length) - this._resize(pData.length); - for (let i in pData) + const pData = this.getLegacySetting(attribs, "PlayerData"); + for (let i = 0; i < this.values.length; ++i) if (pData[i] && pData[i].Color) this.setColor(i, pData[i].Color); } diff --git a/binaries/data/mods/public/gui/gamesettings/attributes/PlayerCount.js b/binaries/data/mods/public/gui/gamesettings/attributes/PlayerCount.js index ce904579e2..8758bae85e 100644 --- a/binaries/data/mods/public/gui/gamesettings/attributes/PlayerCount.js +++ b/binaries/data/mods/public/gui/gamesettings/attributes/PlayerCount.js @@ -17,11 +17,8 @@ GameSettings.prototype.Attributes.PlayerCount = class PlayerCount extends GameSe fromInitAttributes(attribs) { - if (!this.getLegacySetting(attribs, "PlayerData")) - return; - let pData = this.getLegacySetting(attribs, "PlayerData"); - if (this.nbPlayers !== pData.length) - this.nbPlayers = pData.length; + if (this.settings.map.map !== undefined) + this.reloadFromLegacy(attribs); } onMapTypeChange(old) @@ -44,7 +41,8 @@ GameSettings.prototype.Attributes.PlayerCount = class PlayerCount extends GameSe { if (this.settings.map.type != "random") { - this.nbPlayers = this.settings.map.data.settings.PlayerData.length; + if (this.settings.map.data && this.settings.map.data.settings && this.settings.map.data.settings.PlayerData) + this.nbPlayers = this.settings.map.data.settings.PlayerData.length; return; } if (!data || !data.settings || data.settings.PlayerData === undefined) diff --git a/binaries/data/mods/public/gui/gamesettings/attributes/PlayerName.js b/binaries/data/mods/public/gui/gamesettings/attributes/PlayerName.js index 97fb6f0518..417cf987a1 100644 --- a/binaries/data/mods/public/gui/gamesettings/attributes/PlayerName.js +++ b/binaries/data/mods/public/gui/gamesettings/attributes/PlayerName.js @@ -31,9 +31,7 @@ GameSettings.prototype.Attributes.PlayerName = class PlayerName extends GameSett if (!this.getLegacySetting(attribs, "PlayerData")) return; const pData = this.getLegacySetting(attribs, "PlayerData"); - if (this.values.length < pData.length) - this._resize(pData.length); - for (const i in pData) + for (let i = 0; i < this.values.length; ++i) if (pData[i] && pData[i].Name !== undefined) { this.values[i] = pData[i].Name; diff --git a/binaries/data/mods/public/gui/gamesettings/attributes/PlayerTeam.js b/binaries/data/mods/public/gui/gamesettings/attributes/PlayerTeam.js index 395ad67a63..bac11efa6a 100644 --- a/binaries/data/mods/public/gui/gamesettings/attributes/PlayerTeam.js +++ b/binaries/data/mods/public/gui/gamesettings/attributes/PlayerTeam.js @@ -27,10 +27,8 @@ GameSettings.prototype.Attributes.PlayerTeam = class PlayerTeam extends GameSett { if (!this.getLegacySetting(attribs, "PlayerData")) return; - let pData = this.getLegacySetting(attribs, "PlayerData"); - if (this.values.length < pData.length) - this._resize(pData.length); - for (let i in pData) + const pData = this.getLegacySetting(attribs, "PlayerData"); + for (let i = 0; i < this.values.length; ++i) if (pData[i] && pData[i].Team !== undefined) this.setValue(i, pData[i].Team); } diff --git a/binaries/data/mods/public/gui/gamesettings/attributes/StartingCamera.js b/binaries/data/mods/public/gui/gamesettings/attributes/StartingCamera.js index c891ed2f06..28e977c7e5 100644 --- a/binaries/data/mods/public/gui/gamesettings/attributes/StartingCamera.js +++ b/binaries/data/mods/public/gui/gamesettings/attributes/StartingCamera.js @@ -28,9 +28,7 @@ GameSettings.prototype.Attributes.StartingCamera = class StartingCamera extends if (!this.getLegacySetting(attribs, "PlayerData")) return; const pData = this.getLegacySetting(attribs, "PlayerData"); - if (this.values.length < pData.length) - this._resize(pData.length); - for (const i in pData) + for (let i = 0; i < this.values.length; ++i) if (pData[i] && pData[i].StartingCamera !== undefined) { this.values[i] = pData[i].StartingCamera;