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

Split the beatmap parser #51

Open
fmang opened this issue Jan 25, 2018 · 0 comments
Open

Split the beatmap parser #51

fmang opened this issue Jan 25, 2018 · 0 comments

Comments

@fmang
Copy link
Owner

fmang commented Jan 25, 2018

The current beatmap parser is pretty intricate, because the parsing process is not as simple as it sounds.

I'd consider splitting it into two phases:

  1. The first phase reads the beatmap as a character stream, and generates a raw AST, faithful to the original contents.
  2. The second phase interprets the raw AST and builds a high-level beatmap object. In particular, it implements inheritance, slider path normalization, unit conversion.

Pros:

  • Makes the code and the concepts easier to understand by splitting the task into 2 clear units.
  • Makes the parser easier to test. See Test the beatmap module #44.
  • Supports parsing beatmaps with misordered sections or objects.

Cons:

  • Performance. An all-in-once process will eat less RAM and CPU. Probably a non-issue since the parser represents a tiny fraction of oshu!'s load time.

Alternative: SAX-like parser

SAX is a way to parse XML by reacting to events like “opened ”, “found a text node”.

For a beatmap, those events would be “got a metadata key-value”, “got a hit object”. These would be defined in an abstract class as pure virtual methods. The beatmap parser then calls these methods as it reads the beatmap.

This approach effectively split the raw decoding logic, and the interpreation logic. I gets most of the pros above, without the cons. It won't support unordered objects naively though.

The two ways aren't incompatible, as the AST builder could be easily implemented using that abstract interface.

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

No branches or pull requests

1 participant