hohav / peppi

Rust parser for Slippi SSBM replay files
MIT License
37 stars 9 forks source link

Fails to parse replay when encountering a direction value of 0 #42

Closed NickCondron closed 7 months ago

NickCondron commented 1 year ago

I have a replay that has a direction of 0 (supposed to be -1 or 1) for pre/post frame update. I think it's a quirk when a warp star item is used (action state 305). This causes a parse error.

Given 0 is a valid/possible value for direction. The real question is how should we change the direction type in the Pre and Post structs? Do we make it an Option<Direction> like it is for the Item struct? Maybe it makes more sense to change the Direction enum to include a variant for 'neither' or 'none'.

pub enum Direction { // how this enum exists in peppi currently
    Left,
    Right,
}

let direction: Option<Direction> = None;

versus

pub enum Direction {
    Left,
    Right,
    Neither,
}

let direction: Direction = Direction::Neither;

Maybe this is splitting hairs, but we generally use a top-level Option to indicate a value that may or may not exist (eg. only exists in later replay versions). Either way this is a simple change, so I can make the PR once we reach a good decision (I lean toward the latter). ~Let me know if I should post the replay.~

NickCondron commented 1 year ago

zero_direction.slp.gz

hohav commented 7 months ago

No longer relevant (direction is now stored "as-is", i.e. as f32).