hohav / peppi Goto Github PK
View Code? Open in Web Editor NEWRust parser for Slippi SSBM replay files
License: MIT License
Rust parser for Slippi SSBM replay files
License: MIT License
Peppi is forward compatible with new replay for deserialization. However it would be impossible to be forward compatible for serialization. We should warn users when they try to serialize a game with a version that is unsupported.
If the user does serialize a replay with an unsupported version, maybe we should modify the version that we write to the file to be the MAX_SUPPORTED_VERSION
instead of the actual version.
Slippc implements a custom compression scheme for slp files: https://github.com/pcrain/slippc#compression
The format filetype is called zlp
. Supporting reading/writing to this format would be really useful.
A new field was added to the game end event:
https://github.com/project-slippi/slippi-wiki/blob/master/SPEC.md#game-end
Parsing is simple enough. It's possible more player-specific fields get added to the game end event in the future. Maybe it's best to create a PlayerEnd
struct to hold these values like the Player
struct in game start.
https://github.com/hohav/peppi/blob/main/peppi/src/serde/de.rs#L859
The expect_bytes
function is called many times. The err!()
macro expands to a format!()
call which inflicts a performance penalty. Changing the line to Err(std::io::Error::from(std::io::ErrorKind::InvalidData))
makes the code 4-5% faster for the event handlers use case. Making similar changes to other err!()
calls made very little difference. It is clear to me that this function is called so much more often that only the performance penalty there is noticeable.
Obviously you lose info that is useful for debugging/troubleshooting, so I'm not saying we should change the code uncritically. However, it might be worth the tradeoff, or there may be a better alternative that isn't so expensive.
When a parsed value contains invalid data, an error is returned. However the location that will get reported to the user will be the location after the end of the last read instead of before it.
When a rollback frame comes in with an item event, that item event will be appended to the vector of items for that frame, when it should be ignored because it is a rollback frame.
Items are appended here. Notice that there is no check for the Rollback::First option, and no check if the item is part of a rollback frame or an original frame.
Line 829 in serde/de.rs can overflow and panic if the replay file is corrupted. The function should return a ParseError instead. I have had this happen on actual replay files that were generated by Slippi, so it doesn't even require random garbage to trigger (I'm not sure why they are corrupted, but I have dozens of such files from playing thousands of games).
There may be other possible places that could panic, but I have not checked the entire parsing code. Any unchecked arithmetic is a potential overflow though. On these particular files it only panics if I am skipping frames though.
Example file attached. Note however that I'm not asking why the file doesn't parse, I know it's corrupt. I'm just saying that the parsing functions should not panic.
Heya! I have some code that runs succesfully on about 99% of my replays. However, 1% of the replays give me the following errors:
[src/replay.rs:113] path = "/home/odd/downloads/replays/Game_20190324T121031.slp"
[src/replay.rs:114] e = Custom {
kind: InvalidData,
error: "expected: [85, 8, 109, 101, 116, 97, 100, 97, 116, 97, 123], got: [53, 109, 63, 128, 0, 0, 0, 0, 0, 0, 66]",
}
[src/replay.rs:113] path = "/home/odd/downloads/replays/Game_20190324T153815.slp"
[src/replay.rs:114] e = Custom {
kind: InvalidData,
error: "expected: [85, 8, 109, 101, 116, 97, 100, 97, 116, 97, 123], got: [53, 13, 54, 1, 161, 55, 0, 63, 56, 0, 37]",
}
The issue seems to be consistent across the different replays. I've uploaded the replays here: https://gofile.io/d/ofg8dO
When I attempt to parse this replay and print out the game object I get the wrong result (irrelevant parts removed). As you can see it looks good in the metadata, but something is wrong in the start block. Verified that it parses correctly using official slippi-js.
Game {
metadata: Metadata {
date: Some(
2022-05-03T04:53:54Z,
),
...
players: Some(
[
Player {
port: P1,
characters: Some(
{
22: 18497,
},
),
netplay: Some(
Netplay {
code: "MANG#0",
name: "mang",
},
),
},
Player {
port: P2,
characters: Some(
{
18: 18497,
},
),
netplay: Some(
Netplay {
code: "ZAIN#0",
name: "Zain",
},
),
},
],
),
},
start: Start {
slippi: Slippi {
version: Version(
3,
12,
0,
),
},
...
players: [
Player {
port: P1,
...
netplay: Some(
Netplay {
name: "mang",
code: "mang", // HERE
},
),
},
Player {
port: P2,
...
netplay: Some(
Netplay {
name: "", // HERE
code: "", // HERE
},
),
},
]
...
}
A couple times using this library I've expected a trait to be implemented for a type when it wasn't. For example the Port type in src/model/primitives.rs
doesn't implement PartialOrd/Ord, so trying to compare the port of two players to determine which comes first in the Frame<N>
struct is tedious.
I don't know when ParitalOrd is sufficient or when you want PartialOrd & Ord. Same goes with ParitalEq and Eq. Seems like some types have just PartialEq when they could also have Eq. I don't know if there's a rhyme or reason why things are the way they are.
See also: https://rust-lang.github.io/api-guidelines/interoperability.html#c-common-traits
I'd be willing to go through and add some of these to the derive statements, but it would help to hear your input.
In the JS version of the slippi parser, there is a method to get the stats of a game getStats()
.
It would be nice if peppi had a way to get those.
This bug is similar to #19, however this bug effects perfectly valid replays that should parse. The issue is on the same line, de.rs:982. For streaming replays length
is 0, which guarantees an underflow and panic. The solution is straightforward: If length
is 0, then do not skip directly to GameEnd
, instead keep processing events normally, but just don't parse the frame events.
I also noticed that there is no unit testing for streaming replays at all, this should be fixed. I have attached a streaming replay that can be used for unit testing. It is identical to game.slp
, except the length field has been modified to 0.
Currently we support skipping the frame data entirely. There are some use cases where we only need the final frame (eg. determining winner after lrastart). It would be nice to support this use case without parsing every frame and then just picking the last one.
We might also want to support parsing only the first frame to handle the sheik fix. Replays before 1.6.0 game start didn't correctly differentiate zelda/sheik, so you have to check the first frame to tell which one actually started the game.
Previously (like 7 months ago) I had the parse::parse_metadata()
function available, but that branch is currently unavailable. I see in the source code that you've introduced a new public state called Config
, which supposedly isn't passed anywhere yet. How can I make it with the const-generics
branch such that I only get the metadata, skipping parsing of frames and making it overall much faster?
Adding new public fields to a public type can break backwards compatibility. If we exect some of the types to change as the slp spec changes, we should use the #[non-exhaustive]
declaration.
I understand peppi currently doesn't make any backwards compatibility guarantees, but it's probably still a good idea.
https://rust-unofficial.github.io/patterns/idioms/priv-extend.html
I've been looking into adding benchmarks. It would also help clarify if certain changes have a justifiable performance impact (#24, #25). Right now, I'm looking at Criterion (https://github.com/bheisler/cargo-criterion). We may also want to use Iai (https://github.com/bheisler/iai) for CI benchmarks.
If anyone has experience with either of these tools (or something else) I would appreciate some guidance.
Also, we need some set of replay files that cover different performance scenarios. Different matchups, Ice Climbers games, long games, short games, netplay games, local games, non-standard games (ie. not 4-stock 8 minutes), etc. I wonder if it would make sense to store the replay set compressed w/ peppi and inflate each time we run the benchmarks.
Hey all, I'm starting to fiddle with this library and I believe some more documentation could go a long way. Is there more of it being kept somewhere else outside of GitHub and if not, how can I help to create some "recipes"? Thanks
I did a quick check of the ground ID's on the tournament legal stages, which I've left in tables below. There's 2 holes in this data: stadium transformations, and the little divots on the walls of yoshi's (though I'm not sure they even have IDs).
Implementation isn't 100% straight forward. Every stage has little areas next to the ledges - sometimes multiple per side - that count as "different ground". We could represent it exactly how it is in the game, or we could just lump them all in under "it's the main stage". If they are lumped, do we lump yoshi's slants in as well? They're the same conceptually, but they affect gameplay way more.
The other option is to just put in both, likely with the generalized one being the default (e.g. ground_id
and ground_id_exact
).
ID 65535 is always the respawn/entry platform
FD:
ID | Position |
---|---|
0 | Left edge |
1 | Main |
2 | Right edge |
Yoshi's:
ID | Position |
---|---|
0 | Randall |
1 | Left plat |
2 | Left slant |
3 | Main |
4 | Top plat |
5 | Right plat |
6 | Right Slant |
Battlefield:
ID | Position |
---|---|
0 | Left edge |
1 | Main |
2 | Left plat |
3 | Top plat |
4 | Right plat |
5 | Right edge |
Stadium:
ID | Position |
---|---|
34 | Main |
35 | Left plat |
36 | Right plat |
51 | Left edge (outside) |
52 | Left edge (inside) |
53 | Right edge (inside) |
54 | Right edge (outside) |
DL:
ID | Position |
---|---|
0 | Left plat |
1 | Right plat |
2 | Top plat |
3 | Left edge |
4 | Main |
5 | Right edge |
FoD:
ID | Position |
---|---|
0 | Left plat |
1 | Right plat |
2 | Top plat |
3 | Left edge (outside) |
4 | Left edge (inside) |
5 | Main |
6 | Right edge (inside) |
7 | Right edge (outside) |
melee isn't consistent about which frame is considered the "first" frame of an action state. Knowing that you're on the first frame of an action can be pretty useful when generating stats, so it might be worth adding this data somewhere.
Altf4 already has a CSV containing all the info necessary, it'd just be a matter of representing it internally and implementing a convenient way to check. In libmelee he reindexes all action states, but i don't think that's necessary at the base level (though may be convenient with higher levels of abstraction like stats generation)
Looks like slippi-js is implementing this feature (https://github.com/project-slippi/slippi-js/compare/feature/gecko-list), so we probably should too. Documentation on gecko codes is oddly hard to find, but I found this:
https://gamehacking.org/faqs/wiicodetypes.html
https://web.archive.org/web/20191211012905/http://geckocodes.org/index.php?arsenal=1
https://github.com/FIX94/Nintendont/blob/master/codehandler/codehandleronly.s
https://github.com/dolphin-emu/dolphin/blob/master/Source/Core/Core/GeckoCode.h
Or we can just copy what fizzi did
It's a little unclear from the readme but it seems like you don't support reading from a live match. When building peppi and running the slp command on a live file it will error out saying the buffer isn't full. Here's the error when trying to parse from a live game
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: ParseError { pos: Some(55), error: Custom { kind: InvalidData, e rror: "expected: [85, 8, 109, 101, 116, 97, 100, 97, 116, 97, 123], got: [54, 3, 7, 0, 0, 50, 1, 134, 76, 195, 0]" } }', src/main.rs:26: 5
There's a long-standing bug in Slippi where Game End events are not always written to the Slippi file (see first paragraph in spec for Game End event https://github.com/project-slippi/slippi-wiki/blob/master/SPEC.md#game-end). There are a lot of normal replay files that are only missing the game end event.
Currently, peppi fails to parse these replays into a game considering it a parse error. This behavior is sound considering the spec technically guarantees the last event to be a Game End. However, it is not useful because there are many normal games that fail to be parsed. Slippi-js parses such files without complaint.
Version 1.0.0-alpha.7 was never pushed to crates. Not too problematic but still would be nice to have
https://github.com/project-slippi/slippi-wiki/blob/master/SPECTATOR_PROTOCOL.md
This could be used to broadcast to a local program providing a faster live feed of a game. I haven't really thought about this further, but I think it's something peppi should do eventually.
offense_ratio, defense_ratio, and model_scale are all 32-bit floats, they are stored consecutively within the game info block starting at 0x60 + 0x24i + 0x18.
However the parsing for these values begins at 0x60 + 0x24i + 0x14 (count carefully). The actual values stored at this location are damage start and damage spawn, which are not currently part of the model.
The tests are incorrect so this is not being caught. See here for example, the offense_ratio should actually be 1.0 but the tests incorrectly claims it is 0.0. If you wish to examine the game.slp file by hand, you'll find that offense_ratio begins at 0x9A and indeed holds a 1.0 value (hex 0x3F800000, big endian), note that it is followed by two more 1.0 values, corresponding to defense_ratio and model_scale for the first player.
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.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.