From 15a102119ad769f1271ca8f2d4eb7dc0d8bc2042 Mon Sep 17 00:00:00 2001 From: Roman Rodionov Date: Tue, 10 Dec 2024 12:55:20 +0100 Subject: [PATCH] #5949 - Delete of micromolecules bonds works wrong (or doesn't work) (#6110) - added invertAfterAllOperations method to atom and bonds operations to allow renderers rely on final state of model before rendering - added deleting of atoms and bonds from molecules struct to synchronize molecules and macromolecules modes --- .../editor/MacromoleculesConverter.ts | 3 +- .../editor/operations/coreAtom/atom.ts | 94 ++++++++++++++++++- .../editor/operations/coreBond/bond.ts | 45 ++++++++- .../render/renderers/RenderersManager.ts | 8 +- .../src/domain/entities/Command.ts | 20 +++- .../src/domain/entities/CoreBond.ts | 1 + .../domain/entities/DrawingEntitiesManager.ts | 74 +++++++++++---- .../src/domain/entities/Operation.ts | 1 + 8 files changed, 215 insertions(+), 31 deletions(-) diff --git a/packages/ketcher-core/src/application/editor/MacromoleculesConverter.ts b/packages/ketcher-core/src/application/editor/MacromoleculesConverter.ts index d82bbd4ac2..164a3113a1 100644 --- a/packages/ketcher-core/src/application/editor/MacromoleculesConverter.ts +++ b/packages/ketcher-core/src/application/editor/MacromoleculesConverter.ts @@ -477,7 +477,7 @@ export class MacromoleculesConverter { ); }); - monomer.monomerItem.struct.bonds.forEach((bond) => { + monomer.monomerItem.struct.bonds.forEach((bond, bondId) => { const firstAtom = atomsMap.get(bond.begin); const secondAtom = atomsMap.get(bond.end); @@ -491,6 +491,7 @@ export class MacromoleculesConverter { secondAtom, bond.type, bond.stereo, + bondId, ), ); }); diff --git a/packages/ketcher-core/src/application/editor/operations/coreAtom/atom.ts b/packages/ketcher-core/src/application/editor/operations/coreAtom/atom.ts index 325e588e3e..c44e1a77bb 100644 --- a/packages/ketcher-core/src/application/editor/operations/coreAtom/atom.ts +++ b/packages/ketcher-core/src/application/editor/operations/coreAtom/atom.ts @@ -19,12 +19,75 @@ import { RenderersManager } from 'application/render/renderers/RenderersManager'; import { Operation } from 'domain/entities/Operation'; import { Atom } from 'domain/entities/CoreAtom'; +import { + Bond as MicromoleculesBond, + Atom as MicromoleculesAtom, +} from 'domain/entities'; +import { KetcherLogger } from 'utilities'; +interface BondWithIdInMicromolecules { + bondId: number; + bond: MicromoleculesBond; +} + +interface DeletedMoleculeStructItems { + atomInMoleculeStruct: MicromoleculesAtom; + bondsInMoleculeStruct: BondWithIdInMicromolecules[]; +} + +function addAtomToMoleculeStruct( + atom: Atom, + atomInMoleculeStruct: MicromoleculesAtom, + bondsInMoleculeStruct: BondWithIdInMicromolecules[] = [], +) { + const moleculeStruct = atom.monomer.monomerItem.struct; + + moleculeStruct.atoms.set(atom.atomIdInMicroMode, atomInMoleculeStruct); + + bondsInMoleculeStruct.forEach(({ bondId, bond }) => { + moleculeStruct.bonds.set(bondId, bond); + }); +} + +function deleteAtomFromMoleculeStruct(atom: Atom) { + const moleculeStruct = atom.monomer.monomerItem.struct; + const atomInMoleculeStruct = moleculeStruct.atoms.get(atom.atomIdInMicroMode); + + if (!atomInMoleculeStruct) { + KetcherLogger.warn('Atom is not found in molecule struct during deletion'); + + return; + } + + const bondsInMoleculeStruct = moleculeStruct.bonds.filter((_, bond) => { + return ( + bond.begin === atom.atomIdInMicroMode || + bond.end === atom.atomIdInMicroMode + ); + }); + + moleculeStruct.atoms.delete(atom.atomIdInMicroMode); + + bondsInMoleculeStruct.forEach((_, bondId) => { + moleculeStruct.bonds.delete(bondId); + }); + + return { + atomInMoleculeStruct, + bondsInMoleculeStruct: [...bondsInMoleculeStruct.entries()].map( + ([bondId, bond]) => { + return { bondId, bond }; + }, + ), + }; +} export class AtomAddOperation implements Operation { public atom: Atom; + private deletedMoleculeStructItems?: DeletedMoleculeStructItems; + constructor( public addAtomChangeModel: (atom?: Atom) => Atom, - public deleteAtomChangeModel: () => void, + public deleteAtomChangeModel: (atom: Atom) => void, ) { this.atom = this.addAtomChangeModel(); } @@ -32,17 +95,29 @@ export class AtomAddOperation implements Operation { public execute(renderersManager: RenderersManager) { this.atom = this.addAtomChangeModel(this.atom); renderersManager.addAtom(this.atom); + + if (this.deletedMoleculeStructItems) { + addAtomToMoleculeStruct( + this.atom, + this.deletedMoleculeStructItems.atomInMoleculeStruct, + this.deletedMoleculeStructItems.bondsInMoleculeStruct, + ); + } } public invert(renderersManager: RenderersManager) { if (this.atom) { - this.deleteAtomChangeModel(); + this.deleteAtomChangeModel(this.atom); renderersManager.deleteAtom(this.atom); } + + this.deletedMoleculeStructItems = deleteAtomFromMoleculeStruct(this.atom); } } export class AtomDeleteOperation implements Operation { + private deletedMoleculeStructItems?: DeletedMoleculeStructItems; + constructor( public atom: Atom, public deleteAtomChangeModel: () => void, @@ -52,10 +127,23 @@ export class AtomDeleteOperation implements Operation { public execute(renderersManager: RenderersManager) { this.deleteAtomChangeModel(); renderersManager.deleteAtom(this.atom); + + this.deletedMoleculeStructItems = deleteAtomFromMoleculeStruct(this.atom); } - public invert(renderersManager: RenderersManager) { + public invert() { this.addAtomChangeModel(this.atom); + + if (this.deletedMoleculeStructItems) { + addAtomToMoleculeStruct( + this.atom, + this.deletedMoleculeStructItems.atomInMoleculeStruct, + this.deletedMoleculeStructItems.bondsInMoleculeStruct, + ); + } + } + + public invertAfterAllOperations(renderersManager: RenderersManager) { renderersManager.addAtom(this.atom); } } diff --git a/packages/ketcher-core/src/application/editor/operations/coreBond/bond.ts b/packages/ketcher-core/src/application/editor/operations/coreBond/bond.ts index 7d9d7eafa4..e931545217 100644 --- a/packages/ketcher-core/src/application/editor/operations/coreBond/bond.ts +++ b/packages/ketcher-core/src/application/editor/operations/coreBond/bond.ts @@ -19,12 +19,32 @@ import { RenderersManager } from 'application/render/renderers/RenderersManager'; import { Operation } from 'domain/entities/Operation'; import { Bond } from 'domain/entities/CoreBond'; +import { Bond as MicromoleculesBond } from 'domain/entities/bond'; + +function addBondToMoleculeStruct( + bond: Bond, + bondInMoleculeStruct: MicromoleculesBond, +) { + const moleculeStruct = bond.firstAtom.monomer.monomerItem.struct; + + moleculeStruct.bonds.set(bond.bondIdInMicroMode, bondInMoleculeStruct); +} + +function deleteBondFromMoleculeStruct(bond: Bond) { + const moleculeStruct = bond.firstAtom.monomer.monomerItem.struct; + const bondInMoleculeStruct = moleculeStruct.bonds.get(bond.bondIdInMicroMode); + + moleculeStruct.bonds.delete(bond.bondIdInMicroMode); + + return bondInMoleculeStruct; +} export class BondAddOperation implements Operation { public bond: Bond; + private bondInMoleculeStruct?: MicromoleculesBond; constructor( public addBondChangeModel: (bond?: Bond) => Bond, - public deleteBondChangeModel: (bond?: Bond) => void, + public deleteBondChangeModel: (bond: Bond) => void, ) { this.bond = this.addBondChangeModel(); } @@ -32,6 +52,10 @@ export class BondAddOperation implements Operation { public execute(renderersManager: RenderersManager) { this.bond = this.addBondChangeModel(this.bond); renderersManager.addBond(this.bond); + + if (this.bondInMoleculeStruct) { + addBondToMoleculeStruct(this.bond, this.bondInMoleculeStruct); + } } public invert(renderersManager: RenderersManager) { @@ -39,23 +63,36 @@ export class BondAddOperation implements Operation { this.deleteBondChangeModel(this.bond); renderersManager.deleteBond(this.bond); } + + this.bondInMoleculeStruct = deleteBondFromMoleculeStruct(this.bond); } } export class BondDeleteOperation implements Operation { + private bondInMoleculeStruct?: MicromoleculesBond; + constructor( public bond: Bond, - public deleteBondChangeModel: (bond?: Bond) => void, - public addBondChangeModel: (bond?: Bond) => Bond, + public deleteBondChangeModel: (bond: Bond) => void, + public addBondChangeModel: (bond: Bond) => Bond, ) {} public execute(renderersManager: RenderersManager) { this.deleteBondChangeModel(this.bond); renderersManager.deleteBond(this.bond); + + this.bondInMoleculeStruct = deleteBondFromMoleculeStruct(this.bond); } - public invert(renderersManager: RenderersManager) { + public invert() { this.addBondChangeModel(this.bond); + + if (this.bondInMoleculeStruct) { + addBondToMoleculeStruct(this.bond, this.bondInMoleculeStruct); + } + } + + public invertAfterAllOperations(renderersManager: RenderersManager) { renderersManager.addBond(this.bond); } } diff --git a/packages/ketcher-core/src/application/render/renderers/RenderersManager.ts b/packages/ketcher-core/src/application/render/renderers/RenderersManager.ts index 57598844bb..bd6666ad34 100644 --- a/packages/ketcher-core/src/application/render/renderers/RenderersManager.ts +++ b/packages/ketcher-core/src/application/render/renderers/RenderersManager.ts @@ -305,12 +305,16 @@ export class RenderersManager { monomer.renderer?.updateAttachmentPoints(); } - public update(modelChanges?: Command) { + public reinitializeViewModel() { const editor = CoreEditor.provideEditorInstance(); const viewModel = editor.viewModel; - modelChanges?.execute(this); viewModel.initialize([...editor.drawingEntitiesManager.bonds.values()]); + } + + public update(modelChanges?: Command) { + this.reinitializeViewModel(); + modelChanges?.execute(this); modelChanges?.executeAfterAllOperations(this); this.runPostRenderMethods(); diff --git a/packages/ketcher-core/src/domain/entities/Command.ts b/packages/ketcher-core/src/domain/entities/Command.ts index 4aff4a7b0e..2ec4d70fd6 100644 --- a/packages/ketcher-core/src/domain/entities/Command.ts +++ b/packages/ketcher-core/src/domain/entities/Command.ts @@ -33,6 +33,8 @@ export class Command { } operations.forEach((operation) => operation.invert(renderersManagers)); + renderersManagers.reinitializeViewModel(); + this.invertAfterAllOperations(renderersManagers, operations); renderersManagers.runPostRenderMethods(); } @@ -43,14 +45,28 @@ export class Command { renderersManagers.runPostRenderMethods(); } - public executeAfterAllOperations(renderersManagers: RenderersManager) { - this.operations.forEach((operation) => { + public executeAfterAllOperations( + renderersManagers: RenderersManager, + operations = this.operations, + ) { + operations.forEach((operation) => { if (operation.executeAfterAllOperations) { operation.executeAfterAllOperations(renderersManagers); } }); } + public invertAfterAllOperations( + renderersManagers: RenderersManager, + operations = this.operations, + ) { + operations.forEach((operation) => { + if (operation.invertAfterAllOperations) { + operation.invertAfterAllOperations(renderersManagers); + } + }); + } + public clear() { this.operations = []; } diff --git a/packages/ketcher-core/src/domain/entities/CoreBond.ts b/packages/ketcher-core/src/domain/entities/CoreBond.ts index f97ce19bb8..5d713a30d0 100644 --- a/packages/ketcher-core/src/domain/entities/CoreBond.ts +++ b/packages/ketcher-core/src/domain/entities/CoreBond.ts @@ -11,6 +11,7 @@ export class Bond extends DrawingEntity { constructor( public firstAtom: Atom, public secondAtom: Atom, + public bondIdInMicroMode, public type = 1, public stereo = 0, ) { diff --git a/packages/ketcher-core/src/domain/entities/DrawingEntitiesManager.ts b/packages/ketcher-core/src/domain/entities/DrawingEntitiesManager.ts index 3fdf0bd7de..3d7a657c11 100644 --- a/packages/ketcher-core/src/domain/entities/DrawingEntitiesManager.ts +++ b/packages/ketcher-core/src/domain/entities/DrawingEntitiesManager.ts @@ -285,7 +285,7 @@ export class DrawingEntitiesManager { } else if (drawingEntity instanceof Bond) { return this.deleteBond(drawingEntity); } else if (drawingEntity instanceof Atom) { - return this.deleteAtom(drawingEntity); + return this.deleteAtom(drawingEntity, needToDeleteConnectedEntities); } else { return new Command(); } @@ -2005,6 +2005,7 @@ export class DrawingEntitiesManager { const bondAddCommand = targetDrawingEntitiesManager.addBond( newFirstAtom, newSecondAtom, + bond.bondIdInMicroMode, bond.type, bond.stereo, ); @@ -2403,13 +2404,7 @@ export class DrawingEntitiesManager { atomIdInMicroMode, label, ), - this.addAtomChangeModel.bind( - this, - position, - monomer, - atomIdInMicroMode, - label, - ), + this.deleteAtomChangeModel.bind(this), ); command.addOperation(atomAddOperation); @@ -2423,7 +2418,7 @@ export class DrawingEntitiesManager { return atom; } - private deleteAtom(atom: Atom) { + private deleteAtom(atom: Atom, needToDeleteConnectedEntities = true) { const command = new Command(); command.addOperation( @@ -2440,6 +2435,16 @@ export class DrawingEntitiesManager { ), ); + if (needToDeleteConnectedEntities) { + atom.bonds.forEach((bond) => { + if (bond.selected) { + return; + } + + command.merge(this.deleteBond(bond)); + }); + } + return command; } @@ -2448,6 +2453,7 @@ export class DrawingEntitiesManager { secondAtom: Atom, type: number, stereo: number, + bondIdInMicroMode: number, _bond?: Bond, ) { if (_bond) { @@ -2456,7 +2462,13 @@ export class DrawingEntitiesManager { return _bond; } - const bond = new Bond(firstAtom, secondAtom, type, stereo); + const bond = new Bond( + firstAtom, + secondAtom, + bondIdInMicroMode, + type, + stereo, + ); this.bonds.set(bond.id, bond); firstAtom.addBond(bond); @@ -2470,11 +2482,20 @@ export class DrawingEntitiesManager { secondAtom: Atom, type: number, stereo: number, + bondIdInMicroMode: number, ) { const command = new Command(); const bondAddOperation = new BondAddOperation( - this.addBondChangeModel.bind(this, firstAtom, secondAtom, type, stereo), - this.addBondChangeModel.bind(this, firstAtom, secondAtom, type, stereo), + (bond?: Bond) => + this.addBondChangeModel( + firstAtom, + secondAtom, + type, + stereo, + bondIdInMicroMode, + bond, + ), + (bond: Bond) => this.deleteBondChangeModel(bond), ); command.addOperation(bondAddOperation); @@ -2495,13 +2516,15 @@ export class DrawingEntitiesManager { new BondDeleteOperation( bond, this.deleteBondChangeModel.bind(this, bond), - this.addBondChangeModel.bind( - this, - bond.firstAtom, - bond.secondAtom, - bond.type, - bond.stereo, - ), + (bond: Bond) => + this.addBondChangeModel( + bond.firstAtom, + bond.secondAtom, + bond.bondIdInMicroMode, + bond.type, + bond.stereo, + bond, + ), ), ); @@ -2926,6 +2949,19 @@ export class DrawingEntitiesManager { return command; } + + public get monomersArray() { + return [...this.monomers.values()]; + } + + public get molecules() { + return this.monomersArray.filter((monomer) => { + return ( + monomer.monomerItem.props.isMicromoleculeFragment && + !isMonomerSgroupWithAttachmentPoints(monomer) + ); + }); + } } function getFirstPosition( height: number, diff --git a/packages/ketcher-core/src/domain/entities/Operation.ts b/packages/ketcher-core/src/domain/entities/Operation.ts index 15d6eb8995..76d3639f92 100644 --- a/packages/ketcher-core/src/domain/entities/Operation.ts +++ b/packages/ketcher-core/src/domain/entities/Operation.ts @@ -15,4 +15,5 @@ export interface Operation { execute(renderersManager: RenderersManager): void; invert(renderersManager: RenderersManager): void; executeAfterAllOperations?(renderersManager: RenderersManager): void; + invertAfterAllOperations?(renderersManager: RenderersManager): void; }