Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions src/modifiers/colormodifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,11 @@ class ColorModifier extends Modifier {
this.computeName = undefined;
}

runByProperty = (input: ModifierInput, output: ModifierOutput) => {
runByProperty = (
input: ModifierInput,
output: ModifierOutput,
everything: boolean = false,
) => {
Comment on lines +132 to +136
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The everything parameter is accepted but not used within the runByProperty method. This might be an oversight. If there are operations that should be forced to run regardless of caching, they should be conditioned on this flag, similar to how it's used in runByType.

if (!input.renderState.visualizer) {
return;
}
Expand Down Expand Up @@ -187,7 +191,9 @@ class ColorModifier extends Modifier {
everything: boolean = false,
) => {
if (
(this.previousColoringMethod === "type" && !output.colorsDirty) ||
(!everything &&
this.previousColoringMethod === "type" &&
!output.colorsDirty) ||
!input.renderState.visualizer
) {
return;
Expand All @@ -214,13 +220,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);
this.runByProperty(input, output, everything);
} else {
this.runByType(input, output);
this.runByType(input, output, everything);
}
};
}
Comment on lines +223 to +233
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While run has been correctly converted to a standard method to align with the base abstract class, the helper methods runByProperty and runByType remain as public arrow function properties. For better encapsulation and consistency, consider converting them to private methods.

}

export default ColorModifier;
8 changes: 4 additions & 4 deletions src/modifiers/modifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ interface ModifierProps {
active: boolean;
}

class Modifier {
abstract class Modifier {
public name: string;
public key: string;
public active: boolean;
Expand All @@ -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;
10 changes: 7 additions & 3 deletions src/modifiers/syncbondsmodifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -43,7 +47,7 @@ class SyncBondsModifier extends Modifier {
if (newBonds.mesh) {
newBonds.mesh.count = numBonds;
}
return newBonds;
return;
}

const bonds1Ptr = input.lammps.getBondsPosition1Pointer() / 4;
Expand All @@ -64,7 +68,7 @@ class SyncBondsModifier extends Modifier {
newBonds.mesh.count = numBonds;
}
newBonds.markNeedsUpdate();
};
}
}

export default SyncBondsModifier;
6 changes: 3 additions & 3 deletions src/modifiers/synccomputesmodifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -121,7 +121,7 @@ class SyncComputesModifier extends Modifier {
}
output.computes[name] = compute;
}
};
}
}

export default SyncComputesModifier;
6 changes: 3 additions & 3 deletions src/modifiers/syncfixesmodifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -116,7 +116,7 @@ class SyncFixesModifier extends Modifier {
}
output.fixes[name] = fix;
}
};
}
}

export default SyncFixesModifier;
9 changes: 4 additions & 5 deletions src/modifiers/syncparticlesmodifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ class SyncParticlesModifier extends Modifier {
super({ name, active });
}

run = (
run(
input: ModifierInput,
output: ModifierOutput,
everything: boolean = false,
) => {
_everything: boolean = false,
): void {
if (!this.active) {
if (output.particles) {
output.particles.count = 0;
Expand Down Expand Up @@ -75,8 +75,7 @@ class SyncParticlesModifier extends Modifier {
}

newParticles.markNeedsUpdate();
return newParticles;
};
}
}

export default SyncParticlesModifier;
6 changes: 3 additions & 3 deletions src/modifiers/syncvariablesmodifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -118,7 +118,7 @@ class SyncVariablesModifier extends Modifier {
}
output.variables[name] = variable;
}
};
}
}

export default SyncVariablesModifier;