Merge pull request #729 from CesiumGS/fix-default-property-constructor

Fix `EmptyPropertyWithDefault` constructors
CesiumGS · Sep 20, 2023 · d135c92 · d135c92
2 parents 107ab03 + 8a1fb1a
commit d135c92
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 36 deletions.
diff --git a/CesiumGltf/include/CesiumGltf/PropertyAttributePropertyView.h b/CesiumGltf/include/CesiumGltf/PropertyAttributePropertyView.h
@@ -150,16 +150,20 @@ class PropertyAttributePropertyView<ElementType, false>
       const ClassProperty& classProperty,
       int64_t size) noexcept
       : PropertyView<ElementType, false>(classProperty), _accessor{}, _size{0} {
-    // Don't override the status / size if something is wrong with the class
-    // property's definition.
     if (this->_status != PropertyAttributePropertyViewStatus::Valid) {
+      // Don't override the status / size if something is wrong with the class
+      // property's definition.
       return;
     }
 
-    assert(
-        classProperty.defaultProperty &&
-        "Cannot construct a valid property view for an empty property with no "
-        "default value.");
+    if (!classProperty.defaultProperty) {
+      // This constructor should only be called if the class property *has* a
+      // default value. But in the case that it does not, this property view
+      // becomes invalid.
+      this->_status =
+          PropertyAttributePropertyViewStatus::ErrorNonexistentProperty;
+      return;
+    }
 
     this->_status =
         PropertyAttributePropertyViewStatus::EmptyPropertyWithDefault;
@@ -302,16 +306,20 @@ class PropertyAttributePropertyView<ElementType, true>
       const ClassProperty& classProperty,
       int64_t size) noexcept
       : PropertyView<ElementType, true>(classProperty), _accessor{}, _size{0} {
-    // Don't override the status / size if something is wrong with the class
-    // property's definition.
     if (this->_status != PropertyAttributePropertyViewStatus::Valid) {
+      // Don't override the status / size if something is wrong with the class
+      // property's definition.
       return;
     }
 
-    assert(
-        classProperty.defaultProperty &&
-        "Cannot construct a valid property view for an empty property with no "
-        "default value.");
+    if (!classProperty.defaultProperty) {
+      // This constructor should only be called if the class property *has* a
+      // default value. But in the case that it does not, this property view
+      // becomes invalid.
+      this->_status =
+          PropertyAttributePropertyViewStatus::ErrorNonexistentProperty;
+      return;
+    }
 
     this->_status =
         PropertyAttributePropertyViewStatus::EmptyPropertyWithDefault;

diff --git a/CesiumGltf/include/CesiumGltf/PropertyTablePropertyView.h b/CesiumGltf/include/CesiumGltf/PropertyTablePropertyView.h
@@ -213,16 +213,19 @@ class PropertyTablePropertyView<ElementType, false>
         _stringOffsets{},
         _stringOffsetType{PropertyComponentType::None},
         _stringOffsetTypeSize{0} {
-    // Don't override the status / size if something is wrong with the class
-    // property's definition.
     if (this->_status != PropertyTablePropertyViewStatus::Valid) {
+      // Don't override the status / size if something is wrong with the class
+      // property's definition.
       return;
     }
 
-    assert(
-        classProperty.defaultProperty &&
-        "Cannot construct a valid property view for an empty property with no "
-        "default value.");
+    if (!classProperty.defaultProperty) {
+      // This constructor should only be called if the class property *has* a
+      // default value. But in the case that it does not, this property view
+      // becomes invalid.
+      this->_status = PropertyTablePropertyViewStatus::ErrorNonexistentProperty;
+      return;
+    }
 
     this->_status = PropertyTablePropertyViewStatus::EmptyPropertyWithDefault;
     this->_size = size;
@@ -562,16 +565,19 @@ class PropertyTablePropertyView<ElementType, true>
         _arrayOffsets{},
         _arrayOffsetType{PropertyComponentType::None},
         _arrayOffsetTypeSize{0} {
-    // Don't override the status / size if something is wrong with the class
-    // property's definition.
     if (this->_status != PropertyTablePropertyViewStatus::Valid) {
+      // Don't override the status / size if something is wrong with the class
+      // property's definition.
       return;
     }
 
-    assert(
-        classProperty.defaultProperty &&
-        "Cannot construct a valid property view for an empty property with no "
-        "default value.");
+    if (!classProperty.defaultProperty) {
+      // This constructor should only be called if the class property *has* a
+      // default value. But in the case that it does not, this property view
+      // becomes invalid.
+      this->_status = PropertyTablePropertyViewStatus::ErrorNonexistentProperty;
+      return;
+    }
 
     this->_status = PropertyTablePropertyViewStatus::EmptyPropertyWithDefault;
     this->_size = size;

diff --git a/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h b/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h
@@ -271,16 +271,20 @@ class PropertyTexturePropertyView<ElementType, false>
         _texCoordSetIndex(0),
         _channels(),
         _swizzle() {
-    // Don't override the status / size if something is wrong with the class
-    // property's definition.
     if (this->_status != PropertyTexturePropertyViewStatus::Valid) {
+      // Don't override the status / size if something is wrong with the class
+      // property's definition.
       return;
     }
 
-    assert(
-        classProperty.defaultProperty &&
-        "Cannot construct a valid property view for an empty property with no "
-        "default value.");
+    if (!classProperty.defaultProperty) {
+      // This constructor should only be called if the class property *has* a
+      // default value. But in the case that it does not, this property view
+      // becomes invalid.
+      this->_status =
+          PropertyTexturePropertyViewStatus::ErrorNonexistentProperty;
+      return;
+    }
 
     this->_status = PropertyTexturePropertyViewStatus::EmptyPropertyWithDefault;
   }
@@ -500,16 +504,20 @@ class PropertyTexturePropertyView<ElementType, true>
         _texCoordSetIndex(0),
         _channels(),
         _swizzle() {
-    // Don't override the status / size if something is wrong with the class
-    // property's definition.
     if (this->_status != PropertyTexturePropertyViewStatus::Valid) {
+      // Don't override the status / size if something is wrong with the class
+      // property's definition.
       return;
     }
 
-    assert(
-        classProperty.defaultProperty &&
-        "Cannot construct a valid property view for an empty property with no "
-        "default value.");
+    if (!classProperty.defaultProperty) {
+      // This constructor should only be called if the class property *has* a
+      // default value. But in the case that it does not, this property view
+      // becomes invalid.
+      this->_status =
+          PropertyTexturePropertyViewStatus::ErrorNonexistentProperty;
+      return;
+    }
 
     this->_status = PropertyTexturePropertyViewStatus::EmptyPropertyWithDefault;
   }