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

[Menu] Have "Save Controller Profile" say: Saved "[Controller driver directory]/[File]" in the Controller Profile Directory. #16121

Closed
davidhedlund opened this issue Jan 15, 2024 · 19 comments · Fixed by #16267
Labels
bug: menu Any issues related to the menu system.

Comments

@davidhedlund
Copy link

davidhedlund commented Jan 15, 2024

Current

Settings -> Input -> RetroPad Binds -> Port 1 Controls -> Save Controller Profile says "Controller profile saved successfully.", see bottom left on the screenshot:

image

Feature request

I suggest that Settings -> Input -> RetroPad Binds -> Port 1 Controls -> Save Controller Profile should say "Saved new controller profile to [controller profile directory]/[device driver]/[$input_device].cfg.".

Image manipulate example of the bottom of the screenshot:

This will make sure that people find the .cfg file, which is needed if they want to upload it for retroarch-joypad-autoconfig.

I spent about 24 hours coding a bash script used to generate dedicated autoconfig files, since I thought that "Save Controller Profile" was saved to retroarch.cfg. My script is useless since "Save Controller Profile" is actually already generating dedicated autoconfig files. However, this could have been prevented if the pop-up notification included the file path name. With that said, it's important that this issue becomes fixed to prevent other people from being confused in that want to upload autoconfig files. This issue is listed in:

Main Menu -> Configuration -> Save Current Configuration displays [directory/file] so its source code can be reused.

Proposed 1 (new)

Example:

  • Saved "udev/Sony Interactive Entertainment DualSense Wireless Controller.cfg" in Controller Profile directory.

image

Proposed 2 (old)

image

Related issues

Smartphones with average screen size are too narrow, so this issue must be solved to make display the full text:
#16147

Version/Commit

You can find this information under Information/System Information

  • RetroArch: 1.16.0

Environment information

  • OS: [The operating system you're running]
  • Compiler: [In case you are running local builds]
@davidhedlund
Copy link
Author

@davidhedlund davidhedlund changed the title Save Controller Profile: Add "Saved new config to .../[Name of the Controller].cfg Save Controller Profile: Add "Saved new config to .../[Name of the Controller].cfg" Jan 15, 2024
@davidhedlund
Copy link
Author

davidhedlund commented Jan 15, 2024

@RobLoach Since it costed me at least 24 hours of development time to make a script that turned out to be useless since RA already can do this, can we please implement this ASAP?

Also, it will help all users to find where the generated .cfg file is, which will prevent them from creating .cfg files from scratch by scraping variables from retroarch.cfg.

@davidhedlund davidhedlund changed the title Save Controller Profile: Add "Saved new config to .../[Name of the Controller].cfg" [TOP PRIORITY] Save Controller Profile: Add "Saved new config to .../[Name of the Controller].cfg" Jan 15, 2024
@RobLoach
Copy link
Member

RobLoach commented Jan 15, 2024

It saves the controller profile autoconfigs when you select Save to Controller Profile.

Screenshot_20240115-080520.png

Configuration File > Save config will save RetroArch.cfg, with your controller settings. We could add the file that it saves in the description too.

Is there anything that appears in the console logs when run with --verbose?

@davidhedlund
Copy link
Author

davidhedlund commented Jan 15, 2024

We could add the file that it saves in the description too.

"Main Menu -> Configuration File -> Save Current Configuration" already show where the file is saved:

image

However, "Settings -> Input -> RetroPad Binds -> Port 1 Controls -> Save Controller Profile" does not mention where the file is saved, so I propose that. This is what this issue is about.

Is there anything that appears in the console logs when run with --verbose?

No, I don't get any output in the terminal when I:

  • /RetroArch-Linux-x86_64.AppImage --verbose
  • Settings -> Input -> RetroPad Binds -> Port 1 Controls -> Set all Controls
  • Settings -> Input -> RetroPad Binds -> Port 1 Controls -> Save Controller Profile

@davidhedlund davidhedlund changed the title [TOP PRIORITY] Save Controller Profile: Add "Saved new config to .../[Name of the Controller].cfg" [TOP PRIORITY] [feature request] Save Controller Profile: Add "Saved new config to .../[Name of the Controller].cfg" Jan 15, 2024
@RobLoach RobLoach changed the title [TOP PRIORITY] [feature request] Save Controller Profile: Add "Saved new config to .../[Name of the Controller].cfg" [Menu] Have "Save Controller Profile" say what file it saved Jan 15, 2024
@RobLoach RobLoach added the bug: menu Any issues related to the menu system. label Jan 15, 2024
@davidhedlund
Copy link
Author

davidhedlund commented Jan 15, 2024

Thank you very much for adding the label!

@davidhedlund
Copy link
Author

@davidhedlund
Copy link
Author

davidhedlund commented Feb 10, 2024

@zoltanvb It should be very easy to display the saved file name to begin with. What do you think about it?

This will help people to find the autoconfig, which also is helpful for people who want to contribute to https://github.com/libretro/retroarch-joypad-autoconfig

@zoltanvb
Copy link
Contributor

I am investigating the issue, but got a bit stuck when trying to find out why sublabels do not appear in normal binds, only in quick menu :) Anyway, it is not forgotten, I just want to explore a few options for displaying it, as the popup messages are fading quick and may not help a casual user too much identifying a possibly quite long file name.

@davidhedlund
Copy link
Author

davidhedlund commented Feb 15, 2024

@zoltanvb It should be very easy to display the saved file name to begin with. What do you think about it?

Sorry, the directory path must be included as well, in order to distinguish it if the same config file name occur in multiple directories, which sometimes happen: This should be commented in the source code file(s) for "Save Controller Profile".

Do you know which source code file that are involved when "Save Controller Profile" is activated to display "Controller profile save successfully."? @zoltanvb

@zoltanvb
Copy link
Contributor

The message display is here:

runloop_msg_queue_push(

zoltanvb added a commit to zoltanvb/RetroArch that referenced this issue Feb 15, 2024
Due to the port-specific indexes, sublabels for these entries
are handled specially. Some simplification/generalization  was
applied and hand-crafted string joining was removed from a few
places, though it remains still in other places.

Preparation for libretro#16121 (this commit does not do anything yet with
file names).
@davidhedlund
Copy link
Author

The message display is here:

runloop_msg_queue_push(

Thank you.

@davidhedlund
Copy link
Author

davidhedlund commented Feb 15, 2024

@zoltanvb Does your commit add "[dir]/[file]" to the pop-up notification message seen at the bottom of the screenshot (copied from the top post)?:

image

Main Menu -> Configuration -> Save Current Configuration does that, so its source code could be reused for Settings -> Input -> RetroPad Binds -> Port 1 Controls -> Save Controller Profile

@zoltanvb
Copy link
Contributor

No, it does not. I want to get some feedback with the PR before building on top of it.

Unfortunately it is not simple to reuse that, config saving is implemented via command structure, controller profile saving is direct.

@davidhedlund
Copy link
Author

davidhedlund commented Feb 16, 2024

No, it does not. I want to get some feedback with the PR before building on top of it.

Unfortunately it is not simple to reuse that, config saving is implemented via command structure, controller profile saving is direct.

Oh, I see.

Once this has been fixed, I'll take a screenshot of the pop-up notification message and upload it to https://www.retroarch.com/index.php?page=controller-autoconfig, in order to visually guide people how to find the generated autoconfig file (since this issue killed 24 hours of useless Bash scripting for me).

I deeply appreciate your contributions to solve this issue!

@odditude42
Copy link

recommendation (if this isn't already implemented elsewhere) - i'd suggest generally using "saved %file% to %path%" instead of "saved %path%/%file%" - this way, the most significant info is at the front of the notification.

@davidhedlund
Copy link
Author

davidhedlund commented Feb 17, 2024

recommendation (if this isn't already implemented elsewhere) - i'd suggest generally using "saved %file% to %path%" instead of "saved %path%/%file%" - this way, the most significant info is at the front of the notification.

Yeah. See:

@RobLoach
Copy link
Member

File paths often get really ugly as demonstrated in #16121 (comment) . Could we keep this user-friendly and say "...in the Controller Profiles Directory" instead? If people want to know where that is, they can visit the Directory Settings.

@davidhedlund
Copy link
Author

davidhedlund commented Feb 17, 2024

File paths often get really ugly as demonstrated in #16121 (comment) . Could we keep this user-friendly and say "...in the Controller Profiles Directory" instead? If people want to know where that is, they can visit the Directory Settings.

I added this to the top post as a new suggestion:

image

@davidhedlund davidhedlund changed the title [Menu] Have "Save Controller Profile" say what file it saved [Menu] Have "Save Controller Profile" say: Saved "[Controller driver directory]/[File]" in the Controller Profile Directory. Feb 17, 2024
LibretroAdmin pushed a commit that referenced this issue Feb 18, 2024
* Add sublabels for "port x controls" entries

Due to the port-specific indexes, sublabels for these entries
are handled specially. Some simplification/generalization  was
applied and hand-crafted string joining was removed from a few
places, though it remains still in other places.

Preparation for #16121 (this commit does not do anything yet with
file names).

* Further simplifications around joypad_index

Removed a few labels, now unused, and the left/right check, as it
fell back to the default which is given anyway in advance.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug: menu Any issues related to the menu system.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants