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

Thanks for allowing rename playlist, but has an issue ... #16379

Closed
Augusto7743 opened this issue Mar 21, 2024 · 8 comments
Closed

Thanks for allowing rename playlist, but has an issue ... #16379

Augusto7743 opened this issue Mar 21, 2024 · 8 comments

Comments

@Augusto7743
Copy link

Thanks for reading my topic.
Retroarch in recent version was added the feature allowing rename playlist. Thanks.
However has an problem. If you rename an playlist and even if was renamed the thumbnail path directory using both the same name not will be displayed the images thumbnails.
Example.
Playlist "Atari VCS" > Thumbnail directory "Atari VCS"
after was renamed to
Playlist "Atari 2600" ... even renaming the thumbnail directory for "Atari 2600" break showing thumbnails.

Only reporting.
Have an nice day.

@zoltanvb
Copy link
Contributor

I am not aware of playlist rename feature, anyway it is possible by renaming the playlist file. But that should not impact thumbnail paths much.
Can you provide a debug level log of browsing the non-working playlist, where on-demand thumbnail downloads are enabled? (It is not important for the thumbnails to be available online, but this will print out some detailed information this way.)

@Augusto7743
Copy link
Author

Unhappily I had done an wrong action closing that issue. Excuse me.
Doing more test the problem continue happening.
I have uploaded the log files. However I not see inside logs any detail about the thumbnails images path.

Playlist name "Nestopia" and image path directory "Nestopia > show the images
Renamed the playlist to "FC Nestopia" not changing the path directory "Nestopia" > show the images
Renamed the playlist to "FC Nestopia" changing the path directory to "FC Nestopia" > not show the images

Have an nice week.

logs.zip

@zoltanvb
Copy link
Contributor

OK, reproduced. This is as expected.
When doing manual scan with custom system name, the database entry for the playlist items will look like the playlist name:
"db_name": "Nestopia.lpl"
There is no such database, but RA internal logic will use this entry also for thumbnail lookup.

If you rename the playlist (.lpl file), this entry will remain inside the file. To adapt this, run a search-replace operation on the file with any text editor, so that it looks like this for all entries:
"db_name": "FC Nestopia.lpl"

@Augusto7743
Copy link
Author

Doing manual scan using a simple custom name using the same name for thumbnail directory.

" There is no such database, but RA internal logic will use this entry also for thumbnail lookup."
If user use compacted playlist not is possible edit the file. Not is an good alternative.
The correct good sense is RA read the thumbnail directory using only the playlist file name and not any detail inside of file. Only does problem.

Better is the solution above.

@i30817
Copy link
Contributor

i30817 commented Mar 26, 2024

Playlist rename affects the (subsequent to the rename, not before) downloaded thumbnails path because there is a conceptually missing scan_* manual playlist property corresponding to the system name. Instead, RetroArch uses the filename for that, and thats a design flaw (probably a particularly unfortunate optimization by jdgleaver - I'm extremely glad he coded the manual scanner but I wish I had complained more about certain things while he was coding it, since I was already suggesting adding the .dat filter that is so useful for mame manual scans).

Read #16031 carefully to understand, particularly the part where I talk about options to fix it and mention disentangling file name from system name.

Manual scan is too complex for its own good, with certain options providing duplicate functionality for minor performance gains or being user traps for power users to not set a single option once (the default of system name being content directory is one such a option because power users know to name those directories the names RetroArch expects but normal users will just scan whatever and not change the default and then have missing thumbnails), that then can cause problems like in #16378 where if that idea is coded, the manual scan will have to be changed to remove scan_overwrite_playlist to remove another trap card, which wouldn't be such a big deal because there is another option that doesn't delete the file but still removes the missing entries (ie scan_overwrite_playlist is just a optimization).

Tl;Dr: this is a real bug, and saying that 'this is expected' and telling people to run search replace in text editors is particularly annoying. Change the design to fix the parts that don't work! Project members being afraid to touch parts of a codebase is how you know a project is dying.

@i30817
Copy link
Contributor

i30817 commented Mar 26, 2024

If there is such code setting up thumbnail paths that exists before the playlist is parsed \ the user enters the playlist (probably the thumbnails mass download option), it should first initialize the affected playlist and extract that new property instead of system name if this decoupling of system name from filename is done. In fact, in that mass download a parse should already be happening to get the entries names.

There is a alternative of instead of saving the system name save the display playlist name, but that's strictly worse, be cause it would force a parse of all playlists on RA start just to display names, instead of a additional parse of a single playlist in certain situations where they would already be parsed to get the names (like downloading thumbnails).

@i30817
Copy link
Contributor

i30817 commented Mar 26, 2024

Thinking about it some more, the hypothetical scan_system_name should just be system_name, ie: be a property of all playlists, not just manual playlists (except history).

The reason is that you also want to be able to rename normal playlists and get new thumbnails after.

@Augusto7743
Copy link
Author

@i30817
Thanks for sharing #16031 and #16378 .

"this is a real bug, and saying that 'this is expected' and telling people to run search replace in text editors is particularly annoying"
You are correct ...
annother issue is saying about allow thumbnails using jpg and seeing reply saying "not is possible because the files in server are png" even if retroarch internally has jpg support =(
I had done an test converting my all png images had drop from almost 10 GB to less of 2 GB !
Totally no sense not allowing use jpg in thumbnails because "not has jpg files in server".

In moment is better create the playlist using the correct naming and avoid renaming.
Have an nice week.

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

4 participants