Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

osrparse 6.0.0 #30

Merged
merged 25 commits into from
Jan 12, 2022
Merged

osrparse 6.0.0 #30

merged 25 commits into from
Jan 12, 2022

Conversation

tybug
Copy link
Collaborator

@tybug tybug commented Jan 8, 2022

This is essentially a rewrite of osrparse and brings consistency in forward facing methods and naming.

Changes

Parsing replays is now done through static methods on Replay:

  • Replay.from_path - parse from a path (most common)
  • Replay.from_file - parse from a live file object
  • Replay.from_string - from a string holding the contents of a replay

"dumping" has been renamed to "write" and is done through a matching set of methods:

  • Replay.write_path - write the replay back to a path
  • Replay.write_file - write the replay back to a live file object
  • Replay.pack - return the contents of the replay as a string, suitable for writing to an osr file

(note that this isn't a breaking change since no actual release has included dumping yet).

The concept of "pure lzma" and parsing only the replay data portion of a replay has been replaced with parse_replay_data, which takes optional parameters decoded and decompressed, as well as a new mode parameter. It now returns List[ReplayEvent] instead of the nonsensical Replay with only play_data filled, as before.

Also added replay.rng_seed, which will be set for replays which have the rng seed.

Renamings:

  • number_{300, 100, 50}s -> count_{300, 100, 50}
  • {geki, katu, misses} -> {count_geki, count_katu, count_miss}
  • is_perfect_combo -> perfect
  • game_mode -> mode
  • player_name -> username
  • play_data -> replay_data
  • mod_combination -> mods

@bemxio
Copy link
Contributor

bemxio commented Jan 9, 2022

hmm, everything seems good, but it still lacks a life bar parser
it might not be useful that commonly, but with that, osrparse will open every capacity of editing a replay file
also, proper docs (with readthedocs) would also be useful, with all of that, it would be perfect

i can try to create both of these, if that's good

osrparse/replay.py Show resolved Hide resolved
osrparse/replay.py Show resolved Hide resolved
osrparse/dump.py Outdated Show resolved Hide resolved
@tybug
Copy link
Collaborator Author

tybug commented Jan 9, 2022

A life bar parser would be great if you want to make a pr for that (branch off this pr to avoid conflicts). I'd prefer to create the readthedocs myself since I'd like to choose the theme, write a few pages, etc. but I agree that with those two things osrparse will be 100% complete (until lazer switches to their own replay format :P)

@bemxio
Copy link
Contributor

bemxio commented Jan 9, 2022

alright then, i will make a parser and a dumper for the life bar
i don't think there will be much trouble if lazer switches the replay format though, at least for now when it's not much used
but there will be a need for a new parser, specifically for that
i don't think it would be that hard though, but we will see

@bemxio
Copy link
Contributor

bemxio commented Jan 10, 2022

here's the pull request with everything about life bars!
also, outside of that, maybe it'll be good to actually use the leb128 module as a dependency for osrparse?
it will be rightful with the license of it, but i am not sure if that will affect the consumer's expierence a bit

@tybug
Copy link
Collaborator Author

tybug commented Jan 10, 2022

I'd prefer not to add such a simple dependency (one that we can reproduce in 30 lines of code). Dependencies can make breaking changes, or require being updated, or disappear from the internet.

@bemxio
Copy link
Contributor

bemxio commented Jan 10, 2022

alright, i understand
is it required to add its license to osrparse though? i am mainly worried about that

@tybug
Copy link
Collaborator Author

tybug commented Jan 10, 2022

I'm no lawyer, but if we're being super technical and legal then including a copy of its license in osrparse probably is required. That said, nobody really cares and a full copy of the license is way overkill, so linking back to the original library in the copied function (as you've already done) is fine.

@bemxio
Copy link
Contributor

bemxio commented Jan 10, 2022

alright then, i guess we are fine!
tell me if there's something wrong with the life bar packer & unpacker

@bemxio
Copy link
Contributor

bemxio commented Jan 11, 2022

is there anything that needs to be done, except documentation?

@tybug
Copy link
Collaborator Author

tybug commented Jan 11, 2022

nope, documentation is the last thing.

@kszlim I pinged you on discord, but in the hopes you'll respond here sooner, could I get the "maintainer" role on this repo? I'd like to set up readthedocs on github pages (per above), but need access to the repository settings to do so. Should be under settings -> manage access

@github-pages github-pages bot temporarily deployed to github-pages January 11, 2022 17:54 Inactive
@tybug
Copy link
Collaborator Author

tybug commented Jan 12, 2022

documentation done and looks good on the site https://kevin-lim.ca/osu-replay-parser/index.html, merging

@tybug tybug merged commit 64e1c93 into master Jan 12, 2022
@tybug tybug deleted the restructure branch January 12, 2022 18:54
@bemxio
Copy link
Contributor

bemxio commented Jan 12, 2022

alright, everything seems nice! i am glad that i could contribute for it a bit
i guess it is done now, since there's everything included
now, there's only either waiting for a bug or a new osu!lazer replay format

@tybug
Copy link
Collaborator Author

tybug commented Jan 12, 2022

yep - thanks for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants