From 5bbb07e24dbeece6f3962345644f8cbe6afd72e5 Mon Sep 17 00:00:00 2001 From: David Carter Date: Thu, 10 Apr 2025 06:13:38 -0400 Subject: [PATCH] Materials: Use data classes in interface specification --- src/Mod/Material/App/ExternalManager.cpp | 134 +++++++++--------- src/Mod/Material/App/ExternalManager.h | 8 +- .../MaterialAPI/MaterialManagerExternal.py | 37 ++--- 3 files changed, 92 insertions(+), 87 deletions(-) diff --git a/src/Mod/Material/App/ExternalManager.cpp b/src/Mod/Material/App/ExternalManager.cpp index 249e049234..22d3920e11 100644 --- a/src/Mod/Material/App/ExternalManager.cpp +++ b/src/Mod/Material/App/ExternalManager.cpp @@ -65,9 +65,8 @@ ExternalManager::~ExternalManager() _hGrp->Detach(this); } -void ExternalManager::OnChange(ParameterGrp::SubjectType& rCaller, ParameterGrp::MessageType Reason) +void ExternalManager::OnChange(ParameterGrp::SubjectType& /*rCaller*/, ParameterGrp::MessageType Reason) { - const ParameterGrp& rGrp = static_cast(rCaller); if (std::strncmp(Reason, "Current", 7) == 0) { if (_instantiated) { // The old manager object will be deleted when reconnecting @@ -167,36 +166,74 @@ ExternalManager* ExternalManager::getManager() // //===== -std::shared_ptr -ExternalManager::libraryFromTuple(const Py::Tuple& entry) +bool ExternalManager::checkMaterialLibraryType(const Py::Object& entry) { - auto pyName = entry.getItem(0); + return entry.hasAttr("name") && entry.hasAttr("icon") && entry.hasAttr("readOnly") + && entry.hasAttr("timestamp"); +} + +std::shared_ptr +ExternalManager::libraryFromObject(const Py::Object& entry) +{ + if (!checkMaterialLibraryType(entry)) { + throw InvalidLibrary(); + } + + Py::String pyName(entry.getAttr("name")); + Py::Bytes pyIcon(entry.getAttr("icon")); + Py::Boolean pyReadOnly(entry.getAttr("readOnly")); + Py::String pyTimestamp(entry.getAttr("timestamp")); + QString libraryName; if (!pyName.isNone()) { libraryName = QString::fromStdString(pyName.as_string()); } - auto pyIcon = entry.getItem(1); + QString icon; if (!pyIcon.isNone()) { icon = QString::fromStdString(pyIcon.as_string()); } - auto pyReadOnly = entry.getItem(2); + bool readOnly = pyReadOnly.as_bool(); - auto pyTimestamp = entry.getItem(3); + QString timestamp; if (!pyTimestamp.isNone()) { timestamp = QString::fromStdString(pyTimestamp.as_string()); } - Base::Console().Log("Library name '%s', Icon '%s', readOnly %s, timestamp '%s'\n", - libraryName.toStdString().c_str(), - icon.toStdString().c_str(), - readOnly ? "true" : "false", - timestamp.toStdString().c_str()); auto library = std::make_shared(libraryName, icon, readOnly, timestamp); return library; } +bool ExternalManager::checkMaterialObjectType(const Py::Object& entry) +{ + return entry.hasAttr("UUID") && entry.hasAttr("path") && entry.hasAttr("name"); +} + +std::tuple +ExternalManager::materialObjectTypeFromObject(const Py::Object& entry) +{ + QString uuid; + auto pyUUID = entry.getAttr("UUID"); + if (!pyUUID.isNone()) { + uuid = QString::fromStdString(pyUUID.as_string()); + } + + QString path; + auto pyPath = entry.getAttr("path"); + if (!pyPath.isNone()) { + path = QString::fromStdString(pyPath.as_string()); + } + + QString name; + auto pyName = entry.getAttr("name"); + if (!pyName.isNone()) { + name = QString::fromStdString(pyName.as_string()); + } + + return std::tuple(uuid, path, name); +} + std::shared_ptr>> ExternalManager::libraries() { @@ -210,7 +247,7 @@ ExternalManager::libraries() Py::Callable libraries(_managerObject.getAttr("libraries")); Py::List list(libraries.apply()); for (auto lib : list) { - auto library = libraryFromTuple(Py::Tuple(lib)); + auto library = libraryFromObject(Py::Object(lib)); libList->push_back(library); } } @@ -239,7 +276,7 @@ std::shared_ptr>> ExternalManager::modelLib Py::Callable libraries(_managerObject.getAttr("modelLibraries")); Py::List list(libraries.apply()); for (auto lib : list) { - auto library = libraryFromTuple(Py::Tuple(lib)); + auto library = libraryFromObject(Py::Tuple(lib)); libList->push_back(library); } } @@ -268,7 +305,7 @@ std::shared_ptr>> ExternalManager::material Py::Callable libraries(_managerObject.getAttr("materialLibraries")); Py::List list(libraries.apply()); for (auto lib : list) { - auto library = libraryFromTuple(Py::Tuple(lib)); + auto library = libraryFromObject(Py::Tuple(lib)); libList->push_back(library); } } @@ -299,7 +336,7 @@ std::shared_ptr ExternalManager::getLibrary(const QString& name) Py::Tuple result(libraries.apply(args)); Py::Object libObject = result.getItem(0); - auto lib = libraryFromTuple(Py::Tuple(libObject)); + auto lib = libraryFromObject(Py::Tuple(libObject)); return std::make_shared(*lib); } else { @@ -424,25 +461,12 @@ ExternalManager::libraryModels(const QString& libraryName) args.setItem(0, Py::String(libraryName.toStdString())); Py::List list(libraries.apply(args)); for (auto library : list) { - auto entry = Py::Tuple(library); - - auto pyUUID = entry.getItem(0); - QString uuid; - if (!pyUUID.isNone()) { - uuid = QString::fromStdString(pyUUID.as_string()); - } - auto pyPath = entry.getItem(1); - QString path; - if (!pyPath.isNone()) { - path = QString::fromStdString(pyPath.as_string()); - } - auto pyName = entry.getItem(2); - QString name; - if (!pyName.isNone()) { - name = QString::fromStdString(pyName.as_string()); + auto entry = Py::Object(library); + if (!checkMaterialObjectType(entry)) { + throw InvalidModel(); } - modelList->push_back(std::tuple(uuid, path, name)); + modelList->push_back(materialObjectTypeFromObject(entry)); } } else { @@ -473,25 +497,12 @@ ExternalManager::libraryMaterials(const QString& libraryName) args.setItem(0, Py::String(libraryName.toStdString())); Py::List list(libraries.apply(args)); for (auto library : list) { - auto entry = Py::Tuple(library); - - auto pyUUID = entry.getItem(0); - QString uuid; - if (!pyUUID.isNone()) { - uuid = QString::fromStdString(pyUUID.as_string()); - } - auto pyPath = entry.getItem(1); - QString path; - if (!pyPath.isNone()) { - path = QString::fromStdString(pyPath.as_string()); - } - auto pyName = entry.getItem(2); - QString name; - if (!pyName.isNone()) { - name = QString::fromStdString(pyName.as_string()); + auto entry = Py::Object(library); + if (!checkMaterialObjectType(entry)) { + throw InvalidMaterial(); } - materialList->push_back(std::tuple(uuid, path, name)); + materialList->push_back(materialObjectTypeFromObject(entry)); } } else { @@ -534,25 +545,12 @@ ExternalManager::libraryMaterials(const QString& libraryName, Py::Object(new MaterialFilterOptionsPy(new MaterialFilterOptions(options)), true)); Py::List list(libraries.apply(args)); for (auto library : list) { - auto entry = Py::Tuple(library); - - auto pyUUID = entry.getItem(0); - QString uuid; - if (!pyUUID.isNone()) { - uuid = QString::fromStdString(pyUUID.as_string()); - } - auto pyPath = entry.getItem(1); - QString path; - if (!pyPath.isNone()) { - path = QString::fromStdString(pyPath.as_string()); - } - auto pyName = entry.getItem(2); - QString name; - if (!pyName.isNone()) { - name = QString::fromStdString(pyName.as_string()); + auto entry = Py::Object(library); + if (!checkMaterialObjectType(entry)) { + throw InvalidMaterial(); } - materialList->push_back(std::tuple(uuid, path, name)); + materialList->push_back(materialObjectTypeFromObject(entry)); } } else { diff --git a/src/Mod/Material/App/ExternalManager.h b/src/Mod/Material/App/ExternalManager.h index 56c4ae2a62..91f92b49ca 100644 --- a/src/Mod/Material/App/ExternalManager.h +++ b/src/Mod/Material/App/ExternalManager.h @@ -28,6 +28,7 @@ #include class QMutex; +class QString; namespace Materials { @@ -83,13 +84,16 @@ public: private: ExternalManager(); - ~ExternalManager(); + ~ExternalManager() override; static void initManager(); void getConfiguration(); void instantiate(); void connect(); - std::shared_ptr libraryFromTuple(const Py::Tuple& entry); + bool checkMaterialLibraryType(const Py::Object& entry); + std::shared_ptr libraryFromObject(const Py::Object& entry); + bool checkMaterialObjectType(const Py::Object& entry); + std::tuple materialObjectTypeFromObject(const Py::Object& entry); static ExternalManager* _manager; static QMutex _mutex; diff --git a/src/Mod/Material/App/MaterialAPI/MaterialManagerExternal.py b/src/Mod/Material/App/MaterialAPI/MaterialManagerExternal.py index 2f9db88722..152e4b3525 100644 --- a/src/Mod/Material/App/MaterialAPI/MaterialManagerExternal.py +++ b/src/Mod/Material/App/MaterialAPI/MaterialManagerExternal.py @@ -23,19 +23,22 @@ __author__ = "David Carter" __url__ = "https://www.davesrocketshop.com" from abc import ABC, abstractmethod -# from typing import Tuple +from dataclasses import dataclass import Materials -# This requires Python 3.12 or later. This isn't available on all platforms yet -# type MaterialName = str -# type MaterialIcon = str -# type MaterialReadOnly = bool -# type MaterialTimestamp = str -# type MaterialUUID = str -# type MaterialPath = str -# type MaterialLibraryType = Tuple[MaterialName, MaterialIcon, MaterialReadOnly, MaterialTimestamp] -# type MaterialLibraryObjectType = Tuple[MaterialUUID, MaterialPath, MaterialName] +@dataclass +class MaterialLibraryType: + name: str + icon: bytes + readOnly: bool + timestamp: str + +@dataclass +class MaterialLibraryObjectType: + UUID: str + path: str + name: str class MaterialManagerExternal(ABC): """Abstract base class for all external material managers @@ -55,7 +58,7 @@ class MaterialManagerExternal(ABC): # @abstractmethod - def libraries(self) -> list: #[MaterialLibraryType]: + def libraries(self) -> list[MaterialLibraryType]: """Returns a list of libraries managed by this interface The list contains a series of tuples describing all libraries managed by @@ -65,7 +68,7 @@ class MaterialManagerExternal(ABC): pass @abstractmethod - def modelLibraries(self) -> list: #[MaterialLibraryType]: + def modelLibraries(self) -> list[MaterialLibraryType]: """Returns a list of libraries managed by this interface The list contains a series of tuples describing all libraries managed by @@ -78,7 +81,7 @@ class MaterialManagerExternal(ABC): pass @abstractmethod - def materialLibraries(self) -> list: #[MaterialLibraryType]: + def materialLibraries(self) -> list[MaterialLibraryType]: """Returns a list of libraries managed by this interface The list contains a series of tuples describing all libraries managed by @@ -98,7 +101,7 @@ class MaterialManagerExternal(ABC): pass @abstractmethod - def createLibrary(self, name: str, icon: str, readOnly: bool) -> None: + def createLibrary(self, name: str, icon: bytes, readOnly: bool) -> None: """Create a new library Create a new library with the given name""" @@ -112,7 +115,7 @@ class MaterialManagerExternal(ABC): pass @abstractmethod - def changeIcon(self, name: str, icon: str) -> None: + def changeIcon(self, name: str, icon: bytes) -> None: """Change the library icon Change the library icon""" @@ -126,7 +129,7 @@ class MaterialManagerExternal(ABC): pass @abstractmethod - def libraryModels(self, library: str) -> list: #[MaterialLibraryObjectType]: + def libraryModels(self, library: str) -> list[MaterialLibraryObjectType]: """Returns a list of models managed by this library Each list entry is a tuple containing the UUID, path, and name of the model""" @@ -135,7 +138,7 @@ class MaterialManagerExternal(ABC): @abstractmethod def libraryMaterials(self, library: str, filter: Materials.MaterialFilter = None, - options: Materials.MaterialFilterOptions = None) -> list: #[MaterialLibraryObjectType]: + options: Materials.MaterialFilterOptions = None) -> list[MaterialLibraryObjectType]: """Returns a list of materials managed by this library Each list entry is a tuple containing the UUID, path, and name of the material"""