From 22fcd1e55e0e8b4bdc9a9ef0bea0abaa642109d6 Mon Sep 17 00:00:00 2001 From: maelstrom Date: Thu, 10 Apr 2025 18:03:41 +0200 Subject: [PATCH] refactor(serialization): stop using pointers for xml nodes, they already contain one internally --- core/src/datatypes/base.cpp | 24 +++++++------- core/src/datatypes/base.h | 12 +++---- core/src/datatypes/cframe.cpp | 52 +++++++++++++++--------------- core/src/datatypes/cframe.h | 4 +-- core/src/datatypes/color3.cpp | 8 ++--- core/src/datatypes/color3.h | 4 +-- core/src/datatypes/meta.cpp | 10 +++--- core/src/datatypes/meta.h | 4 +-- core/src/datatypes/vector.cpp | 12 +++---- core/src/datatypes/vector.h | 4 +-- core/src/objects/base/instance.cpp | 20 ++++++------ core/src/objects/base/instance.h | 4 +-- core/src/objects/datamodel.cpp | 16 ++++----- core/src/objects/datamodel.h | 2 +- editor/mainwindow.cpp | 12 +++---- 15 files changed, 94 insertions(+), 94 deletions(-) diff --git a/core/src/datatypes/base.cpp b/core/src/datatypes/base.cpp index c7d0b93..fe01029 100644 --- a/core/src/datatypes/base.cpp +++ b/core/src/datatypes/base.cpp @@ -6,7 +6,7 @@ Data::CLASS_NAME::~CLASS_NAME() = default; \ Data::CLASS_NAME::operator const WRAPPED_TYPE() const { return value; } \ const Data::TypeInfo Data::CLASS_NAME::TYPE = { .name = TYPE_NAME, .deserializer = &Data::CLASS_NAME::Deserialize, .fromString = &Data::CLASS_NAME::FromString }; \ const Data::TypeInfo& Data::CLASS_NAME::GetType() const { return Data::CLASS_NAME::TYPE; }; \ -void Data::CLASS_NAME::Serialize(pugi::xml_node* node) const { node->text().set(std::string(this->ToString())); } +void Data::CLASS_NAME::Serialize(pugi::xml_node node) const { node.text().set(std::string(this->ToString())); } Data::Base::~Base() {}; @@ -22,11 +22,11 @@ const Data::String Data::Null::ToString() const { return Data::String("null"); } -void Data::Null::Serialize(pugi::xml_node* node) const { - node->text().set("null"); +void Data::Null::Serialize(pugi::xml_node node) const { + node.text().set("null"); } -Data::Variant Data::Null::Deserialize(pugi::xml_node* node) { +Data::Variant Data::Null::Deserialize(pugi::xml_node node) { return Data::Null(); } @@ -41,8 +41,8 @@ const Data::String Data::Bool::ToString() const { return Data::String(value ? "true" : "false"); } -Data::Variant Data::Bool::Deserialize(pugi::xml_node* node) { - return Data::Bool(node->text().as_bool()); +Data::Variant Data::Bool::Deserialize(pugi::xml_node node) { + return Data::Bool(node.text().as_bool()); } Data::Variant Data::Bool::FromString(std::string string) { @@ -54,8 +54,8 @@ const Data::String Data::Int::ToString() const { return Data::String(std::to_string(value)); } -Data::Variant Data::Int::Deserialize(pugi::xml_node* node) { - return Data::Int(node->text().as_int()); +Data::Variant Data::Int::Deserialize(pugi::xml_node node) { + return Data::Int(node.text().as_int()); } Data::Variant Data::Int::FromString(std::string string) { @@ -67,8 +67,8 @@ const Data::String Data::Float::ToString() const { return Data::String(std::to_string(value)); } -Data::Variant Data::Float::Deserialize(pugi::xml_node* node) { - return Data::Float(node->text().as_float()); +Data::Variant Data::Float::Deserialize(pugi::xml_node node) { + return Data::Float(node.text().as_float()); } Data::Variant Data::Float::FromString(std::string string) { @@ -80,8 +80,8 @@ const Data::String Data::String::ToString() const { return *this; } -Data::Variant Data::String::Deserialize(pugi::xml_node* node) { - return Data::String(node->text().as_string()); +Data::Variant Data::String::Deserialize(pugi::xml_node node) { + return Data::String(node.text().as_string()); } Data::Variant Data::String::FromString(std::string string) { diff --git a/core/src/datatypes/base.h b/core/src/datatypes/base.h index 587b202..cd86cde 100644 --- a/core/src/datatypes/base.h +++ b/core/src/datatypes/base.h @@ -14,14 +14,14 @@ public: \ static const TypeInfo TYPE; \ \ virtual const Data::String ToString() const override; \ - virtual void Serialize(pugi::xml_node* node) const override; \ - static Data::Variant Deserialize(pugi::xml_node* node); \ + virtual void Serialize(pugi::xml_node node) const override; \ + static Data::Variant Deserialize(pugi::xml_node node); \ static Data::Variant FromString(std::string); \ }; namespace Data { class Variant; - typedef std::function Deserializer; + typedef std::function Deserializer; typedef std::function FromString; struct TypeInfo { @@ -36,7 +36,7 @@ namespace Data { virtual ~Base(); virtual const TypeInfo& GetType() const = 0; virtual const Data::String ToString() const = 0; - virtual void Serialize(pugi::xml_node* node) const = 0; + virtual void Serialize(pugi::xml_node node) const = 0; }; class Null : Base { @@ -47,8 +47,8 @@ namespace Data { static const TypeInfo TYPE; virtual const Data::String ToString() const override; - virtual void Serialize(pugi::xml_node* node) const override; - static Data::Variant Deserialize(pugi::xml_node* node); + virtual void Serialize(pugi::xml_node node) const override; + static Data::Variant Deserialize(pugi::xml_node node); }; DEF_WRAPPER_CLASS(Bool, bool) diff --git a/core/src/datatypes/cframe.cpp b/core/src/datatypes/cframe.cpp index ddc5cfd..814739e 100644 --- a/core/src/datatypes/cframe.cpp +++ b/core/src/datatypes/cframe.cpp @@ -112,35 +112,35 @@ Data::CFrame Data::CFrame::operator -(Data::Vector3 vector) const { // Serialization -void Data::CFrame::Serialize(pugi::xml_node* node) const { - node->append_child("X").text().set(std::to_string(this->X())); - node->append_child("Y").text().set(std::to_string(this->Y())); - node->append_child("Z").text().set(std::to_string(this->Z())); - node->append_child("R00").text().set(std::to_string(this->rotation[0][0])); - node->append_child("R01").text().set(std::to_string(this->rotation[1][0])); - node->append_child("R02").text().set(std::to_string(this->rotation[2][0])); - node->append_child("R10").text().set(std::to_string(this->rotation[0][1])); - node->append_child("R11").text().set(std::to_string(this->rotation[1][1])); - node->append_child("R12").text().set(std::to_string(this->rotation[2][1])); - node->append_child("R20").text().set(std::to_string(this->rotation[0][2])); - node->append_child("R21").text().set(std::to_string(this->rotation[1][2])); - node->append_child("R22").text().set(std::to_string(this->rotation[2][2])); +void Data::CFrame::Serialize(pugi::xml_node node) const { + node.append_child("X").text().set(std::to_string(this->X())); + node.append_child("Y").text().set(std::to_string(this->Y())); + node.append_child("Z").text().set(std::to_string(this->Z())); + node.append_child("R00").text().set(std::to_string(this->rotation[0][0])); + node.append_child("R01").text().set(std::to_string(this->rotation[1][0])); + node.append_child("R02").text().set(std::to_string(this->rotation[2][0])); + node.append_child("R10").text().set(std::to_string(this->rotation[0][1])); + node.append_child("R11").text().set(std::to_string(this->rotation[1][1])); + node.append_child("R12").text().set(std::to_string(this->rotation[2][1])); + node.append_child("R20").text().set(std::to_string(this->rotation[0][2])); + node.append_child("R21").text().set(std::to_string(this->rotation[1][2])); + node.append_child("R22").text().set(std::to_string(this->rotation[2][2])); } -Data::Variant Data::CFrame::Deserialize(pugi::xml_node* node) { +Data::Variant Data::CFrame::Deserialize(pugi::xml_node node) { return Data::CFrame( - node->child("X").text().as_float(), - node->child("Y").text().as_float(), - node->child("Z").text().as_float(), - node->child("R00").text().as_float(), - node->child("R01").text().as_float(), - node->child("R02").text().as_float(), - node->child("R10").text().as_float(), - node->child("R11").text().as_float(), - node->child("R12").text().as_float(), - node->child("R20").text().as_float(), - node->child("R21").text().as_float(), - node->child("R22").text().as_float() + node.child("X").text().as_float(), + node.child("Y").text().as_float(), + node.child("Z").text().as_float(), + node.child("R00").text().as_float(), + node.child("R01").text().as_float(), + node.child("R02").text().as_float(), + node.child("R10").text().as_float(), + node.child("R11").text().as_float(), + node.child("R12").text().as_float(), + node.child("R20").text().as_float(), + node.child("R21").text().as_float(), + node.child("R22").text().as_float() ); } \ No newline at end of file diff --git a/core/src/datatypes/cframe.h b/core/src/datatypes/cframe.h index f312d7b..6072d6b 100644 --- a/core/src/datatypes/cframe.h +++ b/core/src/datatypes/cframe.h @@ -37,8 +37,8 @@ namespace Data { static const TypeInfo TYPE; virtual const Data::String ToString() const override; - virtual void Serialize(pugi::xml_node* parent) const override; - static Data::Variant Deserialize(pugi::xml_node* node); + virtual void Serialize(pugi::xml_node parent) const override; + static Data::Variant Deserialize(pugi::xml_node node); operator glm::mat4() const; operator rp::Transform() const; diff --git a/core/src/datatypes/color3.cpp b/core/src/datatypes/color3.cpp index fe457c3..a45fddb 100644 --- a/core/src/datatypes/color3.cpp +++ b/core/src/datatypes/color3.cpp @@ -43,10 +43,10 @@ Data::Color3 Data::Color3::FromHex(std::string hex) { // Serialization -void Data::Color3::Serialize(pugi::xml_node* node) const { - node->text().set(this->ToHex()); +void Data::Color3::Serialize(pugi::xml_node node) const { + node.text().set(this->ToHex()); } -Data::Variant Data::Color3::Deserialize(pugi::xml_node* node) { - return Color3::FromHex(node->text().get()); +Data::Variant Data::Color3::Deserialize(pugi::xml_node node) { + return Color3::FromHex(node.text().get()); } diff --git a/core/src/datatypes/color3.h b/core/src/datatypes/color3.h index d2c4df1..dc1580f 100644 --- a/core/src/datatypes/color3.h +++ b/core/src/datatypes/color3.h @@ -25,8 +25,8 @@ namespace Data { virtual const Data::String ToString() const override; std::string ToHex() const; - virtual void Serialize(pugi::xml_node* node) const override; - static Data::Variant Deserialize(pugi::xml_node* node); + virtual void Serialize(pugi::xml_node node) const override; + static Data::Variant Deserialize(pugi::xml_node node); operator glm::vec3() const; diff --git a/core/src/datatypes/meta.cpp b/core/src/datatypes/meta.cpp index ca511b8..f6b3801 100644 --- a/core/src/datatypes/meta.cpp +++ b/core/src/datatypes/meta.cpp @@ -11,19 +11,19 @@ Data::String Data::Variant::ToString() const { }, this->wrapped); } -void Data::Variant::Serialize(pugi::xml_node* node) const { +void Data::Variant::Serialize(pugi::xml_node node) const { std::visit([&](auto&& it) { it.Serialize(node); }, this->wrapped); } -Data::Variant Data::Variant::Deserialize(pugi::xml_node* node) { - if (Data::TYPE_MAP.count(node->name()) == 0) { - Logger::fatalErrorf("Unknown type for instance: '%s'", node->name()); +Data::Variant Data::Variant::Deserialize(pugi::xml_node node) { + if (Data::TYPE_MAP.count(node.name()) == 0) { + Logger::fatalErrorf("Unknown type for instance: '%s'", node.name()); panic(); } - const Data::TypeInfo* type = Data::TYPE_MAP[node->name()]; + const Data::TypeInfo* type = Data::TYPE_MAP[node.name()]; return type->deserializer(node); } diff --git a/core/src/datatypes/meta.h b/core/src/datatypes/meta.h index 912f0e6..68affde 100644 --- a/core/src/datatypes/meta.h +++ b/core/src/datatypes/meta.h @@ -34,8 +34,8 @@ namespace Data { template T get() { return std::get(wrapped); } Data::String ToString() const; - void Serialize(pugi::xml_node* node) const; - static Data::Variant Deserialize(pugi::xml_node* node); + void Serialize(pugi::xml_node node) const; + static Data::Variant Deserialize(pugi::xml_node node); }; // Map of all data types to their type names diff --git a/core/src/datatypes/vector.cpp b/core/src/datatypes/vector.cpp index 168bcd2..cf10504 100644 --- a/core/src/datatypes/vector.cpp +++ b/core/src/datatypes/vector.cpp @@ -65,12 +65,12 @@ float Data::Vector3::Dot(Data::Vector3 other) const { // Serialization -void Data::Vector3::Serialize(pugi::xml_node* node) const { - node->append_child("X").text().set(std::to_string(this->X())); - node->append_child("Y").text().set(std::to_string(this->Y())); - node->append_child("Z").text().set(std::to_string(this->Z())); +void Data::Vector3::Serialize(pugi::xml_node node) const { + node.append_child("X").text().set(std::to_string(this->X())); + node.append_child("Y").text().set(std::to_string(this->Y())); + node.append_child("Z").text().set(std::to_string(this->Z())); } -Data::Variant Data::Vector3::Deserialize(pugi::xml_node* node) { - return Data::Vector3(node->child("X").text().as_float(), node->child("Y").text().as_float(), node->child("Z").text().as_float()); +Data::Variant Data::Vector3::Deserialize(pugi::xml_node node) { + return Data::Vector3(node.child("X").text().as_float(), node.child("Y").text().as_float(), node.child("Z").text().as_float()); } diff --git a/core/src/datatypes/vector.h b/core/src/datatypes/vector.h index 1a21724..23b79c9 100644 --- a/core/src/datatypes/vector.h +++ b/core/src/datatypes/vector.h @@ -24,8 +24,8 @@ namespace Data { static Data::Vector3 ONE; virtual const Data::String ToString() const override; - virtual void Serialize(pugi::xml_node* node) const override; - static Data::Variant Deserialize(pugi::xml_node* node); + virtual void Serialize(pugi::xml_node node) const override; + static Data::Variant Deserialize(pugi::xml_node node); operator glm::vec3() const; operator rp::Vector3() const; diff --git a/core/src/objects/base/instance.cpp b/core/src/objects/base/instance.cpp index 655d540..df768d2 100644 --- a/core/src/objects/base/instance.cpp +++ b/core/src/objects/base/instance.cpp @@ -186,8 +186,8 @@ std::vector Instance::GetProperties() { // Serialization -void Instance::Serialize(pugi::xml_node* parent) { - pugi::xml_node node = parent->append_child("Item"); +void Instance::Serialize(pugi::xml_node parent) { + pugi::xml_node node = parent.append_child("Item"); node.append_attribute("class").set_value(this->GetClass()->className); // Add properties @@ -198,17 +198,17 @@ void Instance::Serialize(pugi::xml_node* parent) { pugi::xml_node propertyNode = propertiesNode.append_child(meta.type->name); propertyNode.append_attribute("name").set_value(name); - GetPropertyValue(name)->Serialize(&propertyNode); + GetPropertyValue(name)->Serialize(propertyNode); } // Add children for (InstanceRef child : this->children) { - child->Serialize(&node); + child->Serialize(node); } } -InstanceRef Instance::Deserialize(pugi::xml_node* node) { - std::string className = node->attribute("class").value(); +InstanceRef Instance::Deserialize(pugi::xml_node node) { + std::string className = node.attribute("class").value(); if (INSTANCE_MAP.count(className) == 0) { Logger::fatalErrorf("Unknown type for instance: '%s'", className.c_str()); panic(); @@ -221,7 +221,7 @@ InstanceRef Instance::Deserialize(pugi::xml_node* node) { // const InstanceType* type = INSTANCE_MAP.at(className); // Read properties - pugi::xml_node propertiesNode = node->child("Properties"); + pugi::xml_node propertiesNode = node.child("Properties"); for (pugi::xml_node propertyNode : propertiesNode) { std::string propertyName = propertyNode.attribute("name").value(); auto meta_ = object->GetPropertyMeta(propertyName); @@ -229,13 +229,13 @@ InstanceRef Instance::Deserialize(pugi::xml_node* node) { Logger::fatalErrorf("Attempt to set unknown property '%s' of %s", propertyName.c_str(), object->GetClass()->className.c_str()); continue; } - Data::Variant value = Data::Variant::Deserialize(&propertyNode); + Data::Variant value = Data::Variant::Deserialize(propertyNode); object->SetPropertyValue(propertyName, value); } // Read children - for (pugi::xml_node childNode : node->children("Item")) { - InstanceRef child = Instance::Deserialize(&childNode); + for (pugi::xml_node childNode : node.children("Item")) { + InstanceRef child = Instance::Deserialize(childNode); object->AddChild(child); } diff --git a/core/src/objects/base/instance.h b/core/src/objects/base/instance.h index 8df267b..9828f40 100644 --- a/core/src/objects/base/instance.h +++ b/core/src/objects/base/instance.h @@ -94,8 +94,8 @@ public: std::vector GetProperties(); // Serialization - void Serialize(pugi::xml_node* parent); - static std::shared_ptr Deserialize(pugi::xml_node* node); + void Serialize(pugi::xml_node parent); + static std::shared_ptr Deserialize(pugi::xml_node node); }; typedef std::shared_ptr InstanceRef; diff --git a/core/src/objects/datamodel.cpp b/core/src/objects/datamodel.cpp index a110e96..f6a008c 100644 --- a/core/src/objects/datamodel.cpp +++ b/core/src/objects/datamodel.cpp @@ -53,7 +53,7 @@ void DataModel::SaveToFile(std::optional path) { pugi::xml_node root = doc.append_child("openblocks"); for (InstanceRef child : this->GetChildren()) { - child->Serialize(&root); + child->Serialize(root); } doc.save(outStream); @@ -62,8 +62,8 @@ void DataModel::SaveToFile(std::optional path) { Logger::info("Place saved successfully"); } -void DataModel::DeserializeService(pugi::xml_node* node) { - std::string className = node->attribute("class").value(); +void DataModel::DeserializeService(pugi::xml_node node) { + std::string className = node.attribute("class").value(); if (INSTANCE_MAP.count(className) == 0) { Logger::fatalErrorf("Unknown service: '%s'", className.c_str()); panic(); @@ -79,7 +79,7 @@ void DataModel::DeserializeService(pugi::xml_node* node) { AddChild(object); // Read properties - pugi::xml_node propertiesNode = node->child("Properties"); + pugi::xml_node propertiesNode = node.child("Properties"); for (pugi::xml_node propertyNode : propertiesNode) { std::string propertyName = propertyNode.attribute("name").value(); auto meta_ = object->GetPropertyMeta(propertyName); @@ -87,13 +87,13 @@ void DataModel::DeserializeService(pugi::xml_node* node) { Logger::fatalErrorf("Attempt to set unknown property '%s' of %s", propertyName.c_str(), object->GetClass()->className.c_str()); continue; } - Data::Variant value = Data::Variant::Deserialize(&propertyNode); + Data::Variant value = Data::Variant::Deserialize(propertyNode); object->SetPropertyValue(propertyName, value); } // Add children - for (pugi::xml_node childNode : node->children("Item")) { - InstanceRef child = Instance::Deserialize(&childNode); + for (pugi::xml_node childNode : node.children("Item")) { + InstanceRef child = Instance::Deserialize(childNode); object->AddChild(child); } @@ -111,7 +111,7 @@ std::shared_ptr DataModel::LoadFromFile(std::string path) { std::shared_ptr newModel = std::make_shared(); for (pugi::xml_node childNode : rootNode.children("Item")) { - newModel->DeserializeService(&childNode); + newModel->DeserializeService(childNode); } newModel->Init(); diff --git a/core/src/objects/datamodel.h b/core/src/objects/datamodel.h index 49cf237..ec9832e 100644 --- a/core/src/objects/datamodel.h +++ b/core/src/objects/datamodel.h @@ -15,7 +15,7 @@ class Service; // The root instance to all objects in the hierarchy class DataModel : public Instance { private: - void DeserializeService(pugi::xml_node* node); + void DeserializeService(pugi::xml_node node); public: const static InstanceType TYPE; diff --git a/editor/mainwindow.cpp b/editor/mainwindow.cpp index 02b7955..b3e259c 100644 --- a/editor/mainwindow.cpp +++ b/editor/mainwindow.cpp @@ -248,7 +248,7 @@ MainWindow::MainWindow(QWidget *parent) pugi::xml_document rootDoc; for (InstanceRefWeak inst : getSelection()) { if (inst.expired()) continue; - inst.lock()->Serialize(&rootDoc); + inst.lock()->Serialize(rootDoc); } std::ostringstream encoded; @@ -262,7 +262,7 @@ MainWindow::MainWindow(QWidget *parent) pugi::xml_document rootDoc; for (InstanceRefWeak inst : getSelection()) { if (inst.expired()) continue; - inst.lock()->Serialize(&rootDoc); + inst.lock()->Serialize(rootDoc); inst.lock()->SetParent(std::nullopt); } @@ -284,7 +284,7 @@ MainWindow::MainWindow(QWidget *parent) rootDoc.load_string(encoded.c_str()); for (pugi::xml_node instNode : rootDoc.children()) { - InstanceRef inst = Instance::Deserialize(&instNode); + InstanceRef inst = Instance::Deserialize(instNode); gWorkspace()->AddChild(inst); } }); @@ -303,7 +303,7 @@ MainWindow::MainWindow(QWidget *parent) rootDoc.load_string(encoded.c_str()); for (pugi::xml_node instNode : rootDoc.children()) { - InstanceRef inst = Instance::Deserialize(&instNode); + InstanceRef inst = Instance::Deserialize(instNode); selectedParent->AddChild(inst); } }); @@ -319,7 +319,7 @@ MainWindow::MainWindow(QWidget *parent) for (InstanceRefWeak inst : getSelection()) { if (inst.expired()) continue; - inst.lock()->Serialize(&modelRoot); + inst.lock()->Serialize(modelRoot); } modelDoc.save(outStream); @@ -337,7 +337,7 @@ MainWindow::MainWindow(QWidget *parent) modelDoc.load(inStream); for (pugi::xml_node instNode : modelDoc.child("openblocks").children("Item")) { - InstanceRef inst = Instance::Deserialize(&instNode); + InstanceRef inst = Instance::Deserialize(instNode); selectedParent->AddChild(inst); } });