mupen64plus.sh: use extracted configVersion in GlideN64 config section #3688
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We already extract GlideN64 config version. Use it to finally set right config version in mupen64plus package script. Mupen64plus startup script should do this as well, but it is not wrong to do it here. -set right GlideN64 config version in "$config.rp-dist" -set right GlideN64 config version in "$config" because GlideN64 overwrites user settings if config version is wrong fixes: RetroPie#3654
joolswills
reviewed
Mar 23, 2023
| @@ -321,6 +321,10 @@ function configure_mupen64plus() { | |||
| # on the rp-dist file. This preserves any user configs from modification and allows us to have | |||
| # a default config for reference | |||
| if [[ -f "$config" ]]; then | |||
| # patch existing mupen64plus config because Gliden64 overwrites GlideN64 user configs | |||
| # if configVersion is wrong | |||
| grep -q "configVersion" "$config" && iniConfig " = " "" "$config" && iniSet "configVersion" "$(cat $md_inst/share/mupen64plus/GLideN64_config_version.ini)" | |||
There was a problem hiding this comment.
This isn't a good one-liner imho. It would be more readable to split it up and have inside an if
Also duplicating the path - keeping the config in a variable would be better.
Also note you can do $(<FILE) instead of cat.
|
Related to this, I don't like the logic and ini file handling in the launch script code. For GLideN64 I think we should just make sure there is a section present. On non RPI systems this will be generated on first launch using GLideN64, but we could create a minimal config in configure_mupen64plus with the configVersion. Probably we should handle all configs that the startup script can handle and make sure they are already set by default. However initially, just removing the config version handling would be a good start. I'm going to submit a PR for this (Also as I had assigned the original issue to myself). |
joolswills
added a commit
to joolswills/RetroPie-Setup
that referenced
this pull request
Mar 23, 2023
Add missing "local source" in install_mupen64plus Set configVersion from GLideN64_config_version.ini in $md_conf_root/n64/mupen64plus.cfg as well as mupen64plus.cfg.rp-dist Add defaults and values handled by startup script for Video-GLideN64 and Video-Rice to default mupen64plus.cfg and re-order logic. Remove logic in mupen64plus.sh startup script to set configVersion and only change values if they are present in the config (Rather than checking for a section and then adding it) Fixes RetroPie#3654 and closes RetroPie#3688
joolswills
added a commit
to joolswills/RetroPie-Setup
that referenced
this pull request
Mar 23, 2023
Add missing "local source" in install_mupen64plus Set configVersion from GLideN64_config_version.ini in $md_conf_root/n64/mupen64plus.cfg as well as mupen64plus.cfg.rp-dist Add defaults and values handled by startup script for Video-GLideN64 and Video-Rice to default mupen64plus.cfg and re-order logic. Remove logic in mupen64plus.sh startup script to set configVersion and only change values if they are present in the config (Rather than checking for a section and then adding it) Fixes RetroPie#3654 and closes RetroPie#3688
|
This needs some testing - feedback welcome #3696 (And some of the original launch code auto conf work was yours I believe). |
joolswills
added a commit
to joolswills/RetroPie-Setup
that referenced
this pull request
Mar 23, 2023
Add missing "local source" in install_mupen64plus Set configVersion from GLideN64_config_version.ini in $md_conf_root/n64/mupen64plus.cfg as well as mupen64plus.cfg.rp-dist Add defaults and values handled by startup script for Video-GLideN64 and Video-Rice to default mupen64plus.cfg and re-order logic. Remove logic in mupen64plus.sh startup script to set configVersion and only change values if they are present in the config (Rather than checking for a section and then adding it) Fixes RetroPie#3654 and closes RetroPie#3688
joolswills
added a commit
to joolswills/RetroPie-Setup
that referenced
this pull request
Mar 23, 2023
Add missing "local source" in install_mupen64plus Set configVersion from GLideN64_config_version.ini in $md_conf_root/n64/mupen64plus.cfg as well as mupen64plus.cfg.rp-dist Add defaults and values handled by startup script for Video-GLideN64 and Video-Rice to default mupen64plus.cfg and re-order logic. Remove logic in mupen64plus.sh startup script to set configVersion and only change values if they are present in the config (Rather than checking for a section and then adding it) Fixes RetroPie#3654 and closes RetroPie#3688
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We already extract GlideN64 config version. Use it to finally set right config version in mupen64plus package script. Mupen64plus startup script should do this as well, but it is not wrong to do it here. -set right GlideN64 config version in "$config.rp-dist" -set right GlideN64 config version in "$config" because GlideN64 overwrites user settings if config version is wrong fixes: #3654