From fdd04d5a40ed172bd74661854232d694609c7721 Mon Sep 17 00:00:00 2001 From: CoolSpy3 <55305038+CoolSpy3@users.noreply.github.com> Date: Sun, 22 Dec 2024 02:22:11 -0800 Subject: [PATCH] Fix `WbField::isParameter` for Unconnected Fields (#6735) * fix WbField::isParameter * fix dependencies on old behavior * remove WbTemplateManager::regenerateNodeFromField/ParameterChange * move WbField::isParameter to cpp file * fix WbField::isParameter definition * update changelog and fix PR reference comment * formatting --- docs/reference/changelog-r2024.md | 1 + src/webots/nodes/utils/WbTemplateManager.cpp | 27 ++++++------------- src/webots/nodes/utils/WbTemplateManager.hpp | 4 +-- src/webots/scene_tree/WbAddNodeDialog.cpp | 2 +- .../scene_tree/WbExtendedStringEditor.cpp | 2 +- src/webots/vrml/WbField.cpp | 6 +++++ src/webots/vrml/WbField.hpp | 2 +- 7 files changed, 19 insertions(+), 25 deletions(-) diff --git a/docs/reference/changelog-r2024.md b/docs/reference/changelog-r2024.md index ccce0aa12c2..c8f495b3a51 100644 --- a/docs/reference/changelog-r2024.md +++ b/docs/reference/changelog-r2024.md @@ -33,3 +33,4 @@ Released on December **th, 2023. - Fixed bug where the rotation axis of the omni wheel rollers were incorrect ([6710](https://github.com/cyberbotics/webots/pull/6710)). - Fixed a bug where Webots would crash if a geometry was inserted into a `Shape` node used a bounding box ([#6691](https://github.com/cyberbotics/webots/pull/6691)). - Removed the old `wb_supervisor_field_import_sf_node` and `wb_supervisor_field_import_mf_node` functions from the list of editor autocomplete suggestions ([#6701](https://github.com/cyberbotics/webots/pull/6701)). + - Fixed a bug preventing nodes from being inserted into unconnected proto fields ([#6735](https://github.com/cyberbotics/webots/pull/6735)). diff --git a/src/webots/nodes/utils/WbTemplateManager.cpp b/src/webots/nodes/utils/WbTemplateManager.cpp index 3bfda1e37c6..0d850c72d64 100644 --- a/src/webots/nodes/utils/WbTemplateManager.cpp +++ b/src/webots/nodes/utils/WbTemplateManager.cpp @@ -131,10 +131,9 @@ bool WbTemplateManager::nodeNeedsToSubscribe(WbNode *node) { void WbTemplateManager::recursiveFieldSubscribeToRegenerateNode(WbNode *node, bool subscribedNode, bool subscribedDescendant) { if (subscribedNode || subscribedDescendant) { if (node->isProtoInstance()) - connect(node, &WbNode::parameterChanged, this, &WbTemplateManager::regenerateNodeFromParameterChange, - Qt::UniqueConnection); + connect(node, &WbNode::parameterChanged, this, &WbTemplateManager::regenerateNodeFromField, Qt::UniqueConnection); else - connect(node, &WbNode::fieldChanged, this, &WbTemplateManager::regenerateNodeFromFieldChange, Qt::UniqueConnection); + connect(node, &WbNode::fieldChanged, this, &WbTemplateManager::regenerateNodeFromField, Qt::UniqueConnection); } // if PROTO node: @@ -177,25 +176,15 @@ void WbTemplateManager::recursiveFieldSubscribeToRegenerateNode(WbNode *node, bo } } -void WbTemplateManager::regenerateNodeFromFieldChange(WbField *field) { - // retrieve the right node - WbNode *templateNode = dynamic_cast(sender()); - assert(templateNode); - if (templateNode) - regenerateNodeFromField(templateNode, field, false); -} - -void WbTemplateManager::regenerateNodeFromParameterChange(WbField *field) { +// intermediate function to determine which node should be updated +// Note: The security is probably overkill there, but its also safer for the first versions of the template mechanism +void WbTemplateManager::regenerateNodeFromField(WbField *field) { // retrieve the right node WbNode *templateNode = dynamic_cast(sender()); assert(templateNode); - if (templateNode) - regenerateNodeFromField(templateNode, field, true); -} + if (!templateNode) + return; -// intermediate function to determine which node should be updated -// Note: The security is probably overkill there, but its also safer for the first versions of the template mechanism -void WbTemplateManager::regenerateNodeFromField(WbNode *templateNode, WbField *field, bool isParameter) { // 1. retrieve upper template node where the modification appeared in a template regenerator field WbNode *upperTemplateNode = WbVrmlNodeUtilities::findUpperTemplateNeedingRegenerationFromField(field, templateNode); @@ -203,7 +192,7 @@ void WbTemplateManager::regenerateNodeFromField(WbNode *templateNode, WbField *f return; // 2. check it's not a parameter managed by ODE - if (!isParameter && dynamic_cast(templateNode) && + if (!field->isParameter() && dynamic_cast(templateNode) && ((field->name() == "translation" && field->type() == WB_SF_VEC3F) || (field->name() == "rotation" && field->type() == WB_SF_ROTATION) || (field->name() == "position" && field->type() == WB_SF_FLOAT))) diff --git a/src/webots/nodes/utils/WbTemplateManager.hpp b/src/webots/nodes/utils/WbTemplateManager.hpp index 6929d75cfb8..9429ed2783a 100644 --- a/src/webots/nodes/utils/WbTemplateManager.hpp +++ b/src/webots/nodes/utils/WbTemplateManager.hpp @@ -52,8 +52,7 @@ class WbTemplateManager : public QObject { private slots: void unsubscribe(QObject *node); - void regenerateNodeFromFieldChange(WbField *field); - void regenerateNodeFromParameterChange(WbField *field); + void regenerateNodeFromField(WbField *field); void regenerateNode(WbNode *node, bool restarted = false); void nodeNeedRegeneration(); @@ -67,7 +66,6 @@ private slots: bool nodeNeedsToSubscribe(WbNode *node); void recursiveFieldSubscribeToRegenerateNode(WbNode *node, bool subscribedNode, bool subscribedDescendant); - void regenerateNodeFromField(WbNode *templateNode, WbField *field, bool isParameter); QList mTemplates; QSet mNodesSubscribedForRegeneration; diff --git a/src/webots/scene_tree/WbAddNodeDialog.cpp b/src/webots/scene_tree/WbAddNodeDialog.cpp index 7964bffd1b3..4b0f233c8d0 100644 --- a/src/webots/scene_tree/WbAddNodeDialog.cpp +++ b/src/webots/scene_tree/WbAddNodeDialog.cpp @@ -450,7 +450,7 @@ void WbAddNodeDialog::buildTree() { static const QString INVALID_FOR_INSERTION_IN_BOUNDING_OBJECT("N"); const WbField *const actualField = - (mField->isParameter() && !mField->alias().isEmpty()) ? mField->internalFields().at(0) : mField; + (!mField->internalFields().isEmpty() && !mField->alias().isEmpty()) ? mField->internalFields().at(0) : mField; bool boInfo = actualField->name() == "boundingObject"; if (!boInfo) boInfo = nodeUse & WbNode::BOUNDING_OBJECT_USE; diff --git a/src/webots/scene_tree/WbExtendedStringEditor.cpp b/src/webots/scene_tree/WbExtendedStringEditor.cpp index 0f9709048a2..ce1ecc30fa5 100644 --- a/src/webots/scene_tree/WbExtendedStringEditor.cpp +++ b/src/webots/scene_tree/WbExtendedStringEditor.cpp @@ -453,7 +453,7 @@ void WbExtendedStringEditor::edit(bool copyOriginalValue) { if (copyOriginalValue) { const WbField *effectiveField = field(); - if (effectiveField->isParameter()) + if (!effectiveField->internalFields().isEmpty()) effectiveField = effectiveField->internalFields().at(0); mStringType = fieldNameToStringType(effectiveField->name(), effectiveField->parentNode()); diff --git a/src/webots/vrml/WbField.cpp b/src/webots/vrml/WbField.cpp index 23838c6ec6b..f5ba894fe9b 100644 --- a/src/webots/vrml/WbField.cpp +++ b/src/webots/vrml/WbField.cpp @@ -104,6 +104,12 @@ bool WbField::isDeprecated() const { return mModel->isDeprecated(); } +// Because of unconnected fields, the only way to definitively check if a field is a parameter is to check its parent node +// If that is not possible, fallback to the old behavior (See #6604 and #6735) +bool WbField::isParameter() const { + return parentNode() ? parentNode()->isProtoInstance() : !mInternalFields.isEmpty(); +} + void WbField::readValue(WbTokenizer *tokenizer, const QString &worldPath) { if (mWasRead) tokenizer->reportError(tr("Duplicate field value: '%1'").arg(name())); diff --git a/src/webots/vrml/WbField.hpp b/src/webots/vrml/WbField.hpp index 016b21cbade..86ef9aca40e 100644 --- a/src/webots/vrml/WbField.hpp +++ b/src/webots/vrml/WbField.hpp @@ -69,7 +69,7 @@ class WbField : public QObject { void redirectTo(WbField *parameter, bool skipCopy = false); WbField *parameter() const { return mParameter; } const QList &internalFields() const { return mInternalFields; } - bool isParameter() const { return mInternalFields.size() != 0; } + bool isParameter() const; void clearInternalFields() { mInternalFields.clear(); }