UltraStar-Deluxe / Play

Free and open source singing game with song editor for desktop, mobile, and smart TV
https://ultrastar-play.com
MIT License
398 stars 74 forks source link

implement storing and loading game settings #6

Closed basisbit closed 5 years ago

basisbit commented 6 years ago

Actual behaviour

Changes to game settings are lost after user closes the game.

Expected behaviour

The game stores changes to settings. Settings survive game updates.

Steps to reproduce

  1. Open the game
  2. Change a setting in settings menu
  3. Restart the game and check if the previous change survived.
pawciut commented 6 years ago

I have a few questions before I get started

Question1: Is there a documentation of settings(list of available setting keys with parameters)? If not I can create such a list based on ESettings.cs and initial values in GameSettings.cs, but where to put it, wiki?

Question2: My plan would be to implement this feature using persistentDataPath but what kind of data format would you prefer xml, json?

Question3: Are we planning to add some backward compatibility ( f.e in version 1.0 there is ESetting.TopScores and in version 1.1 we renamed it to ESettings.HighScores) Do we want to copy old settings values and fix the settings file to new version(contain only currently supported setting keys) or we just set it to default if we didn't find it in old settings file?

Question4: Does settings get saved after any changes or should user decide when he want to save changes or restore defaults?

Question5: Is there a template/style sample for settings value inputs (labels and textboxes, sliders,etc) because I didn't find any, so I'm guessing I should come up with something, correct?

basisbit commented 6 years ago

wow, so many questions, and this project just started a couple of days ago 😄

  1. there is no document available anywhere yet. The settings stuff in this repository is just a first draft, based on the settings which are supported by UltraStar Deluxe. Best place to find all the keys and parameters there would be https://github.com/UltraStar-Deluxe/USDX/blob/master/src/base/UIni.pas I guess using Enums for all these settings would make the code easier readable. Yes, the wiki would fit (if we want to follow the contributing.md and write this as documentation mainly for users of the game, not just for developers.
  2. persistentDataPath sounds good. regarding format, feel free to use what would be easiest to implement / best for you. Honestly, even serialized binary object would be kind of ok, if you can implement it so that it roughly supports future additions / changes to the settings
  3. No need to be able to support parsing settings files from UltraStar Deluxe. Not sure about our own future settings, but I guess there eventually will be some changes to settings where we want to support reading from old settings (folder displaying yes/no, sorting, microphone recording settings and what microphone shall be mapped to what player) so people don't have to reconfigure everything once the game installed some small update. Personal opinion: yolo, just set it to defaults if no valid value. This will also force us to not change all the settings again and again at each version.
  4. "keep it simple & stupid" -> save&apply at every change
  5. so far there is no template/style thing. For a first draft, just open current UltraStar Deluxe and take what you like. Here once again, "keep it simple & stupid". For this new sing-along karaoke game, we should probably focus a lot on reducing game complexity, thus also reducing amount of options to improve user experience out of the box.

When looking at Performous or Stepmania, you might see that too many changeable settings easily frustrate new or occasional users of such software. Thus, at least initially for the first versions until we have some users feedback, please don't add "all the possible settings" to the UI yet.

barbeque-squared commented 5 years ago

I messed around with a settings implementation in a branch on my fork. It's far from finished yet, but the goals I set myself were:

To achieve this, it uses Unity's PlayerPrefs thing and a bunch of abstract classes so that, for example, having a setting for Language literally means creating an enum class for the possible values (duh) and a class to actually use that as a setting, like this:

public class DisplayLanguageSetting : SimpleEnumSetting<ELanguage>
{
  protected override ELanguage GetDefaultValue()
  {
    return ELanguage.English;
  }
}

The parent classes have some public methods to Get() or Set(ELanguage newLanguage) the value, and Set(...) also immediately saves. There is also a more complex example of a List setting (for the song directories) that uses XML (de)serialization, though the XML serialization can probably handle pretty much everything you throw at it. In terms of upgradability, for more complex things (like per-microphone settings): not sure on that, though worst-case you just create a bunch of really tiny settings that take into account some microphone-specific prefix, that way your deserializers will not break when adding new settings for them.

Some measure of error handling should also still be added (example: French used to be a supported language, but now it isn't anymore, it should just default back to whatever is the current default).

Numeric settings can probably also have stuff like minimum, maximum and step things.

I'm not sure if some/all of the current setting methods (like Get and Set) should be made static or not, assuming that is even possible.


My own thoughts on the 5 questions and answers in the above posts:

  1. We should probably not try to mirror USDX too close. Things like microphone threshold and boost should really be per-microphone settings. I'd use it as inspiration.
  2. Obviously I went a different route, but my route might not be the best
  3. See earlier in this comment. My current implementation relies mostly on class names. You can move the class file around however you want (I threw them into separate folders per option screen).
  4. My current implementation does this on Set (or, in the case of the List one, also on Add and, if necessary, on Remove)
  5. No comment (I'm not into GUI stuff).

As an aside, there should probably be a 'factory reset everything' command line option that just deletes any and all saved settings, regardless of whether they're still relevant or from some really old version. Maybe in a later version we'd split that into a 'wipe settings' and a 'wipe all highscores/players', but for a first version just deleting everything is fine I think.

barbeque-squared commented 5 years ago

I did some more work on this, and even wrote a few tests, in my fork. As a proof of concept and extensibility, I think it does a fine job, though I couldn't get the IList implementation to work (I don't understand the IEnumerator methods).

Although I'm not too fond of the way it's currently done, also notice that the SimpleEnumSetting will survive game upgrades. It isn't too hard to do something similar for numeric values as well (min/max/step), but this is the point where I'd like some general feedback first before implementing half a dozen classes.

achimmihca commented 5 years ago

This has been done in #57