[CCSpriteCache] allow re-add plist & add testcase

cocos/2d/CCSpriteFrameCache.cpp

@@ -293,6 +293,7 @@ void SpriteFrameCache::addSpriteFramesWithDictionary(ValueMap& dictionary, Textu
         // add sprite frame
         _spriteFramesCache.insertFrame(plist, spriteFrameName, spriteFrame);
     }
+    _spriteFramesCache.markPlistFull(plist, true);
     CC_SAFE_DELETE(image);
 }
 
@@ -350,11 +351,6 @@ void SpriteFrameCache::addSpriteFramesWithDictionary(ValueMap& dict, const std::
 
 void SpriteFrameCache::addSpriteFramesWithFile(const std::string& plist, Texture2D *texture)
 {
-    if (_spriteFramesCache.hasPlist(plist))
-    {
-        return; // We already added it
-    }
-
     std::string fullPath = FileUtils::getInstance()->fullPathForFilename(plist);
     ValueMap dict = FileUtils::getInstance()->getValueMapFromFile(fullPath);
 
@@ -370,10 +366,6 @@ void SpriteFrameCache::addSpriteFramesWithFileContent(const std::string& plist_c
 void SpriteFrameCache::addSpriteFramesWithFile(const std::string& plist, const std::string& textureFileName)
 {
     CCASSERT(textureFileName.size()>0, "texture name should not be null");
-    if (_spriteFramesCache.hasPlist(plist))
-    {
-        return; // We already added it
-    }
     const std::string fullPath = FileUtils::getInstance()->fullPathForFilename(plist);
     ValueMap dict = FileUtils::getInstance()->getValueMapFromFile(fullPath);
     addSpriteFramesWithDictionary(dict, textureFileName, plist);
@@ -391,45 +383,42 @@ void SpriteFrameCache::addSpriteFramesWithFile(const std::string& plist)
         return;
     }
 
-    if (!_spriteFramesCache.hasPlist(plist))
-    {
-        ValueMap dict = FileUtils::getInstance()->getValueMapFromFile(fullPath);
+    ValueMap dict = FileUtils::getInstance()->getValueMapFromFile(fullPath);
 
-        string texturePath("");
+    string texturePath("");
 
-        if (dict.find("metadata") != dict.end())
-        {
-            ValueMap& metadataDict = dict["metadata"].asValueMap();
-            // try to read  texture file name from meta data
-            texturePath = metadataDict["textureFileName"].asString();
-        }
+    if (dict.find("metadata") != dict.end())
+    {
+        ValueMap& metadataDict = dict["metadata"].asValueMap();
+        // try to read  texture file name from meta data
+        texturePath = metadataDict["textureFileName"].asString();
+    }
 
-        if (!texturePath.empty())
-        {
-            // build texture path relative to plist file
-            texturePath = FileUtils::getInstance()->fullPathFromRelativeFile(texturePath, plist);
-        }
-        else
-        {
-            // build texture path by replacing file extension
-            texturePath = plist;
+    if (!texturePath.empty())
+    {
+        // build texture path relative to plist file
+        texturePath = FileUtils::getInstance()->fullPathFromRelativeFile(texturePath, plist);
+    }
+    else
+    {
+        // build texture path by replacing file extension
+        texturePath = plist;
 
-            // remove .xxx
-            size_t startPos = texturePath.find_last_of("."); 
-            texturePath = texturePath.erase(startPos);
+        // remove .xxx
+        size_t startPos = texturePath.find_last_of("."); 
+        texturePath = texturePath.erase(startPos);
 
-            // append .png
-            texturePath = texturePath.append(".png");
+        // append .png
+        texturePath = texturePath.append(".png");
 
-            CCLOG("cocos2d: SpriteFrameCache: Trying to use file %s as texture", texturePath.c_str());
-        }
-        addSpriteFramesWithDictionary(dict, texturePath, plist);
+        CCLOG("cocos2d: SpriteFrameCache: Trying to use file %s as texture", texturePath.c_str());
     }
+    addSpriteFramesWithDictionary(dict, texturePath, plist);
 }
 
 bool SpriteFrameCache::isSpriteFramesWithFileLoaded(const std::string& plist) const
 {
-    return _spriteFramesCache.hasPlist(plist);
+    return _spriteFramesCache.isPlistUsed(plist) && _spriteFramesCache.isPlistFull(plist);
 }
 
 void SpriteFrameCache::addSpriteFrame(SpriteFrame* frame, const std::string& frameName)
@@ -686,7 +675,7 @@ bool SpriteFrameCache::reloadTexture(const std::string& plist)
 {
     CCASSERT(plist.size()>0, "plist filename should not be nullptr");
 
-    if (_spriteFramesCache.hasPlist(plist)) {
+    if (_spriteFramesCache.isPlistUsed(plist)) {
         _spriteFramesCache.erasePlistIndex(plist);
     }
     else
@@ -749,25 +738,21 @@ void SpriteFrameCache::PlistFramesCache::insertFrame(const std::string &plist, c
     _indexFrame2plist[frame] = plist;           //insert index frameName->plist
 }
 
-bool SpriteFrameCache::PlistFramesCache::isPlistUsed(const std::string &plist) const
-{
-    //plist loaded && not empty
-    auto it = _indexPlist2Frames.find(plist);
-    return it != _indexPlist2Frames.end() && !it->second.empty();
-}
-
 bool SpriteFrameCache::PlistFramesCache::eraseFrame(const std::string &frame)
 {
     _spriteFrames.erase(frame);                             //drop SpriteFrame
     auto itFrame = _indexFrame2plist.find(frame);
     if (itFrame != _indexFrame2plist.end())
     {
         auto plist = itFrame->second;
+        markPlistFull(plist, false);
         _indexPlist2Frames[plist].erase(frame);             //update index plist->[frameNames]
         _indexFrame2plist.erase(itFrame);                   //update index frame->plist
         // erase plist index if all frames was erased
         if (_indexFrame2plist.empty())
+        {
             _indexPlist2Frames.erase(plist);
+        }
         return true;
     }
     return false;
@@ -796,6 +781,7 @@ bool SpriteFrameCache::PlistFramesCache::erasePlistIndex(const std::string &plis
         _indexFrame2plist.erase(f);                             //erase plist frame frameName->plist
     }
     _indexPlist2Frames.erase(plist);                            //update index plist->[frameNames]
+    _isPlistFull.erase(plist);                                  //erase full status
     return true;
 }
 
@@ -804,17 +790,19 @@ void SpriteFrameCache::PlistFramesCache::clear()
     _indexPlist2Frames.clear();
     _indexFrame2plist.clear();
     _spriteFrames.clear();
+    _isPlistFull.clear();
 }
 
 bool SpriteFrameCache::PlistFramesCache::hasFrame(const std::string &frame) const
 {
     return _indexFrame2plist.find(frame) != _indexFrame2plist.end();
 }
 
-bool SpriteFrameCache::PlistFramesCache::hasPlist(const std::string &plist) const
+bool SpriteFrameCache::PlistFramesCache::isPlistUsed(const std::string &plist) const
 {
-    return _indexPlist2Frames.find(plist) != _indexPlist2Frames.end();
-}
+    auto frames = _indexPlist2Frames.find(plist);
+    return frames != _indexPlist2Frames.end() && frames->second.size() > 0;
+} 
 
 SpriteFrame * SpriteFrameCache::PlistFramesCache::at(const std::string &frame)
 {

cocos/2d/CCSpriteFrameCache.h

@@ -116,17 +116,22 @@ class CC_DLL SpriteFrameCache : public Ref
         void clear();
 
         inline bool hasFrame(const std::string &frame) const;
-        inline bool hasPlist(const std::string &plist) const;
+        inline bool isPlistUsed(const std::string &plist) const;
 
         inline SpriteFrame *at(const std::string &frame);
         inline Map<std::string, SpriteFrame*>& getSpriteFrames();
 
-        inline bool isPlistUsed(const std::string &plist) const;
-
+        void markPlistFull(const std::string &plist, bool full) { _isPlistFull[plist] = full; }
+        bool isPlistFull(const std::string &plist) const
+        {
+            auto it = _isPlistFull.find(plist);
+            return it == _isPlistFull.end() ? false : it->second;
+        }
     private:
         Map<std::string, SpriteFrame*> _spriteFrames;
         std::unordered_map<std::string, std::set<std::string>> _indexPlist2Frames;
         std::unordered_map<std::string, std::string> _indexFrame2plist;
+        std::unordered_map<std::string, bool> _isPlistFull;
     };
 
 public:

tests/cpp-tests/Classes/SpriteFrameCacheTest/SpriteFrameCacheTest.cpp

@@ -26,6 +26,8 @@
 
 #include "SpriteFrameCacheTest.h"
 
+#include <cassert>
+
 // enable log
 #define COCOS2D_DEBUG 1
 
@@ -34,6 +36,8 @@ USING_NS_CC;
 SpriteFrameCacheTests::SpriteFrameCacheTests()
 {
     ADD_TEST_CASE(SpriteFrameCachePixelFormatTest);
+    ADD_TEST_CASE(SpriteFrameCacheLoadMultipleTimes);
+    ADD_TEST_CASE(SpriteFrameCacheFullCheck);
 }
 
 SpriteFrameCachePixelFormatTest::SpriteFrameCachePixelFormatTest()
@@ -84,4 +88,55 @@ void SpriteFrameCachePixelFormatTest::loadSpriteFrames(const std::string &file,
 
     SpriteFrameCache::getInstance()->removeSpriteFramesFromFile(file);
     Director::getInstance()->getTextureCache()->removeTexture(texture);
-}
+}
+
+
+SpriteFrameCacheLoadMultipleTimes::SpriteFrameCacheLoadMultipleTimes()
+{
+    const Size screenSize = Director::getInstance()->getWinSize();
+
+    // load atlas definition with specified PixelFormat and check that it matches to expected format
+    loadSpriteFrames("Images/sprite_frames_test/test_RGBA8888.plist", Texture2D::PixelFormat::RGBA8888);
+    loadSpriteFrames("Images/sprite_frames_test/test_RGBA8888.plist", Texture2D::PixelFormat::RGBA8888);
+    loadSpriteFrames("Images/sprite_frames_test/test_RGBA8888.plist", Texture2D::PixelFormat::RGBA8888);
+    loadSpriteFrames("Images/sprite_frames_test/test_RGBA8888.plist", Texture2D::PixelFormat::RGBA8888);
+
+}
+
+void SpriteFrameCacheLoadMultipleTimes::loadSpriteFrames(const std::string &file, cocos2d::Texture2D::PixelFormat expectedFormat)
+{
+    SpriteFrameCache::getInstance()->addSpriteFramesWithFile(file);
+    SpriteFrame *spriteFrame = SpriteFrameCache::getInstance()->getSpriteFrameByName("grossini.png");
+    Texture2D *texture = spriteFrame->getTexture();
+    CC_ASSERT(texture->getPixelFormat() == expectedFormat);
+
+    SpriteFrameCache::getInstance()->removeSpriteFrameByName("grossini.png");
+    Director::getInstance()->getTextureCache()->removeTexture(texture);
+}
+
+
+SpriteFrameCacheFullCheck::SpriteFrameCacheFullCheck()
+{
+    const Size screenSize = Director::getInstance()->getWinSize();
+    // load atlas definition with specified PixelFormat and check that it matches to expected format
+    loadSpriteFrames("Images/test_polygon.plist", Texture2D::PixelFormat::RGBA8888);
+}
+
+void SpriteFrameCacheFullCheck::loadSpriteFrames(const std::string &file, cocos2d::Texture2D::PixelFormat expectedFormat)
+{
+    auto cache = SpriteFrameCache::getInstance();
+
+    CCASSERT(cache->isSpriteFramesWithFileLoaded("plist which not exists") == false, "Plist not exists");
+
+    cache->addSpriteFramesWithFile(file);
+    CCASSERT(cache->isSpriteFramesWithFileLoaded(file) == true, "Plist should be full after loaded");
+
+    cache->removeSpriteFrameByName("not_exists_grossinis_sister.png");
+    CCASSERT(cache->isSpriteFramesWithFileLoaded(file) == true, "Plist should not be still full");
+
+    cache->removeSpriteFrameByName("grossinis_sister1.png");
+    CCASSERT(cache->isSpriteFramesWithFileLoaded(file) == false, "Plist should not be full after remove any sprite");
+
+    cache->addSpriteFramesWithFile(file);
+    CCASSERT(cache->isSpriteFramesWithFileLoaded(file) == true, "Plist should be full after reloaded");
+}

tests/cpp-tests/Classes/SpriteFrameCacheTest/SpriteFrameCacheTest.h

@@ -45,4 +45,35 @@ class SpriteFrameCachePixelFormatTest : public TestCase
 
 private:
     cocos2d::Label *infoLabel;
+};
+
+class SpriteFrameCacheLoadMultipleTimes : public TestCase
+{
+public:
+    CREATE_FUNC(SpriteFrameCacheLoadMultipleTimes);
+
+    virtual std::string title() const override { return "Load same plist multiple times"; }
+    virtual std::string subtitle() const override { return "It shouldn't crash"; }
+
+    SpriteFrameCacheLoadMultipleTimes();
+
+private:
+    void loadSpriteFrames(const std::string &file, cocos2d::Texture2D::PixelFormat expectedFormat);
+
+};
+
+
+class SpriteFrameCacheFullCheck: public TestCase
+{
+public:
+    CREATE_FUNC(SpriteFrameCacheFullCheck);
+
+    virtual std::string title() const override { return "Test isSpriteFramesWithFileLoaded"; }
+    virtual std::string subtitle() const override { return "It shouldn't crash"; }
+
+    SpriteFrameCacheFullCheck();
+
+private:
+    void loadSpriteFrames(const std::string &file, cocos2d::Texture2D::PixelFormat expectedFormat);
+
 };