Fixed|libcore|Scripting: RecordValue duplication behavior

RecordValue is now duplicated like any other value: a true duplicate
copy is constructed with the exact same characteristics.

Record no longer needs to make a special case when copying subrecords.

NameExpression now handles RecordValues separately and specifically
makes unowned duplicates out of them, because during expression
evaluation, records should always be handled via reference.

doomsday/libcore/include/de/data/record.h

@@ -67,7 +67,7 @@ class DENG2_PUBLIC Record
     typedef std::pair<String, String> KeyValue;
     typedef QList<KeyValue> List;
 
-    enum CopyBehavior {
+    enum Behavior {
         AllMembers,
         IgnoreDoubleUnderscoreMembers
     };
@@ -87,14 +87,16 @@ class DENG2_PUBLIC Record
      * @param other     Record to copy.
      * @param behavior  Which members to copy.
      */
-    Record(Record const &other, CopyBehavior behavior = AllMembers);
+    Record(Record const &other, Behavior behavior = AllMembers);
 
     virtual ~Record();
 
     /**
      * Deletes all the variables in the record.
+     *
+     * @param behavior  Clear behavior: which members to remove.
      */
-    void clear();
+    void clear(Behavior behavior = AllMembers);
 
     /**
      * Adds a copy of each member of another record into this record. The
@@ -104,13 +106,24 @@ class DENG2_PUBLIC Record
      * @param other     Record whose members are to be copied.
      * @param behavior  Copy behavior.
      */
-    void copyMembersFrom(Record const &other, CopyBehavior behavior = AllMembers);
+    void copyMembersFrom(Record const &other, Behavior behavior = AllMembers);
 
     /**
      * Assignment operator.
+     * @return This record.
      */
     Record &operator = (Record const &other);
 
+    /**
+     * Assignment with specific behavior. All existing members in this record
+     * are cleared (unless ignored due to @a behavior).
+     *
+     * @param behavior  Which members to assign.
+     *
+     * @return This record.
+     */
+    Record &assign(Record const &other, Behavior behavior = AllMembers);
+
     /**
      * Determines if the record contains a variable or a subrecord named @a variableName.
      */

doomsday/libcore/include/de/data/recordvalue.h

@@ -119,6 +119,8 @@ class DENG2_PUBLIC RecordValue : public Value, DENG2_OBSERVES(Record, Deletion)
     // Observes Record deletion.
     void recordBeingDeleted(Record &record);
 
+    RecordValue *duplicateUnowned() const;
+
 public:
     DENG2_PRIVATE(d)
 };

doomsday/libcore/src/data/record.cpp

@@ -183,7 +183,7 @@ DENG2_AUDIENCE_METHOD(Record, Removal)
 Record::Record() : RecordAccessor(this), d(new Instance(*this))
 {}
 
-Record::Record(Record const &other, CopyBehavior behavior)
+Record::Record(Record const &other, Behavior behavior)
     : RecordAccessor(this)
     , d(new Instance(*this))
 {
@@ -199,55 +199,53 @@ Record::~Record()
     clear();
 }
 
-void Record::clear()
+void Record::clear(Behavior behavior)
 {
     if(!d->members.empty())
     {
+        Record::Members remaining; // Contains all members that are not removed.
+
         DENG2_FOR_EACH(Members, i, d->members)
         {
+            if(behavior == IgnoreDoubleUnderscoreMembers &&
+               i.key().startsWith("__"))
+            {
+                remaining.insert(i.key(), i.value());
+                continue;
+            }
+
             DENG2_FOR_AUDIENCE2(Removal, o) o->recordMemberRemoved(*this, **i);
 
             i.value()->audienceForDeletion() -= this;
             delete i.value();
         }
-        d->members.clear();
+
+        d->members = remaining;
     }
 }
 
-void Record::copyMembersFrom(Record const &other, CopyBehavior behavior)
+void Record::copyMembersFrom(Record const &other, Behavior behavior)
 {
     DENG2_FOR_EACH_CONST(Members, i, other.d->members)
     {
         if(behavior == IgnoreDoubleUnderscoreMembers &&
            i.key().startsWith("__")) continue;
 
         Variable *var = new Variable(*i.value());
-
-        // Ownerships of copied subrecords should be retained in the copy.
-        if(RecordValue *recVal = var->value().maybeAs<RecordValue>())
-        {
-            DENG2_ASSERT(!recVal->hasOwnership()); // RecordValue duplication behavior
-
-            RecordValue const &original = i.value()->value().as<RecordValue>();
-            if(original.hasOwnership())
-            {
-                DENG2_ASSERT(recVal->record() == original.record());
-
-                // Make a true copy of the subrecord.
-                recVal->setRecord(new Record(*recVal->record(), behavior),
-                                  RecordValue::OwnsRecord);
-            }
-        }
-
         var->audienceForDeletion() += this;
         d->members[i.key()] = var;
     }
 }
 
 Record &Record::operator = (Record const &other)
 {
-    clear();
-    copyMembersFrom(other);
+    return assign(other);
+}
+
+Record &Record::assign(Record const &other, Behavior behavior)
+{
+    clear(behavior);
+    copyMembersFrom(other, behavior);
     return *this;
 }
 

doomsday/libcore/src/data/recordvalue.cpp

@@ -145,7 +145,17 @@ Record const &RecordValue::dereference() const
 Value *RecordValue::duplicate() const
 {
     verify();
-    /// The return duplicated value does not own the record, just references it.
+    if(hasOwnership())
+    {
+        // Make a complete duplicate using a new record.
+        return new RecordValue(new Record(*d->record), OwnsRecord);
+    }
+    return new RecordValue(d->record);
+}
+
+RecordValue *RecordValue::duplicateUnowned() const
+{
+    verify();
     return new RecordValue(d->record);
 }
 

doomsday/libcore/src/scriptsys/nameexpression.cpp

@@ -169,6 +169,13 @@ Value *NameExpression::evaluate(Evaluator &evaluator) const
             // Reference to the variable.
             return new RefValue(variable);
         }
+        else if(RecordValue *recVal = variable->value().maybeAs<RecordValue>())
+        {
+            // As a special case, RecordValue is always passed as an unowned reference.
+            // One specifically has to request a copy with the built-in Record()
+            // function to get a copy.
+            return recVal->duplicateUnowned();
+        }
         else
         {
             // Variables evaluate to their values.