From 0a6091eeebaac3deeee5d898b22e2e95ccafe519 Mon Sep 17 00:00:00 2001
From: James Turner <zakalawe@mac.com>
Date: Sat, 26 Jun 2021 16:27:22 +0100
Subject: [PATCH] Error reporting for animations / conditions
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Don’t report errors for each condition parse failure, but do
aggregate them at the animation level in ReaderWriterXML

Sentry-Id: FLIGHTGEAR-DD
---
 simgear/props/condition.cxx               | 12 ++++++++----
 simgear/scene/model/SGReaderWriterXML.cxx | 21 +++++++++++++++------
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/simgear/props/condition.cxx b/simgear/props/condition.cxx
index 1da575a4..9886aec2 100644
--- a/simgear/props/condition.cxx
+++ b/simgear/props/condition.cxx
@@ -535,7 +535,8 @@ readComparison( SGPropertyNode *prop_root,
 {
   SGComparisonCondition * condition = new SGComparisonCondition(type, reverse);
   if (node->nChildren() < 2 || node->nChildren() > 3 ) {
-    throw sg_exception("condition: comparison without two or three children");
+    throw sg_exception("condition: comparison without two or three children",
+                       {}, {}, false);
   }
   
   const SGPropertyNode* left = node->getChild(0), 
@@ -551,7 +552,8 @@ readComparison( SGPropertyNode *prop_root,
       SGExpressiond* exp = SGReadDoubleExpression(prop_root, left->getChild(0));
       condition->setLeftDExpression(exp);
     } else {
-      throw sg_exception("Unknown condition comparison left child:" + leftName);
+      throw sg_exception("Unknown condition comparison left child:" + leftName,
+                         {}, {}, false);
     }
   }
     
@@ -565,7 +567,8 @@ readComparison( SGPropertyNode *prop_root,
       SGExpressiond* exp = SGReadDoubleExpression(prop_root, right->getChild(0));
       condition->setRightDExpression(exp);
     } else {
-      throw sg_exception("Unknown condition comparison right child:" + rightName);
+      throw sg_exception("Unknown condition comparison right child:" + rightName,
+                         {}, {}, false);
     }
   }
   
@@ -580,7 +583,8 @@ readComparison( SGPropertyNode *prop_root,
       SGExpressiond* exp = SGReadDoubleExpression(prop_root, n->getChild(0));
       condition->setPrecisionDExpression(exp);
     } else {
-      throw sg_exception("Unknown condition comparison precision child:" + name );
+      throw sg_exception("Unknown condition comparison precision child:" + name,
+                         {}, {}, false);
     }
   }
   
diff --git a/simgear/scene/model/SGReaderWriterXML.cxx b/simgear/scene/model/SGReaderWriterXML.cxx
index dc0c210b..55bf60ec 100644
--- a/simgear/scene/model/SGReaderWriterXML.cxx
+++ b/simgear/scene/model/SGReaderWriterXML.cxx
@@ -574,13 +574,22 @@ sgLoad3DModel_internal(const SGPath& path,
             } // of object-names in the animation
             continue;
         }
-        /*
-         * Setup the model data for the node currently being animated.
-         */
-        modelData.LoadAnimationValuesForElement(animation_nodes[i], i);
+        
+        try {
+            /*
+             * Setup the model data for the node currently being animated.
+             */
+            modelData.LoadAnimationValuesForElement(animation_nodes[i], i);
 
-        /// OSGFIXME: duh, why not only model?????
-        SGAnimation::animate(modelData);
+            /// OSGFIXME: duh, why not only model?????
+            SGAnimation::animate(modelData);
+        } catch (sg_exception& e) {
+            simgear::reportFailure(simgear::LoadFailure::NotFound, simgear::ErrorCode::XMLModelLoad,
+                                   "Couldn't load animation " + animation_nodes[i]->getNameString()
+                                    + ":" + e.getFormattedMessage(),
+                                   modelpath);
+            throw;
+        }
     }
 
     animationcount += animation_nodes.size();