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.

@yxkalle
Copy link
Contributor

yxkalle commented Jan 14, 2025

I like it. It's optional. If you dont want an ini hell it's up to the end user, right?

@sorgelig
Copy link
Member

I like it. It's optional. If you dont want an ini hell it's up to the end user, right?

Nope. I will have more "bug" reports because options will come from different files and users won't aware of that.
Ini file already has a default section.

@steverobb
Copy link
Contributor Author

steverobb commented Jan 15, 2025

If users don't use the feature, they won't know the difference and so they won't have any new issues from it. If they do use the feature, it will be by their own use, and so naturally they will be aware of it.

There is already support for different .ini files and there is no sharing of options between them. This is not about sharing settings between different games/cores - which is what the default [MiSTer] section does - this is about sharing settings across multiple .ini files, for which the default section is no use and for which is there is no alternative available.

If switchable .ini configs were instead all under the single MiSTer.ini rather than separate alt files, then that would be another option which could work, e.g:

; MiSTer.ini

; Default values
[MiSTer]
setting_1=5
setting_2=string

; This would normally be in MiSTer_Alt1.ini, along with a copy of the defaults
[MiSTer_Alt1]
setting_1=10

; This would normally be in MiSTer_Alt2.ini, along with a copy of the defaults
[MiSTer_Alt2]
setting_2=altstring

... and so you'd get this:

Main config:
    setting_1=5
    setting_2=string

Alt1 config:
    setting_1=10
    setting_2=string

Alt2 config:
    setting_1=5
    setting_2=altstring

Is there any appetite for that kind of change?

@sorgelig
Copy link
Member

Alternative files are not for just different default values.
I don't see a problem to copy default values to alternative INI files instead of putting them in one as in your example.
You may group default values you never change to a separate block from values you change in alternative INI files.
There are ways to organize options in existing system.

If users don't use the feature, they won't know the difference and so they won't have any new issues from it. If they do use the feature, it will be by their own use, and so naturally they will be aware of it.

many users download packs with pre-made settings so they may not aware what they are using.

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.

4 participants