From de9b3eae06539c0a6587dde2e03c78df103081f4 Mon Sep 17 00:00:00 2001 From: ispyisail Date: Wed, 23 Sep 2026 16:37:45 +1200 Subject: [PATCH] Preserve master/slave links when pasting or duplicating a folio Reviving #659, closed 2026-09-10 purely to clear a review backlog (#630), not on merit. Rebuilt fresh against current master rather than merged from the old branch (elementspanelwidget.cpp had drifted enough that a textual merge risked silently losing content, as it did earlier in this same session for a different revival). Builds discussion #607. Cutting/copying a linked group of elements -- a relay coil with its contacts, a PLC master with its slave I/O elements -- dropped the master/slave link entirely. Traced end to end: Element::toXml() writes each partner's uuid into , Element::fromXml() reads it back into a deferred, unresolved buffer (tmp_uuids_link), and the only code that ever resolves that buffer is initLink(QETProject *) -- called only from Diagram::refreshContents(), itself only called from full project load and macro-block insertion. Neither DiagramView::paste() nor ElementsPanelWidget::duplicateDiagram() ever call it, so tmp_uuids_link is populated correctly and never resolved: the link is silently dropped. duplicateDiagram() already knew this and worked around it by calling clearPendingLinks() -- correct to not link back to a stale source, but it meant folio duplication never preserved a link either. Added Element::initLink(const QList &candidates) -- resolves against a caller-supplied list instead of a project-wide search. The scoping is the subtle part: right after the XML round-trip and before uuids are renewed, a pasted/duplicated element's tmp_uuids_link still holds its source's original partner uuid, which at that exact moment still equals the not-yet-renewed uuid of that partner's own copy, if it was carried along in the same batch. Resolving only within the batch is what stops a linked pair pasted together from matching an original element left elsewhere that happens to still carry that same soon-to-be-replaced uuid. If only one half of a linked group is in the batch, its entry finds no match and is dropped -- the same "leave it unlinked" outcome as before. Wired into PasteDiagramCommand::redo(), before the existing newUuid() loop and gated by the same first_redo flag. Wired into duplicateDiagram() the same way, replacing its clearPendingLinks() call (initLink() clears tmp_uuids_link internally, matched or not). Verified live -- the original PR's own test plan left both of these unchecked, so this closes that gap rather than repeating it. Built a project with a linked PLC master/slave pair (qet-mcp's link_elements), then drove the real interaction under Xvfb: Ctrl+A, Ctrl+C, Ctrl+V: originals 95ad58fc <-> e728632c (unchanged) pasted 513e6bf8 <-> 29aa60b4 (linked to each other) Right-click folio > "Copier et coller": originals 95ad58fc <-> e728632c (unchanged) duplicated 0a33ccb4 <-> 3264fe66 (linked to each other) Neither copy links back to an original or comes in unlinked. Qt 6.10.2, ctest 13/13. Co-Authored-By: Claude Sonnet 5 --- sources/diagramcommands.cpp | 18 +++++++++++++++++- sources/elementspanelwidget.cpp | 27 ++++++++++++++++++++++++--- sources/qetgraphicsitem/element.cpp | 27 +++++++++++++++++++++++++++ sources/qetgraphicsitem/element.h | 19 +++++++++++++++++++ 4 files changed, 87 insertions(+), 4 deletions(-) diff --git a/sources/diagramcommands.cpp b/sources/diagramcommands.cpp index 8c19636f6..fa21ac43c 100644 --- a/sources/diagramcommands.cpp +++ b/sources/diagramcommands.cpp @@ -77,6 +77,23 @@ void PasteDiagramCommand::redo() { first_redo = false; + //Resolve a linked master/slave pair pasted together (bugtracker + //#607) before anything below renews their uuids: at this exact + //moment a pasted element's tmp_uuids_link still holds its + //source's original partner uuid, which still equals the + //not-yet-renewed uuid of that partner's own pasted copy if it + //was carried along in the same batch. Scoped to this batch only + //(not a project-wide search), so a pair pasted together links to + //each other and not to an original element left elsewhere that + //happens to still carry that same soon-to-be-replaced uuid. If + //only one half of a linked group was pasted, its link entry + //simply finds no match here and is dropped -- same "leave it + //unlinked" outcome as always. + const QList elmts_list = content.m_elements; + for (Element *e : elmts_list) { + e->initLink(elmts_list); + } + //make new uuid for every pasted conductor, because old uuid are //the uuid of the copied conductor const QList all_pasted_conductors = content.conductors(); @@ -85,7 +102,6 @@ void PasteDiagramCommand::redo() } //this is the first paste, we do some actions for the new element - const QList elmts_list = content.m_elements; for (Element *e : elmts_list) { //make new uuid, because old uuid are the uuid of the copied element diff --git a/sources/elementspanelwidget.cpp b/sources/elementspanelwidget.cpp index 4aa61b7b1..ca4178f1d 100644 --- a/sources/elementspanelwidget.cpp +++ b/sources/elementspanelwidget.cpp @@ -674,6 +674,27 @@ void ElementsPanelWidget::duplicateDiagram() bool erase_labels = settings.value( "diagramcommands/erase-label-on-copy", true).toBool(); + // Resolve a linked pair duplicated together against each other + // (bugtracker #607) before the loop below renews their uuids or + // clears their pending links: at this exact moment a copy's + // tmp_uuids_link still holds its source's original partner + // uuid, which still equals the not-yet-renewed uuid of that + // partner's own copy if both were duplicated together. Scoped + // to this diagram's own copies, not a project-wide search, so + // this never links back to the source elements the copies were + // made from -- if only one half of a linked pair is here, its + // link entry simply finds no match and is dropped, same as + // clearPendingLinks() used to do unconditionally for every copy. + QList new_elements; + for (QGraphicsItem *item : new_diagram->items()) { + if (Element *elmt = dynamic_cast(item)) { + new_elements << elmt; + } + } + for (Element *elmt : new_elements) { + elmt->initLink(new_elements); + } + for (QGraphicsItem *item : new_diagram->items()) { if (Element *elmt = dynamic_cast(item)) { // The XML round-trip kept the source elements' uuids. Give the @@ -695,9 +716,9 @@ void ElementsPanelWidget::duplicateDiagram() new_diagram->restoreText(elmt); } - // Clear pending links so copies don't link back to - // the source elements via stale UUIDs. - elmt->clearPendingLinks(); + // initLink() above already cleared tmp_uuids_link for + // every copy, matched or not -- nothing left here that + // could link back to a stale source uuid. // Clean up copied element data: // 1. Slaves always lose label/formula/comment/location diff --git a/sources/qetgraphicsitem/element.cpp b/sources/qetgraphicsitem/element.cpp index a42f5ee64..73e7d8237 100644 --- a/sources/qetgraphicsitem/element.cpp +++ b/sources/qetgraphicsitem/element.cpp @@ -1369,6 +1369,33 @@ void Element::initLink(QETProject *prj) tmp_uuids_link.clear(); } +/** + @brief Element::initLink + Overload resolving tmp_uuids_link against @p candidates instead of a + project-wide ElementProvider search -- see the header comment for + why the search has to be scoped this way right after a paste or + folio-duplication XML round-trip, before uuids are renewed. + @param candidates the elements to search for a link partner in +*/ +void Element::initLink(const QList &candidates) +{ + // if nothing to link return now + if (tmp_uuids_link.isEmpty()) return; + + for (int i = 0; i < tmp_uuids_link.size(); ++i) { + for (Element *elmt : candidates) { + if (elmt->uuid() == tmp_uuids_link[i].uuid) { + elmt->linkToElement(this); + if (tmp_uuids_link[i].group_index >= 0) { + m_group_index_map[elmt] = tmp_uuids_link[i].group_index; + } + break; + } + } + } + tmp_uuids_link.clear(); +} + /** * @brief Element::linkTypeToString * \deprecated use instead ElementData::typeToString diff --git a/sources/qetgraphicsitem/element.h b/sources/qetgraphicsitem/element.h index 87344c914..9bb02202d 100644 --- a/sources/qetgraphicsitem/element.h +++ b/sources/qetgraphicsitem/element.h @@ -207,6 +207,25 @@ class Element : public QetGraphicsItem virtual void unlinkAllElements() {} virtual void unlinkElement(Element *) {} virtual void initLink(QETProject *); + /** + Resolve tmp_uuids_link against a caller-supplied candidate + list instead of a project-wide search (bugtracker #607). + Used right after an XML round-trip (paste, folio + duplication) and before the pasted/duplicated elements' + uuids are renewed: at that moment a copy's tmp_uuids_link + still holds its source's original partner uuid, which + still matches the not-yet-renewed uuid of that partner's + own copy if it was carried along in the same batch. + Resolving only within @p candidates -- not the whole + project -- is what stops a linked pair pasted together + from matching an original element left elsewhere that + happens to still carry that same soon-to-be-replaced + uuid. If only one half of a linked group is in + @p candidates, its entry finds no match and is dropped, + same as initLink(QETProject *) leaving an unresolvable + link unlinked. + */ + void initLink(const QList &candidates); QList linkedElements (); int groupIndexForElement(Element *elmt) const;