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

Use either "_directory_path" or "_file_path" for related variables #16309

Open
davidhedlund opened this issue Feb 29, 2024 · 2 comments
Open

Comments

@davidhedlund
Copy link
Contributor

davidhedlund commented Feb 29, 2024

Description

In retroarch.cfg, directory variables end with "_directory", "_dir", or sometimes even "_path". This confusing since file variables names also are using "_path". So, for the sake of consistency, it would be simpler to only make them end with either "_directory_path" or "_file_path".

Expected behavior

Alternative variables that can be used in addition to the current variables:

# File paths

# Replace "_path" with "_directory_file_path"
content_favorites_path = content_favorites_file_path
content_history_path = content_history_file_path
content_image_history_path = content_image_history_file_path
content_music_history_path = content_music_history_file_path
content_video_history_path = content_video_history_file_path

# Directory paths

# Replace "_path" with "_directory_path"
bundle_assets_dst_path = bundle_assets_dst_directory_path
bundle_assets_src_path = bundle_assets_src_directory_path
cheat_database_path = cheat_database_directory_path
content_database_path = content_database_directory_path
core_options_path = core_options_directory_path
libretro_info_path = libretro_info_directory_path
video_font_path = video_font_directory_path

# Replace "_dir" with "_directory_path"
audio_filter_dir = audio_filter_directory_path
joypad_autoconfig_dir = joypad_autoconfig_directory_path
log_dir = log_directory_path
video_filter_dir = video_filter_directory_path
video_shader_dir = video_shader_directory_path

# Replace "_directory" with "_directory_path"
assets_directory = assets_directory_path
cache_directory = cache_directory_path
content_favorites_directory = content_favorites_directory_path
content_history_directory = content_history_directory_path
content_image_history_directory = content_image_history_directory_path
content_music_history_directory = content_music_history_directory_path
content_video_directory = content_video_directory_path
core_assets_directory = core_assets_directory_path
dynamic_wallpapers_directory = dynamic_wallpapers_directory_path
input_remapping_directory = input_remapping_directory_path
libretro_directory = libretro_directory_path
osk_overlay_directory = osk_overlay_directory_path
overlay_directory = overlay_directory_path
playlist_directory = playlist_directory_path
recording_config_directory = recording_config_directory_path
recording_output_directory = recording_output_directory_path
resampler_directory = resampler_directory_path
rgui_browser_directory = rgui_browser_directory_path
rgui_config_directory = rgui_config_directory_path
runtime_log_directory = runtime_log_directory_path
savefile_directory = savefile_directory_path
savestate_directory = savestate_directory_path
screenshot_directory = screenshot_directory_path
system_directory = system_directory_path
thumbnails_directory = thumbnails_directory_path

Actual behavior

[What is actually happening]

Steps to reproduce the bug

file=~/Downloads/Software/RetroArch-Linux-x86_64/RetroArch-Linux-x86_64.AppImage.home/.config/retroarch/retroarch.cfg; grep '_dir =\|_directory =\|_path =' $file | sort | grep -v 'true\|false'

assets_directory = "/usr/share/libretro/assets/"
audio_filter_dir = "/usr/lib/x86_64-linux-gnu/retroarch/filters/audio/"
bundle_assets_dst_path = ""
bundle_assets_src_path = ""
cache_directory = "/tmp"
cheat_database_path = "~/.config/retroarch/cheats"
content_database_path = "~/.config/retroarch/database/rdb"
content_favorites_directory = "default"
content_favorites_path = "~/.config/retroarch/content_favorites.lpl"
content_history_directory = "default"
content_history_path = "~/.config/retroarch/content_history.lpl"
content_image_history_directory = "default"
content_image_history_path = "~/.config/retroarch/content_image_history.lpl"
content_music_history_directory = "default"
content_music_history_path = "~/.config/retroarch/content_music_history.lpl"
content_video_directory = "default"
content_video_history_path = "~/.config/retroarch/content_video_history.lpl"
core_assets_directory = "~/.config/retroarch/downloads"
core_options_path = ""
dynamic_wallpapers_directory = "default"
input_remapping_directory = "~/.config/retroarch/config/remaps"
joypad_autoconfig_dir = "~/.config/retroarch/autoconfig"
libretro_directory = "/usr/lib/x86_64-linux-gnu/libretro/"
libretro_info_path = "/usr/share/libretro/info/"
log_dir = "~/.config/retroarch/logs"
osk_overlay_directory = "~/.config/retroarch/overlays/keyboards"
overlay_directory = "~/.config/retroarch/overlays"
playlist_directory = "~/.config/retroarch/playlists"
recording_config_directory = "~/.config/retroarch/records_config"
recording_output_directory = "~/.config/retroarch/records"
resampler_directory = ""
rgui_browser_directory = "~/"
rgui_config_directory = "~/.config/retroarch/config/"
runtime_log_directory = "default"
savefile_directory = "~/.config/retroarch/saves"
savestate_directory = "~/.config/retroarch/states"
screenshot_directory = "~/.config/retroarch/screenshots"
system_directory = "~/.config/retroarch/system"
thumbnails_directory = "~/.config/retroarch/thumbnails"
video_filter_dir = "/usr/lib/x86_64-linux-gnu/retroarch/filters/video/"
video_font_path = ""
video_shader_dir = "~/.config/retroarch/shaders"

Bisect Results

[Try to bisect and tell us when this started happening]

Version/Commit

You can find this information under Information/System Information

  • RetroArch: 1.17.0 Appimage

Environment information

  • OS: GNU/Linux
  • Compiler: [In case you are running local builds]
@davidhedlund davidhedlund changed the title Replace "_dir" and "_path" with "_directory" for directory variables Replace "_dir" and "_path" with "_directory" for directory variables, and "_path" with "_file" for file variables Feb 29, 2024
@davidhedlund davidhedlund changed the title Replace "_dir" and "_path" with "_directory" for directory variables, and "_path" with "_file" for file variables Use either "_directory_path" or "_file_path" for related variables Mar 1, 2024
@cmitu
Copy link
Contributor

cmitu commented Mar 2, 2024

Changing the configuration variables naming has the potential of breaking countless existing configurations, it shouldn't be done just for naming's sake.

@RogueScholar
Copy link

Changing the configuration variables naming has the potential of breaking countless existing configurations, it shouldn't be done just for naming's sake.

@cmitu, I respectfully disagree with that line of thought. This is a bug that has needed squashing for far too long now, and rather than shoot down the proposal we need to have a mindset of how we can finally get it done. The RetroArch main configuration file is a complete disaster area, riddled with unclear/unhelpful comments, needless duplication and derivative keys, and as has been well-illustrated here, a completely haphazard nomenclature for the variables themselves. I can't really think of a config file I like less or am more often frustrated by in the entire Linux software ecosystem.

I've never taken much of a cotton to the "better to leave something broken than fix it in a way that requires user involvement" philosophy to begin with, and here I'm not even convinced that those are the only available options. As for potential paths forward, I can imagine:

  • A limited time period where migration code is added to Retroarch, wherein it handles the migration from old keys to new keys on first run after upgrade. After two or three release cycles the migration period would end and that code could be pulled back out to ensure no technical debt accrues to the project.

  • Carefully examine the entire corpus of configuration keys and identify the very worst of them (the ones that make the "durrrrr" sound in your head every time you see them) and add permanent aliases for them in the configuration parser that will remain in perpetuity. Update documentation and default config files and allow the old key names to remain functional ad infinitum.

  • Resolve to completely rethink the way RetroArch approaches configuration so that it's nothing like the status quo of three dozen different files in a shopping basket of directories and becomes such an obvious upgrade for users going forward that they have sufficient investment to handle the migration on their own with the support of a really good reference document.

  • Lastly, just update the key names and accept that some configurations will break, users will bitch and moan for a little while, then eventually fix it via a combination of word of mouth on forums and group chats and soon forget about all of it. The vast majority of people living in the developed world right now have spent the bulk of their lives dealing with software and understand that "shit just breaks sometimes" and that it requires some Googling to learn why and how to straighten it back out.

    Remember that this is a gaming emulator, not a pacemaker for weak hearts or an access mechanism to people's life savings…and they didn't even have to pay for it! If it costs them five or ten minutes in a text editor so we can take something bad and make it at least halfway decent, or maybe even awesome, then I'd ultimately view it as giving users a chance to have some skin in the game. (If you couldn't tell by now, this last option's my huckleberry. 😉)

Great bug report, @davidhedlund, and solid proposed changes, too; this has been something I've been wanting to have a discussion about for a long time. If it manages to get past the naysayers, count me in to pitch in on the implementation.

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

No branches or pull requests

3 participants