From ee8e7c34501ceea31056632d2537f6af97bec3c9 Mon Sep 17 00:00:00 2001 From: ispyisail Date: Sat, 3 Oct 2026 21:49:06 +1300 Subject: [PATCH 1/3] Fix qet.deleteElement() leaving the element's wires on the folio qet.deleteElement() built its DeleteQGraphicsItemCommand from the element alone. The Delete key's selection also carries the wires on the element's terminals (DiagramContent's conductors to update), and the command removes those with it. From a script they stayed: still listed by qet.conductorUuids(), still saved, attached to an element that was gone. The wires on the element's terminals now go into the command, as for the Delete key. tst_scriptconductoruuid checks that no wire is left with an end on the deleted element (it failed before this change: 1 left). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015FPuYPS4T7QuEwjNu22rXD --- sources/scripting/qetscriptapi.cpp | 10 ++++++++++ tests/qttest/tst_scriptconductoruuid.cpp | 22 ++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/sources/scripting/qetscriptapi.cpp b/sources/scripting/qetscriptapi.cpp index eea431088..966bec45b 100644 --- a/sources/scripting/qetscriptapi.cpp +++ b/sources/scripting/qetscriptapi.cpp @@ -772,6 +772,16 @@ bool QetScriptApi::deleteElement(int folioIndex, const QString &elementUuid) DiagramContent content; content.m_elements << element; + // The wires on its terminals go with it, as when the Delete key + // removes a selected element (DiagramContent puts them in + // m_conductors_to_update); without them they stay on the folio, + // attached to an element that is no longer there. + for (Terminal *terminal : element->terminals()) { + for (Conductor *conductor : terminal->conductors()) { + if (!content.m_conductors_to_update.contains(conductor)) + content.m_conductors_to_update << conductor; + } + } if (DeleteQGraphicsItemCommand::hasNonDeletableTerminal(content)) { log(QStringLiteral("qet.deleteElement: %1 has a non-deletable terminal (linked master/slave?), refusing").arg(elementUuid)); return false; diff --git a/tests/qttest/tst_scriptconductoruuid.cpp b/tests/qttest/tst_scriptconductoruuid.cpp index 27792ab70..763f61f14 100644 --- a/tests/qttest/tst_scriptconductoruuid.cpp +++ b/tests/qttest/tst_scriptconductoruuid.cpp @@ -97,6 +97,28 @@ private slots: QVERIFY(r.value(QStringLiteral("junk")).toArray().isEmpty()); QVERIFY(r.value(QStringLiteral("badFolio")).toArray().isEmpty()); } + + // qet.deleteElement() takes the wires on the element's terminals with + // it, as the Delete key does; they used to stay on the folio, attached + // to an element that was gone. + void deleteElementTakesItsWires() + { + const QJsonObject r = run(QStringLiteral( + "var p = 'embed://import/probe/v2_fuse.elmt';\n" + "var a = qet.addElement(0, p, 400, 400);\n" + "var b = qet.addElement(0, p, 470, 490);\n" + "qet.addConductor(0, a, 0, b, 0);\n" + "var before = qet.conductorUuids(0).length;\n" + "var deleted = qet.deleteElement(0, a);\n" + "var dangling = qet.conductorUuids(0).filter(function (u) {\n" + " return qet.conductorEnds(0, u).some(function (e) { return e.indexOf(a) === 0; }); });\n" + "qet.log('PROBE ' + JSON.stringify({deleted: deleted, before: before,\n" + " after: qet.conductorUuids(0).length, dangling: dangling.length}));\n")); + QVERIFY2(!r.isEmpty(), "the script logged nothing"); + QVERIFY(r.value(QStringLiteral("deleted")).toBool()); + QCOMPARE(r.value(QStringLiteral("dangling")).toInt(), 0); + QCOMPARE(r.value(QStringLiteral("after")).toInt(), r.value(QStringLiteral("before")).toInt() - 1); + } }; QTEST_APPLESS_MAIN(tst_scriptconductoruuid) From 5d27ed81566461a0e8f449779d801212d87d4d63 Mon Sep 17 00:00:00 2001 From: ispyisail Date: Sat, 3 Oct 2026 21:49:06 +1300 Subject: [PATCH 2/3] Wire terminals one after another under a wires-per-terminal limit Discussion #1158, stacked on the limit itself. Two of QElectroTech's own tools wire several terminals as a star, every terminal to one of them: "create wires in a drawn polygon" (also qet.addConductor()), and deleting a symbol, which rewires the far ends to keep the potential. Six terminals give the hub five wires, which the project's limit forbids. When the project sets a limit (and the master switch is on), both now wire the terminals one after another instead, from the top left one to the nearest not yet wired (WiringRules::chainOrder()). The polygon tool also skips a wire whose terminal is already full. Without a limit, both build the star as before. tst_wiringrules: the chain order, and through the real binary that deleting a symbol wired to four others leaves them with at most two wires each under a limit, three on one of them without (checked to fail with chaining turned off). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015FPuYPS4T7QuEwjNu22rXD --- .../deleteqgraphicsitemcommand.cpp | 41 +++++++-- sources/utils/conductorcreator.cpp | 49 +++++++++-- sources/utils/conductorcreator.h | 1 + sources/wiringrules.cpp | 62 +++++++++++++ sources/wiringrules.h | 6 ++ tests/qttest/tst_wiringrules.cpp | 87 +++++++++++++++++++ 6 files changed, 234 insertions(+), 12 deletions(-) diff --git a/sources/undocommand/deleteqgraphicsitemcommand.cpp b/sources/undocommand/deleteqgraphicsitemcommand.cpp index a2db0baf8..beb1248a3 100644 --- a/sources/undocommand/deleteqgraphicsitemcommand.cpp +++ b/sources/undocommand/deleteqgraphicsitemcommand.cpp @@ -16,8 +16,10 @@ along with QElectroTech. If not, see . */ #include "deleteqgraphicsitemcommand.h" +#include "../wiringrules.h" #include "../diagram.h" +#include "../qetproject.h" #include "addgraphicsobjectcommand.h" #include "../qetdiagrameditor.h" #include "../qetgraphicsitem/ViewItem/qetgraphicstableitem.h" @@ -206,17 +208,44 @@ void DeleteQGraphicsItemCommand::setPotentialsOfRemovedElements() } ConductorProperties properties = hub_terminal->conductors().first()->properties(); - for (Terminal *t : terminals_to_connect_list) + + //Every terminal to the hub (a star), or, when the project + //limits the wires per terminal (discussion #1158), one after + //another (a chain): a star gives the hub a wire per other + //terminal, which the limit forbids. + QList> pairs; + const QETProject *project = hub_terminal->diagram() ? hub_terminal->diagram()->project() : nullptr; + if (project && WiringRules::chainsWires(project->wiringRules(), WiringRules::masterEnabled())) + { + QList chain {hub_terminal}; + chain << terminals_to_connect_list; + QList points; + for (Terminal *ct : std::as_const(chain)) { + points << ct->scenePos(); + } + const QList order = WiringRules::chainOrder(points); + for (int i = 1 ; i < order.size() ; ++i) { + pairs.append(std::make_pair(chain.at(order.at(i - 1)), chain.at(order.at(i)))); + } + } + else + { + for (Terminal *t : std::as_const(terminals_to_connect_list)) { + pairs.append(std::make_pair(hub_terminal, t)); + } + } + + for (const auto &new_pair : std::as_const(pairs)) { //If a conductor was already created between these two terminals //in this undo command, from another removed element, we do nothing bool exist_ = false; for (std::pair pair : m_connected_terminals) { - if (pair.first == hub_terminal && pair.second == t) { + if (pair.first == new_pair.first && pair.second == new_pair.second) { exist_ = true; continue; - } else if (pair.first == t && pair.second == hub_terminal) { + } else if (pair.first == new_pair.second && pair.second == new_pair.first) { exist_ = true; continue; } @@ -224,11 +253,11 @@ void DeleteQGraphicsItemCommand::setPotentialsOfRemovedElements() if (exist_ == false) { - m_connected_terminals.append(std::make_pair((Terminal *)hub_terminal, (Terminal *)t)); + m_connected_terminals.append(new_pair); qInfo() << "m_connected_terminals" << m_connected_terminals; - Conductor *new_cond = new Conductor(hub_terminal, t); + Conductor *new_cond = new Conductor(new_pair.first, new_pair.second); new_cond->setProperties(properties); - new AddGraphicsObjectCommand(new_cond, t->diagram(), QPointF(), this); + new AddGraphicsObjectCommand(new_cond, new_pair.second->diagram(), QPointF(), this); } } } diff --git a/sources/utils/conductorcreator.cpp b/sources/utils/conductorcreator.cpp index 77f2009ec..2d2ed3344 100644 --- a/sources/utils/conductorcreator.cpp +++ b/sources/utils/conductorcreator.cpp @@ -26,6 +26,7 @@ #include "../qetgraphicsitem/element.h" #include "../qetgraphicsitem/terminal.h" #include "../ui/potentialselectordialog.h" +#include "../wiringrules.h" #include "qgraphicsitem.h" #include @@ -48,18 +49,19 @@ ConductorCreator::ConductorCreator(Diagram *d, QList terminals_list) if (!setUpPropertieToUse()) { return; } - Terminal *hub_terminal = hubTerminal(); - d->undoStack().beginMacro(QObject::tr("Création de conducteurs")); + const bool chain = d->project() + && WiringRules::chainsWires(d->project()->wiringRules(), WiringRules::masterEnabled()); QList c_list; - for (Terminal *t : m_terminals_list) + for (const auto &pair : terminalPairs(chain)) { - if (t == hub_terminal) { + //Checked as the chain is built: the wire before this one may + //have just filled a terminal. + if (chain && !pair.first->canBeLinkedTo(pair.second)) { continue; } - - Conductor *cond = new Conductor(hub_terminal, t); + Conductor *cond = new Conductor(pair.first, pair.second); cond->setProperties(m_properties); cond->setSequenceNum(m_sequential_number); d->undoStack().push(new AddGraphicsObjectCommand(cond, d)); @@ -224,6 +226,41 @@ QList ConductorCreator::existingPotential(const QList & @brief ConductorCreator::hubTerminal @return hub_terminal */ +/** + @brief ConductorCreator::terminalPairs + @param chain : the project limits the wires per terminal + (discussion #1158, WiringRules::chainsWires()) + @return the pairs of terminals to wire: all to one hub terminal (a + star), or with \p chain one after another (WiringRules::chainOrder()), + since a star gives the hub a wire per other terminal. +*/ +QList> ConductorCreator::terminalPairs(bool chain) +{ + QList> pairs; + if (chain) + { + QList points; + for (Terminal *t : std::as_const(m_terminals_list)) { + points << t->scenePos(); + } + const QList order = WiringRules::chainOrder(points); + for (int i = 1 ; i < order.size() ; ++i) + { + pairs << qMakePair(m_terminals_list.at(order.at(i - 1)), + m_terminals_list.at(order.at(i))); + } + return pairs; + } + + Terminal *hub_terminal = hubTerminal(); + for (Terminal *t : std::as_const(m_terminals_list)) { + if (t != hub_terminal) { + pairs << qMakePair(hub_terminal, t); + } + } + return pairs; +} + Terminal *ConductorCreator::hubTerminal() { Terminal *hub_terminal = m_terminals_list.first(); diff --git a/sources/utils/conductorcreator.h b/sources/utils/conductorcreator.h index ec287bddd..a8c2c9a6a 100644 --- a/sources/utils/conductorcreator.h +++ b/sources/utils/conductorcreator.h @@ -44,6 +44,7 @@ class ConductorCreator static QList existingPotential(const QList &terminals_list); bool setUpPropertieToUse(); Terminal *hubTerminal(); + QList> terminalPairs(bool chain); QList m_terminals_list; diff --git a/sources/wiringrules.cpp b/sources/wiringrules.cpp index fcc3a66a2..45e347813 100644 --- a/sources/wiringrules.cpp +++ b/sources/wiringrules.cpp @@ -124,3 +124,65 @@ bool WiringRules::hasRoom(int limit, int wires) { return limit <= 0 || wires < limit; } + +/** + @brief WiringRules::chainsWires + @return true if QElectroTech's own tools that wire several terminals at + once must wire them one after another (a chain) rather than all to one + of them (a star): a star gives that one terminal a wire per other + terminal, which the project's limit forbids. +*/ +bool WiringRules::chainsWires(const Settings &settings, bool master_enabled) +{ + return master_enabled && settings.max_wires > 0; +} + +/** + @brief WiringRules::chainOrder + The order in which to wire terminals at \a points one after another: + from the top left one (smallest x, then smallest y, the same terminal + the star used as its hub), each time to the nearest terminal not yet + wired. Distance is along the grid (|dx| + |dy|), since wires run + horizontally and vertically; a tie goes to the earlier point. + @return indexes into \a points, each once +*/ +QList WiringRules::chainOrder(const QList &points) +{ + QList order; + if (points.isEmpty()) { + return order; + } + + int current = 0; + for (int i = 1 ; i < points.size() ; ++i) { + const QPointF &p = points.at(i); + const QPointF &c = points.at(current); + if (p.x() < c.x() || (p.x() == c.x() && p.y() < c.y())) { + current = i; + } + } + + QList done(points.size(), false); + order << current; + done[current] = true; + while (order.size() < points.size()) + { + int nearest = -1; + qreal nearest_distance = 0; + for (int i = 0 ; i < points.size() ; ++i) { + if (done.at(i)) { + continue; + } + const QPointF d = points.at(i) - points.at(current); + const qreal distance = qAbs(d.x()) + qAbs(d.y()); + if (nearest < 0 || distance < nearest_distance) { + nearest = i; + nearest_distance = distance; + } + } + order << nearest; + done[nearest] = true; + current = nearest; + } + return order; +} diff --git a/sources/wiringrules.h b/sources/wiringrules.h index f9cab9247..50842dfe2 100644 --- a/sources/wiringrules.h +++ b/sources/wiringrules.h @@ -19,6 +19,9 @@ #ifndef WIRINGRULES_H #define WIRINGRULES_H +#include +#include + class QDomElement; /** @@ -63,6 +66,9 @@ namespace WiringRules int limit(const Settings &settings, bool master_enabled, bool is_report); bool hasRoom(int limit, int wires); + + bool chainsWires(const Settings &settings, bool master_enabled); + QList chainOrder(const QList &points); } #endif // WIRINGRULES_H diff --git a/tests/qttest/tst_wiringrules.cpp b/tests/qttest/tst_wiringrules.cpp index 79353cf26..683d3fd10 100644 --- a/tests/qttest/tst_wiringrules.cpp +++ b/tests/qttest/tst_wiringrules.cpp @@ -124,6 +124,49 @@ class tst_wiringrules : public QObject return {}; } + // On a fixture with @p rules: a symbol wired to four others is + // deleted, so QElectroTech rewires the four to keep the potential. + // Returns the most wires any one of them ends with. + QString mostWiresAfterDeletingTheHub(const QString &rules) + { + const QString project = fixtureWith(rules); + const QString script_path = m_dir.filePath(QStringLiteral("hub%1.js").arg(m_run)); + QFile script(script_path); + if (project.isEmpty() || !script.open(QIODevice::WriteOnly)) + return {}; + // Placed on a diagonal, so no two terminals line up and nothing + // is auto-connected. + script.write( + "var p = 'embed://import/probe/v2_fuse.elmt';\n" + "var hub = qet.addElement(0, p, 400, 400);\n" + "var others = [];\n" + "for (var i = 1; i <= 4; ++i) others.push(qet.addElement(0, p, 400 + 70 * i, 400 + 90 * i));\n" + "others.forEach(function (o) { qet.addConductor(0, hub, 0, o, 0); });\n" + "qet.deleteElement(0, hub);\n" + "var count = {};\n" + "qet.conductorUuids(0).forEach(function (u) {\n" + " qet.conductorEnds(0, u).forEach(function (e) { count[e] = (count[e] || 0) + 1; }); });\n" + "var most = 0;\n" + "others.forEach(function (o) { most = Math.max(most, count[o + ' terminal 0'] || 0); });\n" + "qet.log('PROBE ' + most);\n"); + script.close(); + + QProcess proc; + proc.setProcessEnvironment(sandbox()); + proc.start(QStringLiteral(QET_TEST_BINARY_PATH), {QStringLiteral("--run"), script_path, project}); + if (!proc.waitForFinished(60000)) + return {}; + const QString out = QString::fromUtf8(proc.readAllStandardOutput() + + proc.readAllStandardError()); + const QString mark = QStringLiteral("PROBE "); + for (const QString &line : out.split(QLatin1Char('\n'))) { + const int i = line.indexOf(mark); + if (i >= 0) + return line.mid(i + mark.size()).trimmed(); + } + return {}; + } + private slots: void initTestCase() { @@ -160,6 +203,39 @@ private slots: QVERIFY(!WiringRules::hasRoom(1, 1)); } + void chainsOnlyUnderALimit() + { + WiringRules::Settings rules; + QVERIFY(!WiringRules::chainsWires(rules, true)); + rules.one_wire_per_report = true; + QVERIFY(!WiringRules::chainsWires(rules, true)); + rules.max_wires = 2; + QVERIFY(WiringRules::chainsWires(rules, true)); + QVERIFY(!WiringRules::chainsWires(rules, false)); + } + + void chainOrder() + { + QCOMPARE(WiringRules::chainOrder({}), QList()); + QCOMPARE(WiringRules::chainOrder({QPointF(5, 5)}), QList({0})); + + // Starts top left, then always the nearest along the grid + const QList row {QPointF(300, 0), QPointF(0, 0), QPointF(200, 0), QPointF(100, 0)}; + QCOMPARE(WiringRules::chainOrder(row), QList({1, 3, 2, 0})); + + // Same x: the higher one starts + const QList column {QPointF(0, 100), QPointF(0, 0), QPointF(0, 50)}; + QCOMPARE(WiringRules::chainOrder(column), QList({1, 2, 0})); + + // Each index exactly once + const QList scattered {QPointF(40, 90), QPointF(10, 300), QPointF(220, 10), + QPointF(10, 10), QPointF(220, 300)}; + QList order = WiringRules::chainOrder(scattered); + QCOMPARE(order.first(), 3); + std::sort(order.begin(), order.end()); + QCOMPARE(order, QList({0, 1, 2, 3, 4})); + } + void xmlRoundTrip() { QDomDocument doc; @@ -210,6 +286,17 @@ private slots: // The master switch off: the project's rule does nothing QCOMPARE(addWireToWiredTerminal(limited, true), QStringLiteral("true")); } + + void deletingASymbolChainsTheWires() + { + // No rule: the four are wired to one of them, as on master + QCOMPARE(mostWiresAfterDeletingTheHub(QString()), QStringLiteral("3")); + + // A limit: one after another, none gets more than two + QCOMPARE(mostWiresAfterDeletingTheHub( + QStringLiteral("")), + QStringLiteral("2")); + } }; QTEST_GUILESS_MAIN(tst_wiringrules) From 09380f0ff73903d12f8400b02ad6f4cdb20af1af Mon Sep 17 00:00:00 2001 From: ispyisail Date: Sun, 4 Oct 2026 00:24:29 +1300 Subject: [PATCH 3/3] tst_wiringrules: skip deletingASymbolChainsTheWires on a build without scripting It drives QElectroTech with --run, which such a build does not have. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015FPuYPS4T7QuEwjNu22rXD --- tests/qttest/tst_wiringrules.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/qttest/tst_wiringrules.cpp b/tests/qttest/tst_wiringrules.cpp index 01e41cd78..77f99630f 100644 --- a/tests/qttest/tst_wiringrules.cpp +++ b/tests/qttest/tst_wiringrules.cpp @@ -355,6 +355,7 @@ private slots: void deletingASymbolChainsTheWires() { + SKIP_WITHOUT_SCRIPTING; // No rule: the four are wired to one of them, as on master QCOMPARE(mostWiresAfterDeletingTheHub(QString()), QStringLiteral("3"));