hohav / py-slippi

Python library for parsing SSBM replay files
MIT License
56 stars 25 forks source link

ActionState/Attack enums - missing value handling? #4

Closed avkliskova closed 5 years ago

avkliskova commented 5 years ago

This appears to be related to #2.

Attempting to parse this replay raises a ValueError:

>>> from slippi import Game
>>> Game('6747-15-Game_20190324T150220.slp')
ValueError: 59 is not a valid Attack

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.7/site-packages/slippi/game.py", line 33, in __init__
    self._parse_file(path)
  File "/usr/lib/python3.7/site-packages/slippi/game.py", line 85, in _parse_file
    event = self._parse_event(stream, payload_sizes)
  File "/usr/lib/python3.7/site-packages/slippi/game.py", line 66, in _parse_event
    evt.Frame.Port.Data.Post(stream))
  File "/usr/lib/python3.7/site-packages/slippi/event.py", line 267, in __init__
    self.last_attack_landed = Attack(last_attack_landed) if last_attack_landed else None #: optional(:py:class:`Attack`): Last attack that this character landed
  File "/usr/lib/python3.7/enum.py", line 309, in __call__
    return cls.__new__(cls, value)
  File "/usr/lib/python3.7/enum.py", line 561, in __new__
    raise exc
  File "/usr/lib/python3.7/enum.py", line 545, in __new__
    result = cls._missing_(value)
  File "/usr/lib/python3.7/enum.py", line 574, in _missing_
    raise ValueError("%r is not a valid %s" % (value, cls.__name__))
ValueError: 59 is not a valid Attack

The missing value 59 probably denotes DK's cargo hold.

The game can also record values for ActionState that will cause similar errors (I suspect that any replay containing DK's down-B, or Kirby's side-B or down-B, will do.) My data indicate that action state values can range as high as 537. Further, values of at least 341 (0x155) indicate special moves, and change from character to character.

Filling in the missing values with dummies (_341 = 341, etc.) should solve the problem. I also recommend that the (erroneous) action state values from 341 to 382 be deleted and replaced with dummies.

Will submit a pull request when time permits.

vladfi1 commented 5 years ago

I'm seeing this as well, with action state 87.

hohav commented 5 years ago

Thanks for the report. Unknown Attack and ActionState values are now left as raw ints (with a warning printed to stderr). I also created #6 to address the spurious ActionState values.