From 81c79044b8cd544e4aada31d1567aaf6708c0621 Mon Sep 17 00:00:00 2001 From: arth-1 Date: Thu, 21 Nov 2024 16:40:28 +0530 Subject: [PATCH 1/6] FIX #2645 Better Faux Pickup Support Changes Made- Added logic to handle non-negative pickup values and support "faux pickups" like 1/3. Ensured notationPickup is called after setting the pickup value. Validated the pickup value before passing it to MeterActions.setPickup to prevent errors. Added input validation for pickup values in flow, ensuring only valid values are processed. --- js/blocks/MeterBlocks.js | 2 +- js/js-export/export.js | 3 ++- js/notation.js | 1 + 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/js/blocks/MeterBlocks.js b/js/blocks/MeterBlocks.js index 71a0f92ffa..86b83f09d0 100644 --- a/js/blocks/MeterBlocks.js +++ b/js/blocks/MeterBlocks.js @@ -1319,7 +1319,7 @@ function setupMeterBlocks(activity) { */ flow(args, logo, turtle, blk) { const arg0 = args[0]; - if (args.length !== 1 || typeof args[0] !== "number") { + if (args.length !== 1 || typeof args[0] !== "number" || arg0 < 0) { activity.errorMsg(NOINPUTERRORMSG, blk); return; } diff --git a/js/js-export/export.js b/js/js-export/export.js index 261234027a..c95dfe2edd 100644 --- a/js/js-export/export.js +++ b/js/js-export/export.js @@ -353,7 +353,8 @@ class MusicBlocks { set PICKUP(value) { const args = JSInterface.validateArgs("PICKUP", [value]); - Singer.MeterActions.setPickup(args[0], this.turIndex); + const validatedValue = Math.max(0, args[0]); + Singer.MeterActions.setPickup(validatedValue, this.turIndex); } get WHOLENOTESPLAYED() { diff --git a/js/notation.js b/js/notation.js index 239afbf173..af542731fd 100644 --- a/js/notation.js +++ b/js/notation.js @@ -349,6 +349,7 @@ class Notation { } else { if (this.activity.logo.runningLilypond) { obj = rationalToFraction(factor); + if (!obj) return; // Prevent undefined obj error this.activity.errorMsg( _("Lilypond cannot process pickup of ") + obj[0] + "/" + obj[1] ); From 5c5d796b32c5a4607c436e481aa746b70e173eec Mon Sep 17 00:00:00 2001 From: arth-1 Date: Thu, 21 Nov 2024 22:19:43 +0530 Subject: [PATCH 2/6] Updating rationalToFraction function to handle all the edge cases for better error handling --- js/notation.js | 1 - js/utils/utils.js | 14 ++++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/js/notation.js b/js/notation.js index af542731fd..a0c44df019 100644 --- a/js/notation.js +++ b/js/notation.js @@ -354,7 +354,6 @@ class Notation { _("Lilypond cannot process pickup of ") + obj[0] + "/" + obj[1] ); } - obj = rationalToFraction(1 - factor); for (let i = 0; i < obj[0]; i++) { this.activity.logo.updateNotation(["R"], obj[1], turtle, false, ""); diff --git a/js/utils/utils.js b/js/utils/utils.js index 289720921f..80096c94d3 100644 --- a/js/utils/utils.js +++ b/js/utils/utils.js @@ -1143,6 +1143,9 @@ readable-fractions/681534#681534 "3/5" */ + if (d === 0 || isNaN(d) || !isFinite(d)) { + return [0, 1]; + } let invert; if (d > 1) { @@ -1154,17 +1157,24 @@ readable-fractions/681534#681534 let df = 1.0; let top = 1; + const maxIterations = 1000; let bot = 1; - while (Math.abs(df - d) > 0.00000001) { + while (Math.abs(df - d) > 0.00000001 && iterations < maxIterations) { if (df < d) { top += 1; } else { bot += 1; - top = Math.floor(d * bot); + top = Math.round(d * bot); } df = top / bot; + iterations++; + } + + if (iterations === maxIterations) { + console.warn("rationalToFraction: Reached iteration limit"); + return [top, bot]; } if (bot === 0 || top === 0) { From d8e3fa2615c5648336d13a3f79a1bb58e1186cbc Mon Sep 17 00:00:00 2001 From: arth-1 Date: Thu, 21 Nov 2024 23:02:48 +0530 Subject: [PATCH 3/6] Forgot to commit the Syntax error fix --- js/utils/utils.js | 1 + 1 file changed, 1 insertion(+) diff --git a/js/utils/utils.js b/js/utils/utils.js index 80096c94d3..9e262f65a2 100644 --- a/js/utils/utils.js +++ b/js/utils/utils.js @@ -1157,6 +1157,7 @@ readable-fractions/681534#681534 let df = 1.0; let top = 1; + let iterations = 0 const maxIterations = 1000; let bot = 1; From 91db9920ad42a931f1d090411ee9567f33877e98 Mon Sep 17 00:00:00 2001 From: arth-1 Date: Thu, 21 Nov 2024 23:32:25 +0530 Subject: [PATCH 4/6] removed warning messgae --- js/utils/utils.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/js/utils/utils.js b/js/utils/utils.js index 9e262f65a2..0c11303ccc 100644 --- a/js/utils/utils.js +++ b/js/utils/utils.js @@ -1158,7 +1158,7 @@ readable-fractions/681534#681534 let df = 1.0; let top = 1; let iterations = 0 - const maxIterations = 1000; + const maxIterations = 10000; let bot = 1; while (Math.abs(df - d) > 0.00000001 && iterations < maxIterations) { @@ -1174,7 +1174,7 @@ readable-fractions/681534#681534 } if (iterations === maxIterations) { - console.warn("rationalToFraction: Reached iteration limit"); + //console.warn("rationalToFraction: Reached iteration limit"); return [top, bot]; } From 2d81543a1f03b6209de5c6f00f7a09f96eb88e01 Mon Sep 17 00:00:00 2001 From: arth-1 Date: Fri, 22 Nov 2024 11:38:11 +0530 Subject: [PATCH 5/6] removed unnecesarry warning --- js/notation.js | 7 ------- 1 file changed, 7 deletions(-) diff --git a/js/notation.js b/js/notation.js index a0c44df019..e652b2ccda 100644 --- a/js/notation.js +++ b/js/notation.js @@ -347,13 +347,6 @@ class Notation { this._notationStaging[turtle].push("pickup", beat); this._pickupPOW2[turtle] = true; } else { - if (this.activity.logo.runningLilypond) { - obj = rationalToFraction(factor); - if (!obj) return; // Prevent undefined obj error - this.activity.errorMsg( - _("Lilypond cannot process pickup of ") + obj[0] + "/" + obj[1] - ); - } obj = rationalToFraction(1 - factor); for (let i = 0; i < obj[0]; i++) { this.activity.logo.updateNotation(["R"], obj[1], turtle, false, ""); From 5bad696f47fc7e8f58b4c4b86a78422e82822c4c Mon Sep 17 00:00:00 2001 From: arth-1 Date: Sat, 23 Nov 2024 21:02:35 +0530 Subject: [PATCH 6/6] Minor Chances, Code improvement and Handling 1. made the updatePluginObj code shorter and combined categories making it easier to maintain in the long term, also ensured that the structure exists 2. minor improvements in mixedNumber function --- js/utils/utils.js | 138 ++++++++++++++++------------------------------ 1 file changed, 48 insertions(+), 90 deletions(-) diff --git a/js/utils/utils.js b/js/utils/utils.js index 0c11303ccc..4992676c43 100644 --- a/js/utils/utils.js +++ b/js/utils/utils.js @@ -831,66 +831,50 @@ const processRawPluginData = (activity, rawData) => { * @param {object} obj - The processed plugin data object. */ const updatePluginObj = (activity, obj) => { - if (obj === null) { + if (!obj) { return; } - for (const name in obj["PALETTEPLUGINS"]) { - activity.pluginObjs["PALETTEPLUGINS"][name] = obj["PALETTEPLUGINS"][name]; - } - - for (const name in obj["PALETTEFILLCOLORS"]) { - activity.pluginObjs["PALETTEFILLCOLORS"][name] = obj["PALETTEFILLCOLORS"][name]; - } - - for (const name in obj["PALETTESTROKECOLORS"]) { - activity.pluginObjs["PALETTESTROKECOLORS"][name] = obj["PALETTESTROKECOLORS"][name]; - } - - for (const name in obj["PALETTEHIGHLIGHTCOLORS"]) { - activity.pluginObjs["PALETTEHIGHLIGHTCOLORS"][name] = obj["PALETTEHIGHLIGHTCOLORS"][name]; - } - - for (const flow in obj["FLOWPLUGINS"]) { - activity.pluginObjs["FLOWPLUGINS"][flow] = obj["FLOWPLUGINS"][flow]; - } - - for (const arg in obj["ARGPLUGINS"]) { - activity.pluginObjs["ARGPLUGINS"][arg] = obj["ARGPLUGINS"][arg]; - } + // All are defined together + const categories = [ + "PALETTEPLUGINS", + "PALETTEFILLCOLORS", + "PALETTESTROKECOLORS", + "PALETTEHIGHLIGHTCOLORS", + "FLOWPLUGINS", + "ARGPLUGINS", + "BLOCKPLUGINS", + "ONLOAD", + "ONSTART", + "ONSTOP", + ]; - for (const block in obj["BLOCKPLUGINS"]) { - activity.pluginObjs["BLOCKPLUGINS"][block] = obj["BLOCKPLUGINS"][block]; - } + categories.forEach((category) => { + if (obj[category]) { + if (!activity.pluginObjs[category]) { + activity.pluginObjs[category] = {}; + } + Object.assign(activity.pluginObjs[category], obj[category]); // Merge objects + } + }); - if ("MACROPLUGINS" in obj) { - for (const macro in obj["MACROPLUGINS"]) { - activity.pluginObjs["MACROPLUGINS"][macro] = obj["MACROPLUGINS"][macro]; + if (obj["MACROPLUGINS"]) { + if (!activity.pluginObjs["MACROPLUGINS"]) { + activity.pluginObjs["MACROPLUGINS"] = {}; } + Object.assign(activity.pluginObjs["MACROPLUGINS"], obj["MACROPLUGINS"]); } - if ("GLOBALS" in obj) { + if (obj["GLOBALS"]) { if (!("GLOBALS" in activity.pluginObjs)) { activity.pluginObjs["GLOBALS"] = ""; } activity.pluginObjs["GLOBALS"] += obj["GLOBALS"]; } - if ("IMAGES" in obj) { + if (obj["IMAGES"]) { activity.pluginObjs["IMAGES"] = obj["IMAGES"]; } - - for (const name in obj["ONLOAD"]) { - activity.pluginObjs["ONLOAD"][name] = obj["ONLOAD"][name]; - } - - for (const name in obj["ONSTART"]) { - activity.pluginObjs["ONSTART"][name] = obj["ONSTART"][name]; - } - - for (const name in obj["ONSTOP"]) { - activity.pluginObjs["ONSTOP"][name] = obj["ONSTOP"][name]; - } }; /** @@ -1218,27 +1202,20 @@ let mixedNumber = (d) => { if (typeof d === "number") { const floor = Math.floor(d); - if (d > floor) { + const isFractional = d > floor; + + if (isFractional) { const obj = rationalToFraction(d - floor); - if (floor === 0) { - return obj[0] + "/" + obj[1]; - } else { - if (obj[0] === 1 && obj[1] === 1) { - return floor + 1; - } else { - if (obj[1] > 99) { - return d.toFixed(2); - } else { - return floor + " " + obj[0] + "/" + obj[1]; - } - } - } - } else { - return d.toString() + "/1"; + if (obj[1] > 99) return d.toFixed(2); // Limit denominator size. + + const fractionPart = `${obj[0]}/${obj[1]}`; + return floor === 0 ? fractionPart : `${floor} ${fractionPart}`; } - } else { - return d; + + return `${d}/1`; // Whole numbers. } + + return d.toString(); // Non-numeric inputs. }; /** @@ -1264,35 +1241,16 @@ let rationalSum = (a, b) => { } // Make sure a and b components are integers. - let obja0, objb0, obja1, objb1; - if (Math.floor(a[0]) !== a[0]) { - obja0 = rationalToFraction(a[0]); - } else { - obja0 = [a[0], 1]; - } - - if (Math.floor(b[0]) !== b[0]) { - objb0 = rationalToFraction(b[0]); - } else { - objb0 = [b[0], 1]; - } - - if (Math.floor(a[1]) !== a[1]) { - obja1 = rationalToFraction(a[1]); - } else { - obja1 = [a[1], 1]; - } - - if (Math.floor(b[1]) !== b[1]) { - objb1 = rationalToFraction(b[1]); - } else { - objb1 = [b[1], 1]; - } + const normalize = (arr) => { + if (!Number.isInteger(arr[0]) || !Number.isInteger(arr[1])) { + const fraction = rationalToFraction(arr[0] / arr[1]); + return [fraction[0], fraction[1]]; + } + return arr; + }; - a[0] = obja0[0] * obja1[1]; - a[1] = obja0[1] * obja1[0]; - b[0] = objb0[0] * objb1[1]; - b[1] = objb0[1] * objb1[0]; + a = normalize(a); + b = normalize(b); // Find the least common denomenator const lcd = LCD(a[1], b[1]);