search

libretro / RetroArch

Cross-platform, sophisticated frontend for the libretro API. Licensed GPLv3.
http://www.libretro.com
GNU General Public License v3.0
10.23k stars 1.82k forks source link

RetroArch does NOT display `Named_Boxarts` when it contains the ampersand `&` character #8700

Open Europia79 opened 5 years ago

Europia79 commented 5 years ago

Description

RetroArch does NOT display Named_Boxarts when it contains the ampersand & character.

Expected behavior

RetroArch seems to retrieve the picture based on matching file names: i.e. matching the name of the ROM file with the name of a corresponding .png image located in the /thumbnails/ directory.

Actual behavior

RetroArch does NOT display Named_Boxarts when it contains the ampersand & character.

Steps to reproduce the bug

  1. Add the ampersand & character to the name of any rom file.
  2. Add ROM to playlist.
  3. Add .png with same name under thumbnails/{playlist}/Named_Boxarts/
  4. Under Settings -> User Interface -> Appearance set Thumbnails and/or Left Thumbnails to Boxarts setting.
  5. Goto playlist, goto the game, & check for the missing boxart display.

Example file names (with .bin & .png extensions): Batman - The Adventures of Batman & Robin Battletoads & Double Dragon D&D - Warriors of the Eternal Sun Mickey & Minnie - The Great Circus Mystery Mickey Mouse & Donald Duck - World of Illusion Might & Magic I - Gates to Another World Might & Magic III - Isels of Terra Puzzle & Action - Ichidanto-R (Japan) Puzzle & Action - Tanto-R (Japan) Sonic & Crackers (Proto) Sonic & Knuckles + Sonic 1 Sonic & Knuckles + Sonic 2 Sonic & Knuckles + Sonic 3 SonicHack - Sonic & Tails - Double Trouble SonicHack - The S-Factor - Sonia & Silver Spider-Man & Venom - Maximum Carnage Spider-Man & Venom - Separation Anxiety Spider-Man & X-Men - Arcade's Revenge Streets of Rage II - BattleToads & Double Dragon (Hack) ToeJam & Earl I ToeJam & Earl II - Panic on Funkotron

Several of these literally have the ampersand & symbol in the name as clearly shown on the boxart.

Version/Commit

Environment information

orbea commented 5 years ago

Can you please try the master to see if this issue is still reproducible?

Europia79 commented 5 years ago

@orbea I didn't build it. I just downloaded it from https://www.retroarch.com/?page=platforms

Would it be a sufficient test to re-download & re-install ?

Or do you have a continuous integration server ?

orbea commented 5 years ago

Yes, downloading the latest nightly would suffice.

RobLoach commented 5 years ago

Are & replaced with _?

Europia79 commented 5 years ago

Bug still exists with git version 9750719074 build date Feb 3 2019

If you have a more up-to-date download, please link.

Europia79 commented 5 years ago

@RobLoach This is Named_Boxarts ...meaning you can name it anything you want as long it's valid system characters for file names. So, for example, if you want to add a ROM Hack, then you can also add corresponding .png to /thumbnails/{playlist}/Named_Boxarts/ with the same name... except, for whatever reason, it's not working if the files have the ampersand & symbol in the file names.

RobLoach commented 5 years ago

What I mean is... Does this work?

/thumbnails/{playlist}/Named_Boxarts/Batman - The Adventures of Batman _ Robin.png
Europia79 commented 5 years ago

@RobLoach Yes, that works. But why doesn't it just match the file name like every other ROM ? Why the convoluted work-around ?

RobLoach commented 5 years ago

Ampersands have been problematic on some platforms. Here's some docs on which characters are replaced... https://github.com/libretro-thumbnails/libretro-thumbnails#usage

RobLoach commented 5 years ago

@Europia79 Can this be closed? Is there a better place where we could document the characters that are replaced for thumbnails?

Europia79 commented 5 years ago

@RobLoach Oh, I thought you were going to fix this. If you're not, then at least give me or someone else a chance to look thru the code first. I mean, I don't understand the behavior ? Why not just fetch the thumbnail by file name, as one would assume RetroArch is doing ? I don't think we need special cases. Just do the comparison and have the Operating System enforce valid and invalid characters ? For example, on Windows, these characters aren't valid for file names: \ / : * ? " < > |

But the ampersand & character is completely valid.

For example, me (or someone else) could release and Operating System tomorrow and say that (just as an example) the exclamation point ! is not a valid character for file names. So... then... will RetroArch really handle this special case ? In my mind, it doesn't need to. It just has to fetch the thumbnail... no special handling is required. Let the OS do its job.

So... Imma look thru the code. Unless you already know where the code in question lives ? If so, it should be an easy fix. Unless there's something that I missing ?

Europia79 commented 5 years ago

https://github.com/libretro/RetroArch/blob/c662da11c3f8247e0e70ea40b142a1d054d25872/ui/drivers/qt/qt_playlist.cpp#L42

https://github.com/libretro/RetroArch/blob/c662da11c3f8247e0e70ea40b142a1d054d25872/ui/drivers/qt/qt_playlist.cpp#L165

Europia79 commented 5 years ago

https://github.com/libretro/RetroArch/blob/775c272029caa99ee669335a49cdee8896f85ea7/menu/drivers/stripes.c#L968-L971

klepp0906 commented 5 years ago

based on reading through this (and knowing jack & shit regarding code) do the last 2 comments indicate a pending fix?

I only ask because id rather live with it not displaying temporarily, then go through and change all my &'s to _'s unnecessarily if that makes sense.

orbea commented 5 years ago

@Europia79 Suggestion seems reasonable to me, but it will need someone to spend the time to actually implement and test it which may or may not be soon?

klepp0906 commented 5 years ago

soon or otherwise is much less relevant to me than knowing its coming :) you guys continue to impress and i know your to-do list has to be mind blowing. Patient as patient can be, just hoped to know one way or the other.

orbea commented 5 years ago

By all means there could be someone more informed than me that could explain why the suggestion wouldn't work out, I'm just saying I think its worth considering and fixing if possible. :)

orcutt989 commented 4 years ago

Definitely is stilll a problem. +1