redruin1 / factorio-draftsman

A complete, well-tested, and up-to-date module to manipulate Factorio blueprint strings. Compatible with mods.
MIT License
92 stars 17 forks source link

Blueprint with turned off constant combiner fail schema validation #77

Closed ca1f closed 1 year ago

ca1f commented 1 year ago

The following blueprint string causes draftman's blueprint string processing to fail:

0eNqNkMGOwjAMRP/F54Bodwslv4JQlRazayl1qsRFW1X5943LBS6rPdqaeePxCr2fcYrEAnYFGgInsJc
VEn2x87qTZUKwQIIjGGA36qQ6cSy7IYw9sZMQIRsgvuEP2CpfDSALCeETtw1Lx/PYYyyCP0EGppCKN7D
mK69p9o2BBeyuOrT7piTdKOLwlByNUiQG3/X47R5UEMV3Jy8Y/9UmSWDU+4cw6yOq9yaUOs25O58wbwt
125fXGXiUqO2auq0+T+f6dDzXh/ajzvkXPZJ2jw==

which contains a single constant combiner:

{
  "blueprint": {
    "icons": [
      {
        "signal": {
          "type": "item",
          "name": "constant-combinator"
        },
        "index": 1
      }
    ],
    "entities": [
      {
        "entity_number": 1,
        "name": "constant-combinator",
        "position": {
          "x": 155.5,
          "y": -108.5
        },
        "direction": 6,
        "control_behavior": {
          "filters": [
            {
              "signal": {
                "type": "item",
                "name": "stone"
              },
              "count": 1,
              "index": 1
            }
          ],
          "is_on": false
        }
      }
    ],
    "item": "blueprint",
    "version": 281479276920832
  }
}

traceback:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/lib/python3.11/site-packages/draftsman/utils.py", line 858, in inner
    result = func(*args, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^
  File "/lib/python3.11/site-packages/draftsman/blueprintable.py", line 49, in get_blueprintable_from_string
    return Blueprint(blueprintable)
           ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/lib/python3.11/site-packages/draftsman/utils.py", line 858, in inner
    result = func(*args, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^
  File "/lib/python3.11/site-packages/draftsman/classes/blueprint.py", line 140, in __init__
    super(Blueprint, self).__init__(
  File "/lib/python3.11/site-packages/draftsman/utils.py", line 858, in inner
    result = func(*args, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^
  File "/lib/python3.11/site-packages/draftsman/classes/blueprintable.py", line 50, in __init__
    self.setup(**init_data[self._root_item])
  File "/lib/python3.11/site-packages/draftsman/utils.py", line 858, in inner
    result = func(*args, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^
  File "/lib/python3.11/site-packages/draftsman/classes/blueprint.py", line 197, in setup
    self._root["entities"] = EntityList(self, kwargs.pop("entities"))
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/lib/python3.11/site-packages/draftsman/utils.py", line 858, in inner
    result = func(*args, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^
  File "/lib/python3.11/site-packages/draftsman/classes/entitylist.py", line 59, in __init__
    self.append(name, **elem)
  File "/lib/python3.11/site-packages/draftsman/utils.py", line 858, in inner
    result = func(*args, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^
  File "/lib/python3.11/site-packages/draftsman/classes/entitylist.py", line 127, in append
    self.insert(len(self.data), name, copy=copy, merge=merge, **kwargs)
  File "/lib/python3.11/site-packages/draftsman/utils.py", line 858, in inner
    result = func(*args, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^
  File "/lib/python3.11/site-packages/draftsman/classes/entitylist.py", line 196, in insert
    entitylike = new_entity(name, **kwargs)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/lib/python3.11/site-packages/draftsman/entity.py", line 157, in new_entity
    return ConstantCombinator(name, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/lib/python3.11/site-packages/draftsman/prototypes/constant_combinator.py", line 50, in __init__
    super(ConstantCombinator, self).__init__(name, constant_combinators, **kwargs)
  File "/lib/python3.11/site-packages/draftsman/classes/mixins/control_behavior.py", line 55, in __init__
    self.control_behavior = kwargs["control_behavior"]
    ^^^^^^^^^^^^^^^^^^^^^
  File "/lib/python3.11/site-packages/draftsman/prototypes/constant_combinator.py", line 71, in control_behavior
    six.raise_from(DataFormatError(e), None)
  File "<string>", line 3, in raise_from
draftsman.error.DataFormatError: Wrong key 'is_on' in {'filters': [{'signal': {'type': 'item', 'name': 'stone'}, 'count': 1, 'index': 1}], 'is_on': False}

It seems like the "is_on" key is entirely missing from the schema.

redruin1 commented 1 year ago

Somewhat impressed I missed this one, as this is rather obvious. This is now added to the correct schema so importing no longer fails. In addition, I added a is_on attribute to ConstantCombinator, which provides a dedicated place to set this variable. I contemplated adding it under the enable_disable moniker, but I feel this is probably more clear:

combinator = ConstantCombinator()
combinator.is_on = False
assert combinator.is_on == False
assert "is_on" in combinator.control_behavior

combinator.is_on = True
assert combinator.is_on == True
assert "is_on" in combinator.control_behavior

combinator.is_on = None
assert combinator.is_on == None
assert "is_on" not in combinator.control_behavior

Test suite has also been adjusted to account for this change. This fix will be in 1.0.5, which will likely be out this week unless you happen to find more bugs to fix.

ca1f commented 1 year ago

Cool, thanks for your efforts, really nice library!