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

#include support for .ini files. #937

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

steverobb
Copy link
Contributor

This change allows settings from one .ini file to be imported from another. The main motivations for this are:

  • Users and the Main_MiSTer repo both update MiSTer.ini, meaning users needs to manually merge changes alongside their own edits.
    • This change renames MiSTer.ini to MiSTerDefaults.ini. The idea is new settings added by this repo will happen in MiSTerDefaults.ini, but users will add their settings to MiSTer.ini and MiSTer_AltN.ini and #include the MiSTerDefaults.ini file. Therefore they get updates, but their own settings are separate and don't need to be merged.
  • Alt ini files need to duplicate all the settings and these need to be kept in sync, even if many of these settings do not change.
    • e.g. I have an alt ini file per output display I have, but I don't want to have to keep input config settings in sync across 4 different files.

I don't know of any consistent import syntax for ini files, so I chose the common C-style #include syntax. I also considered "import=MiSTerDefaults.ini" but ultimately thought it was better to have a distinct syntax from variables.

@birdybro
Copy link
Member

birdybro commented Dec 2, 2024

Just my two cents:

  1. Sane and highly compatible defaults are already defined in code outside of the MiSTer.ini when one is not present. As such this seems kind of redundant, if more sane defaults are needed then they should be added to code instead.
  2. Wouldn't this break the ini_settings.sh script that many users are already using?

@steverobb
Copy link
Contributor Author

Hey, thanks for the feedback.

  1. It's not so much about a fear of incompatible defaults as visibility of new options that go unnoticed. If it was about changes to the defaults that already exist in code, there would be no need to ship a MiSTer.ini at all. However, I still regard that as a secondary benefit over having to sync the same settings across multiple alt inis.
  2. I wasn't aware of the ini_settings.sh script, thanks for the call out. Another direction could be to add a new MiSTer_Main.ini which #includes MiSTer.ini, and MiSTer_Main.ini becomes the primary ini, thus not requiring MiSTer.ini to be renamed at all.

@steverobb
Copy link
Contributor Author

Having a default ini which is distributed and a separate ini for user edits also seems like a possible solution to this issue:

#865

@birdybro
Copy link
Member

birdybro commented Dec 2, 2024

Communication of new ini features to the average user is currently left up to the community to do via the forums (sorgelig usually mentions this in a news comment when a new Main MiSTer binary is shipped), the discord server (the news posts are rss linked to a news channel there), and users just discussing it elsewhere (bluesky, X, youtube videos, etc...).

If a new feature were added, they would only be visible if someone went and manually checked their MiSTerDefaults.ini in your instance after an update, or if they read the update script feedback/log carefully. I personally don't see this as being more user friendly, it's kind of a wash.

I sympathize with the efforts to address this though, it's a tough cookie to solve because MiSTer not having a fancy GUI can be a challenging limitation when coming up with solutions for potential issues like this.

@steverobb
Copy link
Contributor Author

I've reverted the changes to the .ini files because I think I'm overstepping my bounds there to make such a foundational change. If I can get the #include feature in, I can use it (well, am already using it) to define whichever ini hierarchy suits my needs, without affecting other users.

@sorgelig
Copy link
Member

sorgelig commented Dec 3, 2024

Linux guys are coming. Let's make an INI hell where settings will spread through several files..
This is not original intention of INI which was made to overcome some options unable to set through OSD.
It should be kept as simple as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants