From 7c23377348b0e02c6fb22de3c4f2ddea8df9b8fc Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 1 Nov 2025 11:25:11 +0000 Subject: [PATCH 1/5] refactor: Improve base Modifier class design - Convert Modifier class to abstract class - Make run() method abstract to enforce implementation in subclasses - All 6 modifier subclasses already override run(), so this is safe - Improves API clarity and TypeScript type safety Closes #187 --- src/modifiers/modifier.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/modifiers/modifier.ts b/src/modifiers/modifier.ts index 4c0e43bf..474d26be 100644 --- a/src/modifiers/modifier.ts +++ b/src/modifiers/modifier.ts @@ -5,7 +5,7 @@ interface ModifierProps { active: boolean; } -class Modifier { +abstract class Modifier { public name: string; public key: string; public active: boolean; @@ -16,10 +16,10 @@ class Modifier { this.active = active; } - run = ( + abstract run( input: ModifierInput, output: ModifierOutput, - everything: boolean = false, - ) => {}; + everything?: boolean + ): void; } export default Modifier; From b21686c7558f3a4d5b393dc4e4ea88f2d15038f6 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 1 Nov 2025 11:39:36 +0000 Subject: [PATCH 2/5] refactor: Update Modifier subclasses to implement abstract run() method - Convert arrow function properties to method declarations in all subclasses - Add missing 'everything' parameter to SyncBondsModifier and ColorModifier - Remove semicolons after method declarations for consistency - All 6 modifier subclasses now properly implement the abstract run() method Addresses PR review feedback about breaking changes --- src/modifiers/colormodifier.ts | 8 ++++++-- src/modifiers/syncbondsmodifier.ts | 8 ++++++-- src/modifiers/synccomputesmodifier.ts | 6 +++--- src/modifiers/syncfixesmodifier.ts | 6 +++--- src/modifiers/syncparticlesmodifier.ts | 6 +++--- src/modifiers/syncvariablesmodifier.ts | 6 +++--- 6 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/modifiers/colormodifier.ts b/src/modifiers/colormodifier.ts index a6ac6be9..1a0854ea 100644 --- a/src/modifiers/colormodifier.ts +++ b/src/modifiers/colormodifier.ts @@ -214,13 +214,17 @@ class ColorModifier extends Modifier { this.previousColoringMethod = "type"; }; - run = (input: ModifierInput, output: ModifierOutput) => { + run( + input: ModifierInput, + output: ModifierOutput, + everything: boolean = false, + ): void { if (this.computeName) { this.runByProperty(input, output); } else { this.runByType(input, output); } - }; + } } export default ColorModifier; diff --git a/src/modifiers/syncbondsmodifier.ts b/src/modifiers/syncbondsmodifier.ts index 16257a2c..f29c3d80 100644 --- a/src/modifiers/syncbondsmodifier.ts +++ b/src/modifiers/syncbondsmodifier.ts @@ -12,7 +12,11 @@ class SyncBondsModifier extends Modifier { super({ name, active }); } - run = (input: ModifierInput, output: ModifierOutput) => { + run( + input: ModifierInput, + output: ModifierOutput, + everything: boolean = false, + ): void { if (!this.active) { if (output.bonds) { output.bonds.count = 0; @@ -64,7 +68,7 @@ class SyncBondsModifier extends Modifier { newBonds.mesh.count = numBonds; } newBonds.markNeedsUpdate(); - }; + } } export default SyncBondsModifier; diff --git a/src/modifiers/synccomputesmodifier.ts b/src/modifiers/synccomputesmodifier.ts index 069df3c9..f40e867e 100644 --- a/src/modifiers/synccomputesmodifier.ts +++ b/src/modifiers/synccomputesmodifier.ts @@ -11,11 +11,11 @@ class SyncComputesModifier extends Modifier { super({ name, active }); } - run = ( + run( input: ModifierInput, output: ModifierOutput, everything: boolean = false, - ) => { + ): void { if (!this.active || !input.hasSynchronized) { return; } @@ -121,7 +121,7 @@ class SyncComputesModifier extends Modifier { } output.computes[name] = compute; } - }; + } } export default SyncComputesModifier; diff --git a/src/modifiers/syncfixesmodifier.ts b/src/modifiers/syncfixesmodifier.ts index d1d54716..857993ec 100644 --- a/src/modifiers/syncfixesmodifier.ts +++ b/src/modifiers/syncfixesmodifier.ts @@ -11,11 +11,11 @@ class SyncFixesModifier extends Modifier { super({ name, active }); } - run = ( + run( input: ModifierInput, output: ModifierOutput, everything: boolean = false, - ) => { + ): void { if (!this.active || !input.hasSynchronized) { return; } @@ -116,7 +116,7 @@ class SyncFixesModifier extends Modifier { } output.fixes[name] = fix; } - }; + } } export default SyncFixesModifier; diff --git a/src/modifiers/syncparticlesmodifier.ts b/src/modifiers/syncparticlesmodifier.ts index 7a4ad840..e5a92e22 100644 --- a/src/modifiers/syncparticlesmodifier.ts +++ b/src/modifiers/syncparticlesmodifier.ts @@ -12,11 +12,11 @@ class SyncParticlesModifier extends Modifier { super({ name, active }); } - run = ( + run( input: ModifierInput, output: ModifierOutput, everything: boolean = false, - ) => { + ): void { if (!this.active) { if (output.particles) { output.particles.count = 0; @@ -76,7 +76,7 @@ class SyncParticlesModifier extends Modifier { newParticles.markNeedsUpdate(); return newParticles; - }; + } } export default SyncParticlesModifier; diff --git a/src/modifiers/syncvariablesmodifier.ts b/src/modifiers/syncvariablesmodifier.ts index f84923d6..1773201c 100644 --- a/src/modifiers/syncvariablesmodifier.ts +++ b/src/modifiers/syncvariablesmodifier.ts @@ -11,11 +11,11 @@ class SyncVariablesModifier extends Modifier { super({ name, active }); } - run = ( + run( input: ModifierInput, output: ModifierOutput, everything: boolean = false, - ) => { + ): void { if (!this.active || !input.hasSynchronized) { return; } @@ -118,7 +118,7 @@ class SyncVariablesModifier extends Modifier { } output.variables[name] = variable; } - }; + } } export default SyncVariablesModifier; From 1bdcb29b452b914ec705917a1f568db88f2a02b0 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 1 Nov 2025 11:46:56 +0000 Subject: [PATCH 3/5] fix: Remove return statements to match void return type - Remove return newParticles from SyncParticlesModifier.run() - Change return newBonds to return in SyncBondsModifier.run() - Both methods now correctly return void to match abstract method signature --- src/modifiers/syncbondsmodifier.ts | 2 +- src/modifiers/syncparticlesmodifier.ts | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/modifiers/syncbondsmodifier.ts b/src/modifiers/syncbondsmodifier.ts index f29c3d80..28c7bcdb 100644 --- a/src/modifiers/syncbondsmodifier.ts +++ b/src/modifiers/syncbondsmodifier.ts @@ -47,7 +47,7 @@ class SyncBondsModifier extends Modifier { if (newBonds.mesh) { newBonds.mesh.count = numBonds; } - return newBonds; + return; } const bonds1Ptr = input.lammps.getBondsPosition1Pointer() / 4; diff --git a/src/modifiers/syncparticlesmodifier.ts b/src/modifiers/syncparticlesmodifier.ts index e5a92e22..fed8a44b 100644 --- a/src/modifiers/syncparticlesmodifier.ts +++ b/src/modifiers/syncparticlesmodifier.ts @@ -75,7 +75,6 @@ class SyncParticlesModifier extends Modifier { } newParticles.markNeedsUpdate(); - return newParticles; } } From 37194c6ff3e9b818fe4660d179d8798e69f9aaa3 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 1 Nov 2025 11:52:35 +0000 Subject: [PATCH 4/5] fix: Pass everything parameter to runByType in ColorModifier - Pass everything parameter from run() to runByType() - Update runByType to use everything to bypass caching logic - When everything=true, forces runByType to execute even if previousColoringMethod matches and colorsDirty is false Addresses PR review feedback about missing parameter propagation --- src/modifiers/colormodifier.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/modifiers/colormodifier.ts b/src/modifiers/colormodifier.ts index 1a0854ea..614b164f 100644 --- a/src/modifiers/colormodifier.ts +++ b/src/modifiers/colormodifier.ts @@ -187,7 +187,9 @@ class ColorModifier extends Modifier { everything: boolean = false, ) => { if ( - (this.previousColoringMethod === "type" && !output.colorsDirty) || + (!everything && + this.previousColoringMethod === "type" && + !output.colorsDirty) || !input.renderState.visualizer ) { return; @@ -222,7 +224,7 @@ class ColorModifier extends Modifier { if (this.computeName) { this.runByProperty(input, output); } else { - this.runByType(input, output); + this.runByType(input, output, everything); } } } From ed1e00a37a53ba27e4e23a4dd02fbfd8ca6041f2 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 1 Nov 2025 11:59:35 +0000 Subject: [PATCH 5/5] fix: Address PR review comments for modifier implementations - Pass everything parameter to runByProperty in ColorModifier for consistency - Update runByProperty signature to accept everything parameter - Prefix unused everything parameters with underscore in SyncBondsModifier and SyncParticlesModifier to signal intentional non-use Addresses medium-priority review feedback --- src/modifiers/colormodifier.ts | 8 ++++++-- src/modifiers/syncbondsmodifier.ts | 2 +- src/modifiers/syncparticlesmodifier.ts | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/modifiers/colormodifier.ts b/src/modifiers/colormodifier.ts index 614b164f..48224fe6 100644 --- a/src/modifiers/colormodifier.ts +++ b/src/modifiers/colormodifier.ts @@ -129,7 +129,11 @@ class ColorModifier extends Modifier { this.computeName = undefined; } - runByProperty = (input: ModifierInput, output: ModifierOutput) => { + runByProperty = ( + input: ModifierInput, + output: ModifierOutput, + everything: boolean = false, + ) => { if (!input.renderState.visualizer) { return; } @@ -222,7 +226,7 @@ class ColorModifier extends Modifier { everything: boolean = false, ): void { if (this.computeName) { - this.runByProperty(input, output); + this.runByProperty(input, output, everything); } else { this.runByType(input, output, everything); } diff --git a/src/modifiers/syncbondsmodifier.ts b/src/modifiers/syncbondsmodifier.ts index 28c7bcdb..f9400dd7 100644 --- a/src/modifiers/syncbondsmodifier.ts +++ b/src/modifiers/syncbondsmodifier.ts @@ -15,7 +15,7 @@ class SyncBondsModifier extends Modifier { run( input: ModifierInput, output: ModifierOutput, - everything: boolean = false, + _everything: boolean = false, ): void { if (!this.active) { if (output.bonds) { diff --git a/src/modifiers/syncparticlesmodifier.ts b/src/modifiers/syncparticlesmodifier.ts index fed8a44b..047bf5d6 100644 --- a/src/modifiers/syncparticlesmodifier.ts +++ b/src/modifiers/syncparticlesmodifier.ts @@ -15,7 +15,7 @@ class SyncParticlesModifier extends Modifier { run( input: ModifierInput, output: ModifierOutput, - everything: boolean = false, + _everything: boolean = false, ): void { if (!this.active) { if (output.particles) {