Skip to content

Commit

Permalink
Fix issue where "Replace Script" confirmation alert would appear when…
Browse files Browse the repository at this point in the history
… pasting sometimes even if the custom script hadn't been modified
  • Loading branch information
chrismaltby committed Sep 13, 2024
1 parent d4dfeca commit a70c864
Show file tree
Hide file tree
Showing 7 changed files with 168 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fix issue where adding a new song wouldn't warn about unsaved changes in current song
- Fix issue where adding a song with an already existing name wouldn't auto select the newly created song
- Fix issue where scene connection lines could get stuck in place if custom scripts that change scenes are called multiple times from the same scene
- Fix issue where "Replace Script" confirmation alert would appear when pasting sometimes even if the custom script hadn't been modified

## [4.1.2] - 2024-09-09

Expand Down
2 changes: 0 additions & 2 deletions src/shared/lib/entities/entitiesHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -720,7 +720,6 @@ const extractEntityStateSymbols = (state: EntitiesState) => {
...extractEntitySymbols(state.customEvents),
...extractEntitySymbols(state.music),
...extractEntitySymbols(state.sounds),
...extractEntitySymbols(state.scriptEvents),
];
};

Expand Down Expand Up @@ -758,7 +757,6 @@ export const ensureSymbolsUnique = (state: EntitiesState) => {
ensureEntitySymbolsUnique(state.customEvents, symbols);
ensureEntitySymbolsUnique(state.music, symbols);
ensureEntitySymbolsUnique(state.sounds, symbols);
ensureEntitySymbolsUnique(state.scriptEvents, symbols);
};

export const mergeAssetEntity = <T extends Asset & { inode: string }>(
Expand Down
1 change: 0 additions & 1 deletion src/shared/lib/entities/entitiesTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ export type ScriptEventArgs = Record<string, unknown>;
export type ScriptEvent = {
id: string;
command: string;
symbol?: string | undefined;
args?: ScriptEventArgs | undefined;
children?: Record<string, ScriptEvent[] | undefined> | undefined;
};
Expand Down
1 change: 0 additions & 1 deletion src/shared/lib/resources/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ const ScriptEvent = Type.Recursive((This) =>
Type.Object({
id: Type.String(),
command: Type.String(),
symbol: Type.Optional(Type.String()), // Include symbol property to match TypeScript
args: Type.Optional(ScriptEventArgs), // Matches ScriptEventArgs
children: Type.Optional(
Type.Record(
Expand Down
48 changes: 47 additions & 1 deletion src/shared/lib/scripts/scriptHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,24 @@ export const isNormalizedScriptEqual = (
const { args, command } = scriptEvent;
scriptBEvents.push({ args, command });
});
return isEqual(scriptAEvents, scriptBEvents);

// Exit early if script lengths differ
if (scriptAEvents.length !== scriptBEvents.length) {
return false;
}
// Otherwise check that every script event is equivalent
for (let i = 0; i < scriptAEvents.length; i++) {
const scriptEventA = scriptAEvents[i];
const scriptEventB = scriptBEvents[i];
if (scriptEventA.command !== scriptEventB.command) {
return false;
}
if (!isArgsEqual(scriptEventA.args ?? {}, scriptEventB.args ?? {})) {
return false;
}
}

return true;
};

export const generateScriptHash = (script: ScriptEvent[]): string => {
Expand All @@ -43,3 +60,32 @@ export const generateScriptHash = (script: ScriptEvent[]): string => {
});
return SparkMD5.hash(JSON.stringify(data));
};

// Compare args with undefined and missing args as equivalent
const isArgsEqual = (
a: Record<string, unknown>,
b: Record<string, unknown>
): boolean => {
const keys = new Set<string>([...Object.keys(a), ...Object.keys(b)]);
for (const key of keys) {
const hasKeyA = Object.prototype.hasOwnProperty.call(a, key);
const hasKeyB = Object.prototype.hasOwnProperty.call(b, key);
const valA = a[key];
const valB = b[key];

if (hasKeyA && hasKeyB) {
// Both objects have the key
if (!isEqual(valA, valB)) {
return false;
}
} else if (hasKeyA || hasKeyB) {
// One object has the key; check if its value is undefined
const val = hasKeyA ? valA : valB;
if (val !== undefined) {
// Values are different since one is not undefined
return false;
}
}
}
return true;
};
1 change: 0 additions & 1 deletion src/store/features/entities/entitiesState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3416,7 +3416,6 @@ const addScriptEvents: CaseReducer<
const newScriptEvent: ScriptEventNormalized = {
...scriptEventData,
id: action.payload.scriptEventIds[scriptEventIndex],
symbol: undefined,
};
if (scriptEventData.children) {
newScriptEvent.children = Object.keys(scriptEventData.children).reduce(
Expand Down
120 changes: 120 additions & 0 deletions test/helpers/scriptHelpers.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
import { ScriptEventNormalized } from "shared/lib/entities/entitiesTypes";
import { isNormalizedScriptEqual } from "shared/lib/scripts/scriptHelpers";

describe("isNormalizedScriptEqual", () => {
test("should consider empty scripts equal", () => {
const scriptA: string[] = [];
const lookupA: Record<string, ScriptEventNormalized> = {};
const scriptB: string[] = [];
const lookupB: Record<string, ScriptEventNormalized> = {};
expect(isNormalizedScriptEqual(scriptA, lookupA, scriptB, lookupB)).toEqual(
true
);
});

test("should consider identical scripts equal", () => {
const scriptA: string[] = ["event1"];
const lookupA: Record<string, ScriptEventNormalized> = {
event1: {
id: "event1",
command: "EVENT_TEST",
args: {
hello: "WORLD",
},
},
};
const scriptB: string[] = ["event1"];
const lookupB: Record<string, ScriptEventNormalized> = {
event1: {
id: "event1",
command: "EVENT_TEST",
args: {
hello: "WORLD",
},
},
};
expect(isNormalizedScriptEqual(scriptA, lookupA, scriptB, lookupB)).toEqual(
true
);
});

test("should consider functionally identical scripts equal even if ids dont match", () => {
const scriptA: string[] = ["event1"];
const lookupA: Record<string, ScriptEventNormalized> = {
event1: {
id: "event1",
command: "EVENT_TEST",
args: {
hello: "WORLD",
},
},
};
const scriptB: string[] = ["event2"];
const lookupB: Record<string, ScriptEventNormalized> = {
event2: {
id: "event2",
command: "EVENT_TEST",
args: {
hello: "WORLD",
},
},
};
expect(isNormalizedScriptEqual(scriptA, lookupA, scriptB, lookupB)).toEqual(
true
);
});

test("should consider functionally different scripts to not be equal", () => {
const scriptA: string[] = ["event1"];
const lookupA: Record<string, ScriptEventNormalized> = {
event1: {
id: "event1",
command: "EVENT_TEST",
args: {
hello: "WORLD",
},
},
};
const scriptB: string[] = ["event1"];
const lookupB: Record<string, ScriptEventNormalized> = {
event1: {
id: "event1",
command: "EVENT_TEST",
args: {
hello: "THERE",
},
},
};
expect(isNormalizedScriptEqual(scriptA, lookupA, scriptB, lookupB)).toEqual(
false
);
});

test("should consider missing args to be identical to undefined args", () => {
const scriptA: string[] = ["event1"];
const lookupA: Record<string, ScriptEventNormalized> = {
event1: {
id: "event1",
command: "EVENT_TEST",
args: {
hello: "WORLD",
goodbye: undefined,
},
},
};
const scriptB: string[] = ["event1"];
const lookupB: Record<string, ScriptEventNormalized> = {
event1: {
id: "event1",
command: "EVENT_TEST",
args: {
hello: "WORLD",
foo: undefined,
},
},
};
expect(isNormalizedScriptEqual(scriptA, lookupA, scriptB, lookupB)).toEqual(
true
);
});
});

0 comments on commit a70c864

Please sign in to comment.