From 13117a7f70483f5c495610bbfb6335d2b0cfaf7d Mon Sep 17 00:00:00 2001 From: Joseph Davies Date: Tue, 18 Jul 2023 14:27:07 -0700 Subject: [PATCH] Further improve pyImageLibMod interface. Includes fixes suggested by code review. --- .../FeatureLib/pfPython/pyImageLibMod.cpp | 26 +++++++++++-------- .../FeatureLib/pfPython/pyImageLibMod.h | 8 +++--- .../FeatureLib/pfPython/pyImageLibModGlue.cpp | 12 +++------ .../FeatureLib/pfPython/pySceneObjectGlue.cpp | 2 +- .../PubUtilLib/plModifier/plImageLibMod.cpp | 14 +++------- .../PubUtilLib/plModifier/plImageLibMod.h | 5 ++-- 6 files changed, 30 insertions(+), 37 deletions(-) diff --git a/Sources/Plasma/FeatureLib/pfPython/pyImageLibMod.cpp b/Sources/Plasma/FeatureLib/pfPython/pyImageLibMod.cpp index 3c0746f429..4d1c7ae2f5 100644 --- a/Sources/Plasma/FeatureLib/pfPython/pyImageLibMod.cpp +++ b/Sources/Plasma/FeatureLib/pfPython/pyImageLibMod.cpp @@ -58,7 +58,7 @@ void pyImageLibMod::setKey(pyKey& ilmKey) // only for python glue, do NOT call fModifierKey = ilmKey.getKey(); } -pyImage* pyImageLibMod::GetImage(const ST::string& name) const +PyObject* pyImageLibMod::GetImage(const ST::string& name) const { plBitmap* image; @@ -67,12 +67,15 @@ pyImage* pyImageLibMod::GetImage(const ST::string& name) const else image = plImageLibMod::ConvertNoRef(fModifierKey->ObjectIsLoaded())->GetImage(name); - return pyImage::ConvertFrom(pyImage::New(dynamic_cast(image))); + if (image) + return pyImage::New(plMipmap::ConvertNoRef(image)); + + PYTHON_RETURN_NONE; } -const std::vector pyImageLibMod::GetImages() const +std::vector pyImageLibMod::GetImages() const { - std::vector imageList; + std::vector imageList; plImageLibMod* mod; if (fModifier) @@ -80,14 +83,17 @@ const std::vector pyImageLibMod::GetImages() const else mod = plImageLibMod::ConvertNoRef(fModifierKey->ObjectIsLoaded()); - for (const auto& image : mod->GetImages()) - imageList.push_back(pyImage::ConvertFrom(pyImage::New(dynamic_cast(image)))); + imageList.reserve(mod->GetImages().size()); + for (const auto& image : mod->GetImages()) { + if (image) + imageList.push_back(pyImage::New(plMipmap::ConvertNoRef(image))); + } + return imageList; } -const std::vector pyImageLibMod::GetImageNames() const +std::vector pyImageLibMod::GetImageNames() const { - std::vector nameList; plImageLibMod* mod; if (fModifier) @@ -95,7 +101,5 @@ const std::vector pyImageLibMod::GetImageNames() const else mod = plImageLibMod::ConvertNoRef(fModifierKey->ObjectIsLoaded()); - for (const auto& name : mod->GetImageNames()) - nameList.push_back(name); - return nameList; + return mod->GetImageNames(); } diff --git a/Sources/Plasma/FeatureLib/pfPython/pyImageLibMod.h b/Sources/Plasma/FeatureLib/pfPython/pyImageLibMod.h index 71f0d1dcda..4eaacdaea0 100644 --- a/Sources/Plasma/FeatureLib/pfPython/pyImageLibMod.h +++ b/Sources/Plasma/FeatureLib/pfPython/pyImageLibMod.h @@ -52,9 +52,9 @@ You can contact Cyan Worlds, Inc. by email legal@cyan.com #include "pyGlueHelpers.h" -#include "pnKeyedObject/plKey.h" #include "plModifier/plImageLibMod.h" +class plKey; class pyImage; class pyImageLibMod @@ -116,9 +116,9 @@ class pyImageLibMod plKey GetKey() const { return fModifier ? fModifier->GetKey() : fModifierKey; } // for python access - pyImage* GetImage (const ST::string& name) const; - const std::vector GetImageNames() const; - const std::vector GetImages() const; + PyObject* GetImage (const ST::string& name) const; + std::vector GetImageNames() const; + std::vector GetImages() const; }; #endif // pyImageLibMod_h diff --git a/Sources/Plasma/FeatureLib/pfPython/pyImageLibModGlue.cpp b/Sources/Plasma/FeatureLib/pfPython/pyImageLibModGlue.cpp index 99b0813646..cf14b0f674 100644 --- a/Sources/Plasma/FeatureLib/pfPython/pyImageLibModGlue.cpp +++ b/Sources/Plasma/FeatureLib/pfPython/pyImageLibModGlue.cpp @@ -80,19 +80,15 @@ PYTHON_METHOD_DEFINITION(ptImageLibMod, getImage, args) PYTHON_RETURN_ERROR; } - pyImage* image = self->fThis->GetImage(name); - if (image) - return pyImage::New(image->GetKey()); - - PYTHON_RETURN_NONE; + return self->fThis->GetImage(name); } PYTHON_METHOD_DEFINITION_NOARGS(ptImageLibMod, getImages) { - const std::vector imageList = self->fThis->GetImages(); + const std::vector imageList = self->fThis->GetImages(); PyObject* retVal = PyTuple_New(imageList.size()); for (size_t curKey = 0; curKey < imageList.size(); curKey++) - PyTuple_SET_ITEM(retVal, curKey, pyImage::New(imageList[curKey]->GetKey())); + PyTuple_SET_ITEM(retVal, curKey, imageList[curKey]); return retVal; } @@ -101,7 +97,7 @@ PYTHON_METHOD_DEFINITION_NOARGS(ptImageLibMod, getNames) std::vector nameList = self->fThis->GetImageNames(); PyObject* retVal = PyTuple_New(nameList.size()); for (size_t curKey = 0; curKey < nameList.size(); curKey++) - PyTuple_SET_ITEM(retVal, curKey, PyUnicode_FromSTString(nameList[curKey])); // steals the nameList ref + PyTuple_SET_ITEM(retVal, curKey, PyUnicode_FromSTString(nameList[curKey])); return retVal; } diff --git a/Sources/Plasma/FeatureLib/pfPython/pySceneObjectGlue.cpp b/Sources/Plasma/FeatureLib/pfPython/pySceneObjectGlue.cpp index 82c58b2d42..fdaacdce0e 100644 --- a/Sources/Plasma/FeatureLib/pfPython/pySceneObjectGlue.cpp +++ b/Sources/Plasma/FeatureLib/pfPython/pySceneObjectGlue.cpp @@ -185,7 +185,7 @@ PYTHON_METHOD_DEFINITION_NOARGS(ptSceneobject, getImageLibMods) std::vector vecList = self->fThis->GetImageLibMods(); PyObject* retVal = PyTuple_New(vecList.size()); for (int curKey = 0; curKey < vecList.size(); curKey++) - PyTuple_SetItem(retVal, curKey, vecList[curKey]); + PyTuple_SET_ITEM(retVal, curKey, vecList[curKey]); return retVal; } diff --git a/Sources/Plasma/PubUtilLib/plModifier/plImageLibMod.cpp b/Sources/Plasma/PubUtilLib/plModifier/plImageLibMod.cpp index 0b71ea08da..f6abe84857 100644 --- a/Sources/Plasma/PubUtilLib/plModifier/plImageLibMod.cpp +++ b/Sources/Plasma/PubUtilLib/plModifier/plImageLibMod.cpp @@ -99,18 +99,12 @@ plBitmap* plImageLibMod::GetImage(const ST::string& imageName) const return nullptr; } -const std::vector plImageLibMod::GetImages() const -{ - std::vector images; - for (auto image : fImages) - images.emplace_back(image); - return images; -} - -const std::vector plImageLibMod::GetImageNames() const +std::vector plImageLibMod::GetImageNames() const { std::vector names; - for (auto image : fImages) { + names.reserve(fImages.size()); + + for (const auto& image : fImages) { if (image) names.emplace_back(image->GetKeyName()); } diff --git a/Sources/Plasma/PubUtilLib/plModifier/plImageLibMod.h b/Sources/Plasma/PubUtilLib/plModifier/plImageLibMod.h index eaede4c586..305c6a6b0b 100644 --- a/Sources/Plasma/PubUtilLib/plModifier/plImageLibMod.h +++ b/Sources/Plasma/PubUtilLib/plModifier/plImageLibMod.h @@ -59,7 +59,6 @@ class plImageLibMod : public plSingleModifier public: plImageLibMod() {}; - virtual ~plImageLibMod() {}; CLASSNAME_REGISTER( plImageLibMod ); GETINTERFACE_ANY( plImageLibMod, plSingleModifier ); @@ -76,8 +75,8 @@ class plImageLibMod : public plSingleModifier size_t GetNumImages() const { return fImages.size(); } plBitmap* GetImage(const ST::string&) const; - const std::vector GetImages() const; - const std::vector GetImageNames() const; + std::vector GetImages() const { return fImages; } + std::vector GetImageNames() const; }; #endif // plImageLibMod_inc