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

@@ -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)).

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,33 +176,23 @@ void WbTemplateManager::recursiveFieldSubscribeToRegenerateNode(WbNode *node, bo
   }
 }
 
-void WbTemplateManager::regenerateNodeFromFieldChange(WbField *field) {
-  // retrieve the right node
-  WbNode *templateNode = dynamic_cast<WbNode *>(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<WbNode *>(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);
 
   if (!upperTemplateNode)
     return;
 
   // 2. check it's not a parameter managed by ODE
-  if (!isParameter && dynamic_cast<const WbSolid *>(templateNode) &&
+  if (!field->isParameter() && dynamic_cast<const WbSolid *>(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)))

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<WbNode *> mTemplates;
   QSet<const WbNode *> mNodesSubscribedForRegeneration;

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;

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());

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()));

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<WbField *> &internalFields() const { return mInternalFields; }
-  bool isParameter() const { return mInternalFields.size() != 0; }
+  bool isParameter() const;
 
   void clearInternalFields() { mInternalFields.clear(); }