diff --git a/src/simulator/engine/d3-simulator-engine.ts b/src/simulator/engine/d3-simulator-engine.ts index 9d122f7..68f93e5 100644 --- a/src/simulator/engine/d3-simulator-engine.ts +++ b/src/simulator/engine/d3-simulator-engine.ts @@ -164,11 +164,9 @@ export type D3SimulatorEvents = { }; export class D3SimulatorEngine extends Emitter { - protected linkForce!: ForceLink>; - protected simulation!: Simulation; - // TODO(dlozic): Question: I removed readonly here since I'm using _initialSettings to reassign - // the settings in resetSettings(). Is this okay? Should I use Object.assign()? - protected settings: ID3SimulatorEngineSettings; + protected _linkForce!: ForceLink>; + protected _simulation!: Simulation; + protected _settings: ID3SimulatorEngineSettings; protected _edges: ISimulationEdge[] = []; protected _nodes: ISimulationNode[] = []; @@ -188,19 +186,12 @@ export class D3SimulatorEngine extends Emitter { this._initialSettings = Object.assign(copyObject(DEFAULT_SETTINGS), settings); } - /* - // TODO(dlozic): Question: TLastre, settings! are initialized in here, so they are guaranteed - // to be defined, hence the (!). This isn't very readable though, but if not like this there would be - // code duplication. - this.reset(); - */ - // TODO(dlozic): Question2 (TLastre) I reverted this to the following and removed the (!) from settings: - this.settings = this.resetSettings(); + this._settings = this.resetSettings(); this.clearData(); } getSettings(): ID3SimulatorEngineSettings { - return copyObject(this.settings); + return copyObject(this._settings); } /** @@ -213,22 +204,21 @@ export class D3SimulatorEngine extends Emitter { this._initialSettings = Object.assign(copyObject(DEFAULT_SETTINGS), settings); } - // TODO(dlozic): Question: (from line 166, 167) is this then necessary? Why not simple assign? const previousSettings = this.getSettings(); - Object.assign(this.settings, settings); + Object.assign(this._settings, settings); - if (isObjectEqual(this.settings, previousSettings)) { + if (isObjectEqual(this._settings, previousSettings)) { return; } this._applySettingsToSimulation(settings); - this.emit(D3SimulatorEngineEventType.SETTINGS_UPDATE, { settings: this.settings }); + this.emit(D3SimulatorEngineEventType.SETTINGS_UPDATE, { settings: this._settings }); const hasPhysicsBeenDisabled = previousSettings.isPhysicsEnabled && !settings.isPhysicsEnabled; if (hasPhysicsBeenDisabled) { - this.simulation.stop(); - } else if (this.settings.isSimulatingOnSettingsUpdate) { + this._simulation.stop(); + } else if (this._settings.isSimulatingOnSettingsUpdate) { // this.runSimulation({ isUpdatingSettings: true }); this.activateSimulation(); } @@ -247,7 +237,7 @@ export class D3SimulatorEngine extends Emitter { startDragNode() { this._isDragging = true; - if (!this._isStabilizing && this.settings.isPhysicsEnabled) { + if (!this._isStabilizing && this._settings.isPhysicsEnabled) { this.activateSimulation(); } } @@ -265,7 +255,7 @@ export class D3SimulatorEngine extends Emitter { node.fx = data.x; node.fy = data.y; - if (!this.settings.isPhysicsEnabled) { + if (!this._settings.isPhysicsEnabled) { node.x = data.x; node.y = data.y; } @@ -277,12 +267,12 @@ export class D3SimulatorEngine extends Emitter { endDragNode(data: ID3SimulatorNodeId) { this._isDragging = false; - if (this.settings.isPhysicsEnabled) { - this.simulation.alphaTarget(0); + if (this._settings.isPhysicsEnabled) { + this._simulation.alphaTarget(0); } const node = this._nodes[this._nodeIndexByNodeId[data.id]]; // TODO(dlozic): Add special behavior for sticky nodes that have been dragged - if (node && this.settings.isPhysicsEnabled) { + if (node && this._settings.isPhysicsEnabled) { this.unfixNode(node); } } @@ -293,7 +283,7 @@ export class D3SimulatorEngine extends Emitter { */ activateSimulation() { this.unfixNodes(); // If physics is disabled, the nodes get fixed in the callback from the initial setup (`simulation.on('end', () => {})`). - this.simulation.alpha(this.settings.alpha.alpha).alphaTarget(this.settings.alpha.alphaTarget).restart(); + this._simulation.alpha(this._settings.alpha.alpha).alphaTarget(this._settings.alpha.alphaTarget).restart(); } setupData(data: ISimulationGraph) { @@ -301,7 +291,7 @@ export class D3SimulatorEngine extends Emitter { this._initializeNewData(data); - if (this.settings.isSimulatingOnDataUpdate) { + if (this._settings.isSimulatingOnDataUpdate) { this._updateSimulationData(); this._runSimulation(); } @@ -310,7 +300,7 @@ export class D3SimulatorEngine extends Emitter { mergeData(data: Partial) { this._initializeNewData(data); - if (this.settings.isSimulatingOnDataUpdate) { + if (this._settings.isSimulatingOnDataUpdate) { this._updateSimulationData(); this.activateSimulation(); } @@ -318,7 +308,7 @@ export class D3SimulatorEngine extends Emitter { private _initializeNewData(data: Partial) { if (data.nodes) { - data.nodes = this._fixDefinedNodes(data.nodes); + data.nodes = this._fixAndStickDefinedNodes(data.nodes); for (let i = 0; i < data.nodes.length; i += 1) { if (this._nodeIndexByNodeId[data.nodes[i].id]) { this._nodeIndexByNodeId = { @@ -341,7 +331,7 @@ export class D3SimulatorEngine extends Emitter { } updateData(data: ISimulationGraph) { - data.nodes = this._fixDefinedNodes(data.nodes); + data.nodes = this._fixAndStickDefinedNodes(data.nodes); // Keep existing nodes along with their (x, y, fx, fy) coordinates to avoid // rearranging the graph layout. @@ -361,7 +351,7 @@ export class D3SimulatorEngine extends Emitter { // and Memgraph's `id` property which affects the source->target mapping. this._edges = data.edges; - if (this.settings.isSimulatingOnSettingsUpdate) { + if (this._settings.isSimulatingOnSettingsUpdate) { this._updateSimulationData(); this.activateSimulation(); } @@ -385,7 +375,6 @@ export class D3SimulatorEngine extends Emitter { * Removes all internal and D3 simulation node and relationship data. */ clearData() { - // TODO(dlozic): Is it okay for this to also reset the simulation? Is the naming right? const nodes = this._nodes; const edges = this._edges; this._nodes = []; @@ -400,8 +389,8 @@ export class D3SimulatorEngine extends Emitter { */ private _updateSimulationData() { // Update simulation with new data. - this.simulation.nodes(this._nodes); - this.linkForce.links(this._edges); + this._simulation.nodes(this._nodes); + this._linkForce.links(this._edges); } // Restart vs start? re-heat is restart, start resumes stopped? can it perform both functions? @@ -420,7 +409,7 @@ export class D3SimulatorEngine extends Emitter { */ stopSimulation() { // Consider `pauseSimulation()` - this.simulation.stop(); + this._simulation.stop(); this._nodes = []; this._edges = []; this._setNodeIndexByNodeId(); @@ -431,24 +420,25 @@ export class D3SimulatorEngine extends Emitter { * Resets the simulator engine by discarding all existing simulator data (nodes and edges), * and keeping the current simulator engine settings. */ + // TODO(Alex): Listeners memory leak (D3 force research) resetSimulation() { - this.linkForce = forceLink>(this._edges).id( + this._linkForce = forceLink>(this._edges).id( (node) => node.id, ); - this.simulation = forceSimulation(this._nodes).force('link', this.linkForce).stop(); + this._simulation = forceSimulation(this._nodes).force('link', this._linkForce).stop(); - this._applySettingsToSimulation(this.settings); + this._applySettingsToSimulation(this._settings); - this.simulation.on('tick', () => { + this._simulation.on('tick', () => { this.emit(D3SimulatorEngineEventType.SIMULATION_TICK, { nodes: this._nodes, edges: this._edges }); }); - this.simulation.on('end', () => { + this._simulation.on('end', () => { this._isDragging = false; this._isStabilizing = false; this.emit(D3SimulatorEngineEventType.SIMULATION_END, { nodes: this._nodes, edges: this._edges }); - if (!this.settings.isPhysicsEnabled) { + if (!this._settings.isPhysicsEnabled) { this.fixNodes(); } }); @@ -551,7 +541,7 @@ export class D3SimulatorEngine extends Emitter { this.unstickNode(this._nodes[i]); } - if (this.settings.isSimulatingOnUnstick) { + if (this._settings.isSimulatingOnUnstick) { this.activateSimulation(); } } @@ -579,7 +569,7 @@ export class D3SimulatorEngine extends Emitter { node.sx = null; node.sy = null; - if (this.settings.isPhysicsEnabled) { + if (this._settings.isPhysicsEnabled) { node.fx = null; node.fy = null; } @@ -594,9 +584,7 @@ export class D3SimulatorEngine extends Emitter { * @param {ISimulationNode[]} nodes Graph nodes. * @return {ISimulationNodes[]} Graph nodes with attached `{fx, sx}`, and/or `{fy, sy}` coordinates. */ - // _fixAndStick? - private _fixDefinedNodes(nodes: ISimulationNode[]): ISimulationNode[] { - // TODO(dlozic): Question: should this function be extracted or should i use `this.data` everywhere and remove inputs/outputs? + private _fixAndStickDefinedNodes(nodes: ISimulationNode[]): ISimulationNode[] { for (let i = 0; i < nodes.length; i++) { if (nodes[i].x !== null && nodes[i].x !== undefined) { nodes[i].fx = nodes[i].x; @@ -617,24 +605,24 @@ export class D3SimulatorEngine extends Emitter { */ private _applySettingsToSimulation(settings: ID3SimulatorEngineSettingsUpdate) { if (settings.alpha) { - this.simulation + this._simulation .alpha(settings.alpha.alpha) .alphaMin(settings.alpha.alphaMin) .alphaDecay(settings.alpha.alphaDecay) .alphaTarget(settings.alpha.alphaTarget); } if (settings.links) { - this.linkForce.distance(settings.links.distance).iterations(settings.links.iterations); + this._linkForce.distance(settings.links.distance).iterations(settings.links.iterations); } if (settings.collision) { const collision = forceCollide() .radius(settings.collision.radius) .strength(settings.collision.strength) .iterations(settings.collision.iterations); - this.simulation.force('collide', collision); + this._simulation.force('collide', collision); } if (settings.collision === null) { - this.simulation.force('collide', null); + this._simulation.force('collide', null); } if (settings.manyBody) { const manyBody = forceManyBody() @@ -642,31 +630,31 @@ export class D3SimulatorEngine extends Emitter { .theta(settings.manyBody.theta) .distanceMin(settings.manyBody.distanceMin) .distanceMax(settings.manyBody.distanceMax); - this.simulation.force('charge', manyBody); + this._simulation.force('charge', manyBody); } if (settings.manyBody === null) { - this.simulation.force('charge', null); + this._simulation.force('charge', null); } if (settings.positioning?.forceY) { const positioningForceX = forceX(settings.positioning.forceX.x).strength(settings.positioning.forceX.strength); - this.simulation.force('x', positioningForceX); + this._simulation.force('x', positioningForceX); } if (settings.positioning?.forceX === null) { - this.simulation.force('x', null); + this._simulation.force('x', null); } if (settings.positioning?.forceY) { const positioningForceY = forceY(settings.positioning.forceY.y).strength(settings.positioning.forceY.strength); - this.simulation.force('y', positioningForceY); + this._simulation.force('y', positioningForceY); } if (settings.positioning?.forceY === null) { - this.simulation.force('y', null); + this._simulation.force('y', null); } if (settings.centering) { const centering = forceCenter(settings.centering.x, settings.centering.y).strength(settings.centering.strength); - this.simulation.force('center', centering); + this._simulation.force('center', centering); } if (settings.centering === null) { - this.simulation.force('center', null); + this._simulation.force('center', null); } } @@ -676,17 +664,17 @@ export class D3SimulatorEngine extends Emitter { if (this._isStabilizing) { return; } - if (this.settings.isPhysicsEnabled || options?.isUpdatingSettings) { + if (this._settings.isPhysicsEnabled || options?.isUpdatingSettings) { this.unfixNodes(); } this.emit(D3SimulatorEngineEventType.SIMULATION_START, undefined); this._isStabilizing = true; - this.simulation.alpha(this.settings.alpha.alpha).alphaTarget(this.settings.alpha.alphaTarget).stop(); + this._simulation.alpha(this._settings.alpha.alpha).alphaTarget(this._settings.alpha.alphaTarget).stop(); const totalSimulationSteps = Math.ceil( - Math.log(this.settings.alpha.alphaMin) / Math.log(1 - this.settings.alpha.alphaDecay), + Math.log(this._settings.alpha.alphaMin) / Math.log(1 - this._settings.alpha.alphaDecay), ); let lastProgress = -1; @@ -701,10 +689,10 @@ export class D3SimulatorEngine extends Emitter { progress: currentProgress / 100, }); } - this.simulation.tick(); + this._simulation.tick(); } - if (!this.settings.isPhysicsEnabled) { + if (!this._settings.isPhysicsEnabled) { this.fixNodes(); } diff --git a/src/simulator/types/main-thread-simulator.ts b/src/simulator/types/main-thread-simulator.ts index 3e000f9..bb9c451 100644 --- a/src/simulator/types/main-thread-simulator.ts +++ b/src/simulator/types/main-thread-simulator.ts @@ -15,49 +15,49 @@ import { } from '../engine/d3-simulator-engine'; export class MainThreadSimulator extends Emitter implements ISimulator { - protected readonly simulator: D3SimulatorEngine; + protected readonly _simulator: D3SimulatorEngine; constructor() { super(); - this.simulator = new D3SimulatorEngine(); - this.simulator.on(D3SimulatorEngineEventType.SIMULATION_START, () => { + this._simulator = new D3SimulatorEngine(); + this._simulator.on(D3SimulatorEngineEventType.SIMULATION_START, () => { this.emit(SimulatorEventType.SIMULATION_START, undefined); }); - this.simulator.on(D3SimulatorEngineEventType.SIMULATION_PROGRESS, (data) => { + this._simulator.on(D3SimulatorEngineEventType.SIMULATION_PROGRESS, (data) => { this.emit(SimulatorEventType.SIMULATION_PROGRESS, data); }); - this.simulator.on(D3SimulatorEngineEventType.SIMULATION_END, (data) => { + this._simulator.on(D3SimulatorEngineEventType.SIMULATION_END, (data) => { this.emit(SimulatorEventType.SIMULATION_END, data); }); - this.simulator.on(D3SimulatorEngineEventType.NODE_DRAG, (data) => { + this._simulator.on(D3SimulatorEngineEventType.NODE_DRAG, (data) => { this.emit(SimulatorEventType.NODE_DRAG, data); }); - this.simulator.on(D3SimulatorEngineEventType.SIMULATION_TICK, (data) => { + this._simulator.on(D3SimulatorEngineEventType.SIMULATION_TICK, (data) => { this.emit(SimulatorEventType.SIMULATION_STEP, data); }); - this.simulator.on(D3SimulatorEngineEventType.SETTINGS_UPDATE, (data) => { + this._simulator.on(D3SimulatorEngineEventType.SETTINGS_UPDATE, (data) => { this.emit(SimulatorEventType.SETTINGS_UPDATE, data); }); } setupData(data: ISimulationGraph) { - this.simulator.setupData(data); + this._simulator.setupData(data); } mergeData(data: ISimulationGraph) { - this.simulator.mergeData(data); + this._simulator.mergeData(data); } updateData(data: ISimulationGraph) { - this.simulator.updateData(data); + this._simulator.updateData(data); } deleteData(data: ISimulationIds) { - this.simulator.deleteData(data); + this._simulator.deleteData(data); } clearData() { - this.simulator.clearData(); + this._simulator.clearData(); } simulate() { @@ -66,43 +66,43 @@ export class MainThreadSimulator extends Emitter implements ISi } activateSimulation() { - this.simulator.activateSimulation(); + this._simulator.activateSimulation(); } startSimulation() { - this.simulator.startSimulation(); + this._simulator.startSimulation(); } stopSimulation() { - this.simulator.stopSimulation(); + this._simulator.stopSimulation(); } startDragNode() { - this.simulator.startDragNode(); + this._simulator.startDragNode(); } dragNode(nodeId: number, position: IPosition) { - this.simulator.dragNode({ id: nodeId, ...position }); + this._simulator.dragNode({ id: nodeId, ...position }); } endDragNode(nodeId: number) { - this.simulator.endDragNode({ id: nodeId }); + this._simulator.endDragNode({ id: nodeId }); } fixNodes(nodes: ISimulationNode[]) { - this.simulator.stickNodes(nodes); + this._simulator.stickNodes(nodes); } releaseNodes(nodes?: ISimulationNode[] | undefined): void { - this.simulator.unstickNodes(nodes); + this._simulator.unstickNodes(nodes); } setSettings(settings: ID3SimulatorEngineSettingsUpdate) { - this.simulator.setSettings(settings); + this._simulator.setSettings(settings); } terminate() { - this.simulator.removeAllListeners(); + this._simulator.removeAllListeners(); this.removeAllListeners(); } } diff --git a/src/simulator/types/web-worker-simulator/web-worker-simulator.ts b/src/simulator/types/web-worker-simulator/web-worker-simulator.ts index 1049466..fbe073c 100644 --- a/src/simulator/types/web-worker-simulator/web-worker-simulator.ts +++ b/src/simulator/types/web-worker-simulator/web-worker-simulator.ts @@ -14,11 +14,11 @@ import { IWorkerOutputPayload, WorkerOutputType } from './message/worker-output' import { Emitter } from '../../../utils/emitter.utils'; export class WebWorkerSimulator extends Emitter implements ISimulator { - protected readonly worker: Worker; + protected readonly _worker: Worker; constructor() { super(); - this.worker = new Worker( + this._worker = new Worker( new URL( /* webpackChunkName: 'simulator.worker' */ './simulator.worker', @@ -27,7 +27,7 @@ export class WebWorkerSimulator extends Emitter implements ISim { type: 'module' }, ); - this.worker.onmessage = ({ data }: MessageEvent) => { + this._worker.onmessage = ({ data }: MessageEvent) => { switch (data.type) { case WorkerOutputType.SIMULATION_START: { this.emit(SimulatorEventType.SIMULATION_START, undefined); @@ -137,11 +137,11 @@ export class WebWorkerSimulator extends Emitter implements ISim } terminate() { - this.worker.terminate(); + this._worker.terminate(); this.removeAllListeners(); } protected emitToWorker(message: IWorkerInputPayload) { - this.worker.postMessage(message); + this._worker.postMessage(message); } }