Added DiffManager system, with simple undo/redo

src/definition/DefinitionDiff.cpp

@@ -2,7 +2,7 @@
 
 #include "DefinitionNode.h"
 
-DefinitionDiff::DefinitionDiff(const std::string &type_name):
-    type_name(type_name)
+DefinitionDiff::DefinitionDiff(const DefinitionNode *node):
+    type_name(node->getTypeName()), path(node->getPath())
 {
 }

src/definition/DefinitionDiff.h

@@ -14,12 +14,14 @@ namespace definition {
 class DEFINITIONSHARED_EXPORT DefinitionDiff
 {
 public:
-    DefinitionDiff(const std::string &type_name);
+    DefinitionDiff(const DefinitionNode *node);
 
     inline const std::string &getTypeName() const {return type_name;}
+    inline const std::string &getPath() const {return path;}
 
 private:
     std::string type_name;
+    std::string path;
 };
 
 }

src/definition/DefinitionNode.cpp

@@ -3,6 +3,9 @@
 #include "Logs.h"
 #include "PackStream.h"
 #include "DefinitionDiff.h"
+#include "DiffManager.h"
+
+#include <cassert>
 
 DefinitionNode::DefinitionNode(DefinitionNode* parent, const std::string &name, const std::string &type_name):
     parent(parent), type_name(type_name), name(name)
@@ -11,10 +14,12 @@ DefinitionNode::DefinitionNode(DefinitionNode* parent, const std::string &name,
     {
         root = parent->root;
         parent->addChild(this);
+        diffs = NULL;
     }
     else
     {
         root = this;
+        diffs = new DiffManager(this);
     }
 }
 
@@ -26,6 +31,12 @@ DefinitionNode::~DefinitionNode()
         parent = NULL;
     }
 
+    if (diffs)
+    {
+        delete diffs;
+        diffs = NULL;
+    }
+
     // Work on a copy, because the child destructor will modify the array by removing itself using removeChild
     std::vector<DefinitionNode*> children_copy = children;
     for (auto child:children_copy)
@@ -72,6 +83,66 @@ std::string DefinitionNode::toString(int indent) const
     return result;
 }
 
+std::string DefinitionNode::getPath() const
+{
+    if (parent == root)
+    {
+        return parent->getPath() + name;
+    }
+    else if (parent)
+    {
+        return parent->getPath() + "/" + name;
+    }
+    else
+    {
+        return "/";
+    }
+}
+
+DefinitionNode *DefinitionNode::findByPath(const std::string &path) const
+{
+    if (path.empty())
+    {
+        return NULL;
+    }
+    else if (path[0] == '/')
+    {
+        if (path.length() == 1)
+        {
+            return root;
+        }
+        else if (root == this)
+        {
+            return findByPath(path.substr(1));
+        }
+        else
+        {
+            return root->findByPath(path);
+        }
+    }
+    else
+    {
+        size_t seppos = path.find("/");
+        std::string child_name = (seppos == std::string::npos) ? path : path.substr(0, seppos);
+        DefinitionNode *child = ((DefinitionNode *)this)->findChildByName(child_name);  // FIXME findChildByName should be const
+        if (child)
+        {
+            if (seppos == std::string::npos)
+            {
+                return child;
+            }
+            else
+            {
+                return child->findByPath(path.substr(seppos + 1));
+            }
+        }
+        else
+        {
+            return NULL;
+        }
+    }
+}
+
 bool DefinitionNode::applyDiff(const DefinitionDiff *diff, bool)
 {
     // Only do type check, subclasses will do the rest
@@ -86,6 +157,14 @@ bool DefinitionNode::applyDiff(const DefinitionDiff *diff, bool)
     }
 }
 
+void DefinitionNode::addWatcher(DefinitionWatcher *watcher)
+{
+    if (root && root->diffs)
+    {
+        root->diffs->addWatcher(this, watcher);
+    }
+}
+
 void DefinitionNode::save(PackStream* stream) const
 {
     int children_count = (int)children.size();
@@ -202,3 +281,13 @@ int DefinitionNode::getStreamSize() const
 {
     return -1;
 }
+
+void DefinitionNode::addDiff(const DefinitionDiff *diff)
+{
+    assert(diff->getTypeName() == type_name);
+
+    if (root && root->diffs)
+    {
+        root->diffs->addDiff(this, diff);
+    }
+}

src/definition/DefinitionNode.h

@@ -30,6 +30,7 @@ public:
 
     inline const DefinitionNode *getParent() const {return parent;}
     inline const DefinitionNode *getRoot() const {return root;}
+    inline DiffManager *getDiffManager() const {return diffs;}
     inline int getChildrenCount() const {return children.size();}
 
     /**
@@ -37,6 +38,18 @@ public:
      */
     virtual std::string toString(int indent = 0) const;
 
+    /**
+     * Return the path to this node, using '/' delimited syntax, with the first '/' being the root node.
+     */
+    std::string getPath() const;
+
+    /**
+     * Find a node in this tree, by its path (as returned by getPath).
+     *
+     * Return NULL if the path does not exists.
+     */
+    DefinitionNode *findByPath(const std::string &path) const;
+
     /**
      * Apply a diff to the internal value of this node.
      *
@@ -48,6 +61,13 @@ public:
      */
     virtual bool applyDiff(const DefinitionDiff *diff, bool backward=false);
 
+    /**
+     * Add a watcher over this node.
+     *
+     * The watcher will receive DefinitionDiff objects when this node changes.
+     */
+    void addWatcher(DefinitionWatcher *watcher);
+
 protected:
     void addChild(DefinitionNode* child);
     void removeChild(DefinitionNode* child);
@@ -61,9 +81,19 @@ protected:
      */
     int getStreamSize() const;
 
+    /**
+     * Add a diff to the DiffManager of this definition tree, for the current node.
+     *
+     * The manager will take immediate ownership of the diff, handling its freeing.
+     *
+     * The manager will decide if the diff should be committed and will call *applyDiff* if needed.
+     */
+    void addDiff(const DefinitionDiff *diff);
+
 private:
     DefinitionNode *parent;
     DefinitionNode *root;
+    DiffManager *diffs;
     std::string type_name;
     std::string name;
     std::vector<DefinitionNode*> children;

src/definition/DefinitionWatcher.cpp

@@ -0,0 +1,7 @@
+#include "DefinitionWatcher.h"
+
+DefinitionWatcher::DefinitionWatcher()
+{
+
+}
+

src/definition/DefinitionWatcher.h

@@ -0,0 +1,23 @@
+#ifndef DEFINITIONWATCHER_H
+#define DEFINITIONWATCHER_H
+
+#include "definition_global.h"
+
+namespace paysages {
+namespace definition {
+
+/**
+ * Base class for watchers of the definition tree.
+ *
+ * Watchers will be registered in DiffManager to receive DefinitionDiff objects.
+ */
+class DEFINITIONSHARED_EXPORT DefinitionWatcher
+{
+public:
+    DefinitionWatcher();
+};
+
+}
+}
+
+#endif // DEFINITIONWATCHER_H

src/definition/DiffManager.cpp

@@ -0,0 +1,55 @@
+#include "DiffManager.h"
+
+#include "DefinitionNode.h"
+#include "DefinitionDiff.h"
+
+DiffManager::DiffManager(DefinitionNode *tree):
+    tree(tree)
+{
+    undone = 0;
+}
+
+void DiffManager::addWatcher(const DefinitionNode *node, DefinitionWatcher *watcher)
+{
+    watchers[node].push_back(watcher);
+}
+
+void DiffManager::addDiff(DefinitionNode *node, const DefinitionDiff *diff)
+{
+    diffs.push_back(diff);
+
+    // TODO Delayed commit (with merge of consecutive diffs)
+    node->applyDiff(diff);
+
+    for (auto &watcher: watchers[node])
+    {
+        // TODO
+    }
+}
+
+void DiffManager::undo()
+{
+    if (undone <= (int)diffs.size())
+    {
+        undone++;
+        const DefinitionDiff *diff = diffs[diffs.size() - undone];
+
+        // Obtain the node by path and reverse apply diff on it
+        DefinitionNode *node = tree->findByPath(diff->getPath());
+        node->applyDiff(diff, true);
+    }
+}
+
+void DiffManager::redo()
+{
+    if (undone > 0)
+    {
+        const DefinitionDiff *diff = diffs[diffs.size() - undone];
+        undone--;
+
+        // Obtain the node by path and re-apply diff on it
+        DefinitionNode *node = tree->findByPath(diff->getPath());
+        node->applyDiff(diff);
+    }
+}
+

src/definition/DiffManager.h

@@ -0,0 +1,54 @@
+#ifndef DIFFMANAGER_H
+#define DIFFMANAGER_H
+
+#include "definition_global.h"
+
+#include <map>
+#include <vector>
+
+namespace paysages {
+namespace definition {
+
+/**
+ * Manager of diffs produced by a definition tree.
+ *
+ * Watchers can register themselves to received these diffs.
+ */
+class DEFINITIONSHARED_EXPORT DiffManager
+{
+public:
+    DiffManager(DefinitionNode *tree);
+
+    /**
+     * Add a watcher for a specific node.
+     *
+     * The watcher reference must remain valid through the manager lifetime.
+     */
+    void addWatcher(const DefinitionNode *node, DefinitionWatcher *watcher);
+
+    /**
+     * Add a new diff of a node to the change flow.
+     */
+    void addDiff(DefinitionNode *node, const DefinitionDiff *diff);
+
+    /**
+     * Undo a diff in the definition tree.
+     */
+    void undo();
+
+    /**
+     * Redo a diff in the definition tree, undone by undo().
+     */
+    void redo();
+
+private:
+    DefinitionNode *tree;
+    int undone;
+    std::vector<const DefinitionDiff *> diffs;
+    std::map<const DefinitionNode *, std::vector<DefinitionWatcher *>> watchers;
+};
+
+}
+}
+
+#endif // DIFFMANAGER_H

src/definition/FloatDiff.cpp

@@ -1,6 +1,6 @@
 #include "FloatDiff.h"
 
-FloatDiff::FloatDiff(double oldvalue, double newvalue):
-    DefinitionDiff("float"), oldvalue(oldvalue), newvalue(newvalue)
+FloatDiff::FloatDiff(const DefinitionNode *node, double oldvalue, double newvalue):
+    DefinitionDiff(node), oldvalue(oldvalue), newvalue(newvalue)
 {
 }

src/definition/FloatDiff.h

@@ -14,7 +14,7 @@ namespace definition {
 class DEFINITIONSHARED_EXPORT FloatDiff: public DefinitionDiff
 {
 public:
-    FloatDiff(double oldvalue, double newvalue);
+    FloatDiff(const DefinitionNode *node, double oldvalue, double newvalue);
 
     inline double getOldValue() const {return oldvalue;}
     inline double getNewValue() const {return newvalue;}

src/definition/FloatNode.cpp

@@ -42,9 +42,14 @@ void FloatNode::copy(DefinitionNode *destination) const
     }
 }
 
+void FloatNode::setValue(double new_value)
+{
+    addDiff(produceDiff(new_value));
+}
+
 const FloatDiff *FloatNode::produceDiff(double new_value) const
 {
-    return new FloatDiff(value, new_value);
+    return new FloatDiff(this, value, new_value);
 }
 
 bool FloatNode::applyDiff(const DefinitionDiff *diff, bool backward)

src/definition/FloatNode.h

@@ -23,6 +23,12 @@ public:
     virtual void load(PackStream* stream) override;
     virtual void copy(DefinitionNode* destination) const override;
 
+    /**
+     * Change the float value stored.
+     *
+     * The DiffManager is used as intermediary, so that the change may not happen immediately.
+     */
+    void setValue(double new_value);
     const FloatDiff *produceDiff(double new_value) const;
     virtual bool applyDiff(const DefinitionDiff *diff, bool backward=false) override;
 private:

src/definition/definition.pro

@@ -33,7 +33,9 @@ SOURCES += \
     DefinitionNode.cpp \
     FloatNode.cpp \
     DefinitionDiff.cpp \
-    FloatDiff.cpp
+    FloatDiff.cpp \
+    DiffManager.cpp \
+    DefinitionWatcher.cpp
 
 HEADERS +=\
         definition_global.h \
@@ -56,7 +58,9 @@ HEADERS +=\
     DefinitionNode.h \
     FloatNode.h \
     DefinitionDiff.h \
-    FloatDiff.h
+    FloatDiff.h \
+    DiffManager.h \
+    DefinitionWatcher.h
 
 unix:!symbian {
     maemo5 {

src/definition/definition_global.h

@@ -17,6 +17,8 @@ namespace definition {
     class DefinitionDiff;
     class FloatNode;
     class FloatDiff;
+    class DiffManager;
+    class DefinitionWatcher;
     class Scenery;
     class CameraDefinition;
     class SurfaceMaterial;

src/tests/DefinitionNode_Test.cpp

@@ -15,6 +15,35 @@ TEST(DefinitionNode, toString)
     EXPECT_EQ("branch\n leaf1\n leaf2", branch.toString());
 }
 
+TEST(DefinitionNode, getPath)
+{
+    DefinitionNode root(NULL, "root");
+    DefinitionNode branch(&root, "branch");
+    DefinitionNode leaf(&branch, "leaf");
+
+    EXPECT_EQ("/", root.getPath());
+    EXPECT_EQ("/branch", branch.getPath());
+    EXPECT_EQ("/branch/leaf", leaf.getPath());
+}
+
+TEST(DefinitionNode, findByPath)
+{
+    DefinitionNode root(NULL, "root");
+    DefinitionNode branch(&root, "branch");
+    DefinitionNode leaf(&branch, "leaf");
+
+    EXPECT_EQ(&root, root.findByPath("/"));
+    EXPECT_EQ(&branch, root.findByPath("/branch"));
+    EXPECT_EQ(NULL, root.findByPath("/branche"));
+    EXPECT_EQ(&leaf, root.findByPath("/branch/leaf"));
+    EXPECT_EQ(NULL, root.findByPath("/branche/leaf"));
+    EXPECT_EQ(NULL, root.findByPath("/branch/leave"));
+
+    EXPECT_EQ(&branch, root.findByPath("branch"));
+    EXPECT_EQ(&leaf, root.findByPath("branch/leaf"));
+    EXPECT_EQ(&leaf, branch.findByPath("leaf"));
+}
+
 TEST(DefinitionNode, attachDetach)
 {
     DefinitionNode* root = new DefinitionNode(NULL, "root");

src/tests/DiffManager_Test.cpp

@@ -0,0 +1,32 @@
+#include "BaseTestCase.h"
+
+#include "DiffManager.h"
+#include "DefinitionNode.h"
+#include "FloatNode.h"
+
+TEST(DiffManager, undoRedoSimple)
+{
+    DefinitionNode root(NULL, "root");
+    FloatNode leaf(&root, "value", 2.6);
+    DiffManager *diffs = root.getDiffManager();
+
+    EXPECT_DOUBLE_EQ(2.6, leaf.getValue());
+
+    leaf.setValue(4.3);
+    EXPECT_DOUBLE_EQ(4.3, leaf.getValue());
+
+    leaf.setValue(-2.1);
+    EXPECT_DOUBLE_EQ(-2.1, leaf.getValue());
+
+    diffs->undo();
+    EXPECT_DOUBLE_EQ(4.3, leaf.getValue());
+
+    diffs->undo();
+    EXPECT_DOUBLE_EQ(2.6, leaf.getValue());
+
+    diffs->redo();
+    EXPECT_DOUBLE_EQ(4.3, leaf.getValue());
+
+    diffs->redo();
+    EXPECT_DOUBLE_EQ(-2.1, leaf.getValue());
+}

src/tests/FloatNode_Test.cpp

@@ -67,8 +67,9 @@ TEST(FloatNode, produceDiff)
 TEST(FloatNode, applyDiff)
 {
     FloatNode node(NULL, "test", 1.2);
-    FloatDiff diff(1.2, 2.4);
-    DefinitionDiff odiff("badtype");
+    FloatDiff diff(&node, 1.2, 2.4);
+    DefinitionNode onode(NULL, "test", "badtype");
+    DefinitionDiff odiff(&onode);
     bool result;
 
     EXPECT_DOUBLE_EQ(1.2, node.getValue());

src/tests/tests.pro

@@ -24,7 +24,8 @@ SOURCES += main.cpp \
     AtmosphereDefinition_Test.cpp \
     Scenery_Test.cpp \
     DefinitionNode_Test.cpp \
-    FloatNode_Test.cpp
+    FloatNode_Test.cpp \
+    DiffManager_Test.cpp
 
 HEADERS += \
     BaseTestCase.h