circleguard/circlecore

Use slider library

jxu opened this issue · 3 comments

jxu commented

I'm a contributor to https://github.com/llllllllll/slider and I suggest using that library for beatmap parsing in https://github.com/circleguard/circleguard/blob/master/circleguard/osu_parser.py

Reasoning

  • More robust, better documented, cleaner code
  • I am working on unit tests for .osu parsing so that we can be confident changing the code won't break everything

Same applies for https://github.com/circleguard/circleparse but you may not want to switch

circleguard/osu_parser.py was never meant to be used in the final release, as I just quickly wrote something together that could parse The Big Black. We are going to remove it in the next release (see circleguard/circleguard#31), and switch to https://github.com/osufx/osu-parser, which we included in circleparse. Not quite happy with it though since it's badly documented.
@tybug for comment on replay parsing

tybug commented

Hey jxu! Nice to see you following development.

I looked at slider quite a while ago, before we were at the point where we wanted to implement .osu parsing. After reminding myself of its docs it has quite a few features I was planning to implement on our end, like beatmap caching. And it's certainly a robust library that I would be comfortable adding as a dependency and happy to support development of instead of moving osu-parser entirely into circleparse.

@InvisibleSymbol do you think you can convert the ur function to use slider, as well as everything using osu-parser (and by extension circleparse)?

We already switched to slider in circlecore and there is a open pr in circleguard, so i'm going to close this.