fix(blueprint): Entity::extra_data not always correctly deserializing

blueprint/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "blueprint"
-version = "0.7.0"
+version = "0.7.1"
 authors.workspace = true
 edition.workspace = true
 

blueprint/src/blueprint/entity.rs

@@ -47,8 +47,8 @@ pub struct Entity {
     // pub wires -> handled outside of entity
     pub burner_fuel_inventory: Option<InventoryWithFilters>,
 
-    #[serde(flatten, default, skip_serializing_if = "helper::is_default")]
-    pub extra_data: EntityExtraData,
+    #[serde(flatten, default, skip_serializing_if = "Option::is_none")]
+    pub extra_data: Option<EntityExtraData>,
 }
 
 impl PartialOrd for Entity {
@@ -73,9 +73,9 @@ impl crate::GetIDs for Entity {
 
 #[skip_serializing_none]
 #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
-#[serde(untagged, deny_unknown_fields)]
+#[serde(untagged)]
 pub enum EntityExtraData {
-    None {},
+    //None {},
     Accumulator {
         control_behavior: AccumulatorControlBehavior,
     },
@@ -530,20 +530,14 @@ impl EntityExtraData {
     }
 }
 
-impl Default for EntityExtraData {
-    fn default() -> Self {
-        Self::None {}
-    }
-}
-
 impl crate::GetIDs for EntityExtraData {
     #[allow(clippy::too_many_lines)]
     fn get_ids(&self) -> crate::UsedIDs {
         let mut ids = crate::UsedIDs::default();
 
         match self {
-            Self::None {}
-            | Self::ElectricEnergyInterface { .. }
+            //Self::None {} |
+            Self::ElectricEnergyInterface { .. }
             | Self::HeatInterface { .. }
             | Self::LinkedBelt { .. }
             | Self::LinkedContainer { .. }
@@ -998,3 +992,34 @@ pub struct VehicleAutomaticTargetingParameters {
     pub auto_target_without_gunner: bool,
     pub auto_target_with_gunner: bool,
 }
+
+#[cfg(test)]
+mod tests {
+    #![allow(clippy::unwrap_used)]
+
+    use super::*;
+    use crate::bp_string_to_json;
+
+    #[test]
+    fn omni_crafter_entities() {
+        let raw_omni = bp_string_to_json(include_str!("../../tests/omni_crafter_GWP.txt")).unwrap();
+        let json_data = serde_json::from_str::<serde_json::Value>(&raw_omni).unwrap();
+
+        let bps = json_data["blueprint_book"]["blueprints"]
+            .as_array()
+            .unwrap();
+
+        for entry in bps {
+            let entities = entry["blueprint"]["entities"].as_array().unwrap();
+
+            for raw_entity in entities {
+                let entity_json = serde_json::to_string_pretty(raw_entity).unwrap();
+                let Err(e) = serde_json::from_str::<Entity>(&entity_json) else {
+                    continue;
+                };
+
+                panic!("Failed to deserialize entity {e:?}: {entity_json}");
+            }
+        }
+    }
+}

blueprint/src/blueprint/entity/control_behavior.rs

@@ -146,7 +146,7 @@ pub struct AssemblingMachineControlBehavior {
     pub set_recipe: bool,
 
     #[serde(flatten)]
-    shared: FurnaceControlBehavior,
+    pub(super) shared: FurnaceControlBehavior,
 }
 
 impl Deref for AssemblingMachineControlBehavior {
@@ -303,7 +303,7 @@ pub struct FurnaceControlBehavior {
     pub working_signal: Option<SignalID>,
 
     #[serde(flatten)]
-    common: CommonControlBehavior,
+    pub(super) common: CommonControlBehavior,
 }
 
 impl Deref for FurnaceControlBehavior {

blueprint/src/lib.rs

@@ -826,7 +826,7 @@ mod tests {
             assert_eq!(sc.direction, types::Direction::East);
             assert_eq!(sc.position, Position { x: 44.0, y: -26.5 });
 
-            let EntityExtraData::Combinator(cd) = sc.extra_data else {
+            let Some(EntityExtraData::Combinator(cd)) = sc.extra_data else {
                 panic!("extra_data is not Combinator");
             };
 

scanner/src/lib.rs

@@ -300,41 +300,53 @@
     prototypes::entity::RenderOpts {
         position: (&value.position).into(),
         direction: value.direction,
-        orientation: value.extra_data.orientation(),
+        orientation: value.extra_data.as_ref().and_then(|ed| ed.orientation()),
         mirrored: value.mirror,
-        elevated: value.extra_data.rail_layer() == Some(&"elevated".to_owned()),
+        elevated: value.extra_data.as_ref().and_then(|ed| ed.rail_layer())
+            == Some(&"elevated".to_owned()),
         variation: None, // value.extra_data.variation, // variation is not documented, unsure where it might show up
-        pickup_position: value.extra_data.pickup_position().copied(),
+        pickup_position: value
+            .extra_data
+            .as_ref()
+            .and_then(|ed| ed.pickup_position().copied()),
         connections: None,
         underground_in: value
             .extra_data
-            .belt_connection_type()
             .as_ref()
-            .map(|&t| t == blueprint::BeltConnectionType::Input),
+            .and_then(|ed| ed.belt_connection_type())
+            .map(|t| t == blueprint::BeltConnectionType::Input),
         connected_gates: Vec::new(),
         draw_gate_patch: false,
         arithmetic_operation: value
             .extra_data
-            .combinator_data()
+            .as_ref()
+            .and_then(|ed| ed.combinator_data())
             .and_then(blueprint::CombinatorData::arithmetic_parameters)
             .map(blueprint::ArithmeticCombinatorParameters::operation),
         decider_operation: value
             .extra_data
-            .combinator_data()
+            .as_ref()
+            .and_then(|ed| ed.combinator_data())
             .and_then(blueprint::CombinatorData::decider_parameters)
             .map(blueprint::DeciderCombinatorParameters::operation),
         selector_operation: value
             .extra_data
-            .combinator_data()
+            .as_ref()
+            .and_then(|ed| ed.combinator_data())
             .and_then(blueprint::CombinatorData::selector_parameters)
             .map(blueprint::SelectorCombinatorParameters::operation),
-        runtime_tint: value.extra_data.color().map(std::convert::Into::into),
+        runtime_tint: value
+            .extra_data
+            .as_ref()
+            .and_then(|ed| ed.color())
+            .map(std::convert::Into::into),
         entity_id: value.entity_number.into(),
         circuit_connected: false, // TODO: more complex to figure out with wire info being outside of the entity
         logistic_connected: false, // TODO: bit more involved to wire all CBs up
         fluid_recipe: value
             .extra_data
-            .recipe()
+            .as_ref()
+            .and_then(|ed| ed.recipe())
             .map(|r| data.recipe_has_fluid(r))
             .unwrap_or_default(),
     }
@@ -566,7 +578,7 @@
             }
 
             'recipe_icon: {
-                let recipe = e.extra_data.recipe();
+                let recipe = e.extra_data.as_ref().and_then(|ed| ed.recipe());
                 if let Some(recipe) = recipe
                     && e_data.recipe_visible()
                 {
@@ -597,7 +609,7 @@
 
             // filter icons / priority arrows
             'filters_priority: {
-                if let Some(prio_in) = &e.extra_data.input_priority()
+                if let Some(prio_in) = &e.extra_data.as_ref().and_then(|ed| ed.input_priority())
                     && prio_in != &SplitterPriority::None
                 {
                     let offset = e.direction.rotate_vector(
@@ -621,10 +633,11 @@
                     );
                 }
 
-                if let Some(prio_out) = &e.extra_data.output_priority()
+                if let Some(prio_out) = &e.extra_data.as_ref().and_then(|ed| ed.output_priority())
                     && prio_out != &SplitterPriority::None
                 {
-                    if let Some(item_filter) = e.extra_data.splitter_filter()
+                    if let Some(item_filter) =
+                        e.extra_data.as_ref().and_then(|ed| ed.splitter_filter())
                         && let Some(filter) = item_filter.name.as_ref()
                     {
                         let Some(filter) = data.get_item_icon(
@@ -674,20 +687,20 @@
 
                 let mut active_filters = None;
                 let mut _active_mode = None; // TODO: add blacklist overlay when applicable
-                if let blueprint::EntityExtraData::Inserter {
+                if let Some(blueprint::EntityExtraData::Inserter {
                     filters,
                     filter_mode,
                     use_filters: true,
                     ..
-                } = &e.extra_data
+                }) = &e.extra_data
                 {
                     active_filters = Some(filters);
                     _active_mode = Some(filter_mode);
-                } else if let blueprint::EntityExtraData::Loader {
+                } else if let Some(blueprint::EntityExtraData::Loader {
                     filters,
                     filter_mode,
                     ..
-                } = &e.extra_data
+                }) = &e.extra_data
                 {
                     active_filters = Some(filters);
                     _active_mode = Some(filter_mode);
@@ -843,13 +856,23 @@
                 }
 
                 indicator_helper(
-                    proto.get_pickup_position(e.direction, e.extra_data.pickup_position().copied()),
+                    proto.get_pickup_position(
+                        e.direction,
+                        e.extra_data
+                            .as_ref()
+                            .and_then(|ed| ed.pickup_position().copied()),
+                    ),
                     &render_opts,
                     &indicator_line,
                     &mut render_layers,
                 );
                 indicator_helper(
-                    proto.get_insert_position(e.direction, e.extra_data.drop_position().copied()),
+                    proto.get_insert_position(
+                        e.direction,
+                        e.extra_data
+                            .as_ref()
+                            .and_then(|ed| ed.drop_position().copied()),
+                    ),
                     &render_opts,
                     &indicator_arrow,
                     &mut render_layers,
@@ -1072,9 +1095,9 @@
                                 if let Some(dir) = dir {
                                     let u_output = other
                                         .extra_data
-                                        .belt_connection_type()
                                         .as_ref()
-                                        .map(|&t| t == blueprint::BeltConnectionType::Output);
+                                        .and_then(|ed| ed.belt_connection_type())
+                                        .map(|t| t == blueprint::BeltConnectionType::Output);
 
                                     let Some(u_output) = u_output else {
                                         continue;