Issue warning when variant given is exceeds declared variants. Load error sprite in that case.

2 files changed, 24 insertions, 14 deletions

diff --git a/src/resources/sprite/spritedef.cpp b/src/resources/sprite/spritedef.cpp
index d47ce7b23..97ca833aa 100644
--- a/src/resources/sprite/spritedef.cpp
+++ b/src/resources/sprite/spritedef.cpp
@@ -103,12 +103,18 @@ SpriteDef *SpriteDef::load(const std::string &animationFile,
         return nullptr;
     XmlNodePtrConst rootNode = doc->rootNode();
 
-    if ((rootNode == nullptr) || !xmlNameEqual(rootNode, "sprite"))
+    SpriteDef *const def = new SpriteDef;
+    def->mSource = animationFile;
+    def->mProcessedFiles.insert(animationFile);
+
+    if ((rootNode == nullptr)
+        || !xmlNameEqual(rootNode, "sprite")
+        || !def->loadSprite(rootNode, variant, palettes) )
     {
-        reportAlways("Error, failed to parse sprite %s",
-            animationFile.c_str())
+        reportAlways("Error, failed to load sprite %s", animationFile.c_str())
         const std::string errorFile = pathJoin(paths.getStringValue("sprites"),
             paths.getStringValue("spriteErrorFile"));
+        delete def;
         BLOCK_END("SpriteDef::load")
         doc->decRef();
         if (animationFile != errorFile)
@@ -116,10 +122,6 @@ SpriteDef *SpriteDef::load(const std::string &animationFile,
         return nullptr;
     }
 
-    SpriteDef *const def = new SpriteDef;
-    def->mSource = animationFile;
-    def->mProcessedFiles.insert(animationFile);
-    def->loadSprite(rootNode, variant, palettes);
     def->substituteActions();
     if (settings.fixDeadAnimation)
         def->fixDeadAction();
@@ -202,7 +204,7 @@ void SpriteDef::substituteActions()
     substituteAction(SpriteAction::DEADRIDE, SpriteAction::DEAD);
 }
 
-void SpriteDef::loadSprite(XmlNodeConstPtr spriteNode,
+bool SpriteDef::loadSprite(XmlNodeConstPtr spriteNode,
                            const int variant,
                            const std::string &palettes)
 {
@@ -210,17 +212,24 @@ void SpriteDef::loadSprite(XmlNodeConstPtr spriteNode,
     if (spriteNode == nullptr)
     {
         BLOCK_END("SpriteDef::loadSprite")
-        return;
+        return false;
     }
     // Get the variant
     const int variantCount = XML::getProperty(spriteNode, "variants", 0);
     int variant_offset = 0;
 
-    if (variantCount > 0 && variant < variantCount)
+    if (variantCount > 0)
     {
-        variant_offset = variant * XML::getProperty(spriteNode,
-            "variant_offset",
-            0);
+        if (variant < variantCount)
+            variant_offset = variant
+                           * XML::getProperty(spriteNode, "variant_offset",
+                                              0);
+        else {
+            logger->assertLog("Error: variant %d >= %d (variants property)",
+                              variant, variantCount);
+            BLOCK_END("SpriteDef::loadSprite")
+            return false;
+        }
     }
 
     for_each_xml_child_node(node, spriteNode)
@@ -233,6 +242,7 @@ void SpriteDef::loadSprite(XmlNodeConstPtr spriteNode,
             includeSprite(node, variant);
     }
     BLOCK_END("SpriteDef::loadSprite")
+    return true;
 }
 
 void SpriteDef::loadImageSet(XmlNodeConstPtr node,
diff --git a/src/resources/sprite/spritedef.h b/src/resources/sprite/spritedef.h
index 6bfe93f6b..157c3d337 100644
--- a/src/resources/sprite/spritedef.h
+++ b/src/resources/sprite/spritedef.h
@@ -101,7 +101,7 @@ class SpriteDef final : public Resource
         /**
          * Loads a sprite element.
          */
-        void loadSprite(XmlNodeConstPtr spriteNode,
+        bool loadSprite(XmlNodeConstPtr spriteNode,
                         const int variant,
                         const std::string &palettes);