eee555 / ms-toollib

Algorithms for minesweeper, published on various platforms.
MIT License
16 stars 5 forks source link

Draft: Support RMV2 #7

Open ralokt opened 5 days ago

ralokt commented 5 days ago

This is my - CURRENTLY INCOMPLETE - work in progress for supporting RMV2.

Don't worry about the number of commits - they're short.

It'll be a while until there is an official release that supports this format, but I hope to make an alpha release soon, and what better time to implement it here than when I'm also doing that for vsweep!

And of course, RMV2 is still a draft and could still get some minor changes (I made some while working on this because I realized that they were necessary - libraries that already work with binary blobs shouldn't require a complete refactor to support RMV2, but should validate that it's valid utf-8 nonetheless).

Things I already know are missing:

And I'm sure there are a lot of other things I haven't considered.

Re: the extension properties: I could have put those into a HashMap, but also realized that I don't know how this library wants to treat information that only some formats provide - so for now, they're just being parsed, not put anywhere.

In general, please let me know if there is anything I should change or do differently!

I usually try to make my code consistent with the code around it, so I feel like I should justify right away why I put my test replay in its own directory instead of with the other test replays that are versioned. In my experience, having a suite of replays that need to work properly and produce the correct results is an awesome way to test a library like this; but if the replays are in the project base directory, adding more tests adds to project chaos, which makes it less likely to want to add new ones. So I decided to make a designated directory for testing replays (and assets in general), but of course, if you'd prefer, I can move it to the base dir.

I also ignored the convention for the test names because I didn't want to add new warnings. (side note: would you mind a PR that fixes those? As a terminal user they make the compiler output a lot less usable...)

eee555 commented 5 days ago

but also realized that I don't know how this library wants to treat information that only some formats provide - so for now, they're just being parsed, not put anywhere.

Which information is only provided by RMV2? If such information exists, it should be discarded, just like the skin information in AVF. By the way, the most important thing is to record mouse actions in self.data.video_action_state_recorder.

I also ignored the convention for the test names because I didn't want to add new warnings. (side note: would you mind a PR that fixes those? As a terminal user they make the compiler output a lot less usable...)

We welcome all kinds of PRs, including but not limited to: add features, fix bugs, resolving warnings, and comments in native languages...

eee555 commented 5 days ago

having a suite of replays that need to work properly and produce the correct results is an awesome way to test a library like this

I understand that this approach is the best practice, and in principle, we should move towards the best practice.

ralokt commented 3 days ago

but also realized that I don't know how this library wants to treat information that only some formats provide - so for now, they're just being parsed, not put anywhere.

Which information is only provided by RMV2? If such information exists, it should be discarded, just like the skin information in AVF. By the way, the most important thing is to record mouse actions in self.data.video_action_state_recorder.

The extension properties. They're named properties that clone authors can use to extend the format if they need it to support something specific. For example, if a clone wants to record stnb in the replay, they can do that. Or - @putianyi889 told me that there were discussions about turning some of the modes into layerable game rules? So if a bit field is added to EVF to present that, but RMV2 is already finalized, clones could write the same format to an extension property without requiring changes to the format. More details and examples here: https://github.com/ralokt/rmv_spec/#extension-properties

I also ignored the convention for the test names because I didn't want to add new warnings. (side note: would you mind a PR that fixes those? As a terminal user they make the compiler output a lot less usable...)

We welcome all kinds of PRs, including but not limited to: add features, fix bugs, resolving warnings, and comments in native languages...

Awesome! I hope I get around to it soon :)

ralokt commented 3 days ago

We welcome all kinds of PRs, including but not limited to: add features, fix bugs, resolving warnings, and comments in native languages...

By the way - what exactly do you mean by "comments in native languages"? Comments as in code comments? ie, would you want to have multiple translations for the same comments?

putianyi889 commented 3 days ago

turning some of the modes into layerable game rules?

That's just my vision.

ralokt commented 3 days ago

turning some of the modes into layerable game rules?

That's just my vision.

Ah, sorry, misunderstood you there! I like the idea, tho :)

eee555 commented 3 days ago

but also realized that I don't know how this library wants to treat information that only some formats provide - so for now, they're just being parsed, not put anywhere.

There’s currently no place to store it. A new field could be added to BaseVideo<T>, for example, extension_properties: HashMap or Json string or Array<(&str, &str)> or ???. However, this brings up the challenge of FFI between Rust and Python or JavaScript. I’m not sure if such dynamic properties can be perfectly supported—it would require checking the documentation for pyo3 and wasm-bindgen.

Reading extension properties in python may be meaningful. But further, in order to make this more meaningful, we would need to create a set of interfaces for generating RMV files, similar to generate_evf_v3_raw_data and save_to_evf_file.

turning some of the modes into layerable game rules?

I’m not optimistic about this approach. My philosophy is that before designing a spec, the boundaries of the spec need to be well-defined.

I mean that there are actually many more possible variations to Minesweeper rules. It will inevitably surpass your imagination, and achieving complete uniformity of the format will be impossible. For example, the number of mines in each cell could even be negative, or a floating-point value (quantum Minesweeper).

By the way - what exactly do you mean by "comments in native languages"? Comments as in code comments? ie, would you want to have multiple translations for the same comments?

I don't mind having comments in multiple languages in one part of the code.