Ajarmar / universal-pokemon-randomizer-zx

Public repository of source code for the Universal Pokemon Randomizer ZX
GNU General Public License v3.0
566 stars 142 forks source link

[Contribution Idea] Change the Settings String to JSON format #634

Open naptera opened 1 year ago

naptera commented 1 year ago

Is your idea a new setting, a change to an existing setting, or something else? Please describe clearly: This should be more of a Quality of Life change for extending the settings as a developer.

What problem would your contribution solve? Please describe: The current format (bytestring) makes it pretty hard to add new features (e.g. there could be a "allow Legendaries" checkbox for the starters to the randomizer while being not readable by humans. Changing it to a JSON-string would make it more human readable and changable so that you don't have to open the GUI and load the ROM and Settings file again to verify what exactly you have done. It could also open up more possibilities for scripting the randomizer because you don't have to go through the code to see which byte(s) contains which setting(s).

Additional context: The idea came to my while hacking around with the codebase and implementing a Typelock mode (starters and wild Pokemon will all be of the same, specified type). I've already (more or less) finished the mode (haven't tested all of the possible combinations of settings), but it bugs me, that I had to put the type setting at the very end of the config string if I didn't want to increase all of the indexes coming after the newly added bytes and - even worse - break the settings file format for all previous versions. I will add the typelock mode as a contibution idea later but I think that this is a change of greater value for making more features easier to implement. I am aware that this will be a pretty breaking change because the old configs would not work anymore but I intend to add a feature for converting the current state of the config string to JSON while importing the settings file.

I understand that I am expected to implement this change myself in the randomizer's codebase: [x]

SilverstarStream commented 1 year ago

I don't mean to speak for the ZX developers here, but I want to add to the discussion. I want to let you know about some things in case you hadn't thought of them. I do think making the settings more flexible for future settings is a good idea.

naptera commented 1 year ago
* How do you plan to parse the JSON? There is no JSON parser in the Java 8 standard library, and the randomizer does not currently use any external libraries. (I'll mention that there are hacky workarounds for Java 8)

Ah yeah I thought about that today as well and I did notice that it doesn't depend on external libraries and that is probably a good thing (I don't have that much experience with Java but I guess dependencies are not statically linked so you would need the user to install the dependencies beforehand).

* Going by the issue with JSON, have you considered any other formats? My mind first goes to some randomizer-settings-specific text file, like a simple line-by-line text format, or line-by-line but with simple brackets so that settings can still be grouped together.

I think your idea with a simple line by line format would be nice and I admit that JSON may be a bit overkill while there are only some numbers and boolean values as settings (if I didn't miss anything else at least). Grouping would be nice as well, but I would prioritize defining and implementing a really basic settings file format to solve most of the problems the current configstring has.

* Does a settings string to JSON converter need to be made? Differences in randomizer versions already break settings files, so I don't think there's any reason to make one.

That is true, my idea was just to make a non-breaking change but I guess implementing something that (hopefully) doesn't introduce new breaking changes is good enough

As a first idea for a format I would suggest variablename = value with numbers and booleans as possible values and [a, b, ...] for lists of values. Entries would be seperated by newlines and other whitespaces will just be ignored. For blocks maybe blockname = { ... } would be reasonable to have a syntax close to JSON or structs in general.