Further improve pyImageLibMod interface.

Includes fixes suggested by code review.
H-uru · Jul 18, 2023 · e26b575 · e26b575
1 parent a11dc5a
commit e26b575
Show file tree

Hide file tree

Showing 6 changed files with 25 additions and 30 deletions.
diff --git 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,21 +67,27 @@ pyImage* pyImageLibMod::GetImage(const ST::string& name) const
     else
         image = plImageLibMod::ConvertNoRef(fModifierKey->ObjectIsLoaded())->GetImage(name);
 
-    return pyImage::ConvertFrom(pyImage::New(dynamic_cast<plMipmap*>(image)));
+    if (image)
+        return pyImage::New(plMipmap::ConvertNoRef(image));
+
+    PYTHON_RETURN_NONE;
 }
 
-const std::vector<pyImage*> pyImageLibMod::GetImages() const
+const std::vector<PyObject*> pyImageLibMod::GetImages() const
 {
-    std::vector<pyImage*> imageList;
+    std::vector<PyObject*> imageList;
     plImageLibMod* mod;
 
     if (fModifier)
         mod = fModifier;
     else
         mod = plImageLibMod::ConvertNoRef(fModifierKey->ObjectIsLoaded());
 
-    for (const auto& image : mod->GetImages())
-        imageList.push_back(pyImage::ConvertFrom(pyImage::New(dynamic_cast<plMipmap*>(image))));
+    for (const auto& image : mod->GetImages()) {
+        if (image)
+            imageList.push_back(pyImage::New(plMipmap::ConvertNoRef(image)));
+    }
+
     return imageList;
 }
 

diff --git a/Sources/Plasma/FeatureLib/pfPython/pyImageLibMod.h b/Sources/Plasma/FeatureLib/pfPython/pyImageLibMod.h
@@ -116,9 +116,9 @@ class pyImageLibMod
     plKey GetKey() const { return fModifier ? fModifier->GetKey() : fModifierKey; }
 
     // for python access
-    pyImage* GetImage (const ST::string& name) const;
+    PyObject* GetImage (const ST::string& name) const;
     const std::vector<ST::string> GetImageNames() const;
-    const std::vector<pyImage*> GetImages() const;
+    const std::vector<PyObject*> GetImages() const;
 };
 
 #endif  // pyImageLibMod_h
diff --git 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<pyImage*> imageList = self->fThis->GetImages();
+    const std::vector<PyObject*> 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<ST::string> 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
@@ -186,7 +186,7 @@ PYTHON_METHOD_DEFINITION_NOARGS(ptSceneobject, getImageLibMods)
     std::vector<PyObject*> 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
@@ -99,18 +99,12 @@ plBitmap* plImageLibMod::GetImage(const ST::string& imageName) const
     return nullptr;
 }
 
-const std::vector<plBitmap*> plImageLibMod::GetImages() const
-{
-    std::vector<plBitmap*> images;
-    for (auto image : fImages)
-        images.emplace_back(image);
-    return images;
-}
-
-const std::vector<ST::string> plImageLibMod::GetImageNames() const
+std::vector<ST::string> plImageLibMod::GetImageNames() const
 {
     std::vector<ST::string> 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
@@ -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<plBitmap*> GetImages() const;
-    const std::vector<ST::string> GetImageNames() const;
+    std::vector<plBitmap*> GetImages() const { return fImages; }
+    std::vector<ST::string> GetImageNames() const;
 };
 
 #endif // plImageLibMod_inc