libretro / RetroArch

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

Bug with history showing double and no thumbnails when launched from shortcut #8635

Open klepp0906 opened 5 years ago

klepp0906 commented 5 years ago

First and foremost consider this:

Description

Launching a rom directly from a steam shortcut (made with SRM fwiw) adds a second (double) history entry which lacks the thumbnail relative to launching from directly within RA which adds the same entry in history but with the thumbnail.

Expected behavior

Whether launched from within RA or launched from a shortcut, only 1 entry per game which includes the thumbnail.

Actual behavior

See the following images. The first shows the history entry added by a shortcut launch (no thumbnail showing) https://www.dropbox.com/s/tpz2p29i1p8ceyi/no%20thumbnail.png?dl=0

The second shows the history entry added by launching within RA which not only adds a double of the same item, but includes the thumbnail whereas the aforementioned does not. https://www.dropbox.com/s/wpaz5acv3z05zfa/thumbnail.png?dl=0

Steps to reproduce the bug

  1. Select a game from your playlist (presumably by navigating to directory will work as well) and start it so it displays in your history properly showing the thumbnail.
  2. Add a shortcut to steam to launch a rom directly from RA manually or using SRM using a CLA for example in my case: -f -L "F:\Emulators\RetroArch\cores\genesis_plus_gx_libretro.dll" "${filePath}"
  3. launch rom via shortcut, exit, and note that an identical entry to the first (ra launched) is added to history, however this time with no thumbnail.

This being fixed would be monumental to me and my continued use of RA. I know that likely means little in the grand scheme but I figured id mention it. Ive tried everything I can think of to work around it/get it functioning but best I can tell it will take the professionals lol:P

Bisect Results

Has always been the case afaik

RetroArch: 1.7.6

Environment information

Windows 10

orbea commented 5 years ago

I can reproduce this in linux with the following.

  1. Start any game from the menu so it ends up in the history list.
  2. Start the same content/core from the command-line with: retroarch -L /path/to/core /path/to/content
  3. A duplicate entry is added to the history playlist.

@jdgleaver This one might be easy for you when you have a chance. :)

klepp0906 commented 5 years ago

minor! minor you say! You must not be ocd ;p

but in all seriousness, thank you for looking into it. Ive dealt thus far, I can deal until its addressed. Ive been putting a much more substantial amount of time into curating an RA setup and this is one of those things ;p

I imagine watching this thread will be the best way to be updated as to when it gets fixed?

orbea commented 5 years ago

I tagged it minor because its annoying more than anything else rather than something like a crash which is just a deal breaker. Though I agree it would be nice to see this fixed finally!

jdgleaver commented 5 years ago

Regarding the duplicate entries - this can happen in two ways:

1) Content path in playlist is different from content path specified on command line

2) Core path in playlist is different from core path specified on command line

If you post the duplicate entries from your content_history.lpl file, I can tell you which one it is. :)

Regardless, (2) is a very easy fix. @orbea I can't make a PR from my phone, but if you want to fix this yourself you can just change the following:

https://github.com/libretro/RetroArch/blob/60c028a5627444258a38ae40d023927a1fc9490d/playlist.c#L439

becomes:

if (!string_is_equal(path_basename(playlist->entries[i].core_path), path_basename(core_path)))

and:

https://github.com/libretro/RetroArch/blob/60c028a5627444258a38ae40d023927a1fc9490d/playlist.c#L554

becomes:

if (!string_is_equal(path_basename(playlist->entries[i].core_path), path_basename(entry->core_path)))

...otherwise I'll do it when I get back on Monday.

For (1), we'll probably need to wrap the content path comparison with a path_resolve_realpath(). (Can't test this until I can compile something...)


The thumbnail issue is a different kettle of fish entirely.

In order to show a thumbnail, content must be associated with a database. When content is loaded from an existing playlist, this is implicit - it's either the existing db_name value, or the name of the playlist itself. When loading something from the command line, however, this information is entirely absent.

The best we can do is add another command line option for specifying a database name (which will then be copied to the content history playlist). I'm interested in sorting this out, since it really hurts the console ports (e.g. 3DS) which don't have shared library support, and rely on forking to load content - i.e. everything is in effect launched via the command line, so content history can never have thumbnails. I have a huge TODO list at the moment, though, so it'll get done when it gets done... :)

klepp0906 commented 5 years ago

Hi again, and thanks a TON of looking into this. Probably seems trivial to many considering how many probably dont use steam to launch games etc.

Via my exceptionally limited understanding of such things - I did assume it may be due to the path to the rom, as I point the parser for steam shortcuts to symlinks in a folder especially for the purpose of streaming to the tv etc.

Alas, i deleted them, and recreated them pointing to the same directory/directly to the rom - same as the ones scanned into RA.

The result was the same. Double entry, no thumbnail.

here is a quick content history for your professional viewing :) It appears that the shortcut via steam points to the zip whereas launching from RA pre-opens the zip so to speak. I'm sure you know what im talking about without me saying so lol. You'd know what the best course of action is :) I just hope it doesnt consist of having to unzip my entire rom collection as the parser (SRM) cant look inside archives of course :(

https://www.dropbox.com/s/rxf0vqxwa3tq4wl/content_history.lpl?dl=0

thank you again, sincerely

EDIT - as per that last portion (sorry, admittedly skimmed due to excitement of seeing activity lol) - of course! I cant fathom the "list" you guys have. Considering that, priorities within RA, RL, and the fact your doing this at no cost to me or anyone else....not to mention in the sake of being a decent human being - take your time for sure :) I'll keep a patient eye here!

jdgleaver commented 5 years ago

Ah ha!

You were probably affected by having symlinks as well (which a path_resolve_realpath() wrapper should fix), but yes, the real issue here is the disparity between [zip file] vs [zip file]#[content].

Hmm... This is a little sticky, since an archive can contain multiple roms... But it seems the sensible option would be to have the playlist 'entry exists' check just match the first archive path entry when the #[content] part is missing from the input path string. That's certainly doable, and I can sort it out next week :)

klepp0906 commented 5 years ago

BEST - CODER - EVER ^

<3

orbea commented 5 years ago

@jdgleaver I'm not sure we should do this for the core path and it might be better for only the content path?

A common use case for me is to build a core for testing inside its source tree and then use retroarch -L core_libretro.so /path/to/content while I have my main install for that core in /usr/lib64/libretro/core_libretro.so. The former core I built for testing may be removed right away after I am done testing and will not exist later. It might be problematic for any history entries which are using the latter core which I have installed? What do you think?

orbea commented 5 years ago

Also if issue https://github.com/libretro/RetroArch/issues/3237 is ever fixed we may find ourselves with more than one installed version of the same core. :)

jdgleaver commented 5 years ago

@orbea Ah, okay, I kinda see what you mean (although the history entry with the missing core will still be broken, right?).

Not a problem, though - we can just use path_resolve_realpath() on the core path (which will fix any duplicates due to symlinks), and as you say, still allow distinct entries when the core path is 'truly' different. Fixing the content path duplicates shouldn't harm anything.

I've got a chunk of RGUI stuff to do on Monday, and I'll get on this straight after. (The thumbnails addition will come a bit later - I've been stacking up all sorts of niggly things to fix while I've been away...)

orbea commented 5 years ago

Yes, removing the core or content will break the corresponding entries in the playlist, there have been times in the past when trying to load them would crash.

klepp0906 commented 5 years ago

just an update (which of the following, your probably aware now) but just in case.

I went ahead and re-did all my shortcuts in steam to point directly at the same file RA does. Still double entries, still no thumbnail. So neither thumbnails lacking, nor double entries is related to symlinks being used.

jdgleaver commented 5 years ago

Yeah, symlinks will cause the same issue, but in your case it's due to the way that zipped roms are handled.

I've gotten sidetracked with some more RGUI stuff, but I'll work on a duplicates fix hopefully later on today (tomorrow at the latest!)

klepp0906 commented 5 years ago

hey, no hurry my friend. Im just elated that it will be fixed eventually. I know ALL about sidetracking with two toddlers, let me tell ya lol.

Now you said you'll fix the duplicates first, and the thumbnails later - would this be something retroactive that when fixed would have my history then showing thumbnails, or would i need to delete it and begin a new at such a point?

jdgleaver commented 5 years ago

Heh, thanks for your patience!

Unfortunately it's not really possible to retroactively fix content history - especially regarding thumbnails, since it requires information that simply doesn't exist until the entry is entered (i.e. we can't look at an existing playlist and fill in the blanks). So yeah, unless you want to open your content_history.lpl with a text editor and input the db_name fields yourself, you'll need to start over with a fresh history (I know this sucks, but it's only a temporary pain...)

klepp0906 commented 5 years ago

somehow I think i'll survive :)

sincerely appreciate your expertise and help!

jdgleaver commented 5 years ago

@klepp0906 I forgot to post here yesterday, but here's an update:

klepp0906 commented 5 years ago

the irony is, I just spent the past few days redoing EVERY directory so it matches what retroarch uses natively LOL.

however, is it just the directory holding the file, or the path? ie: retroarch uses thumbnails/Nintendo - Super Nintendo....

whereas my roms are a games>consoles.Nintendo - Super Nintendo....

regardless, going to update now and you have my undying thanks. This was pretty important to me regardless of how silly it may seem :) I'm OCD as it is, and a sucker for polish. I feel like its one more step that removes a bit more of the "crude" that retroarch has sprinkled here or there. (i mean that with 0 offense of course)

jdgleaver commented 5 years ago

Yep - it's just the directory. So you can indeed have:

and

Glad you're happy with the fix :)

klepp0906 commented 5 years ago

Should I close this then?

not sure if there are future plans for an un "crude" (as you call it) fix, in the future that would be served by leaving this open - or if it is what it is for now and i should close to clear clutter?

jdgleaver commented 5 years ago

Hmm... I think leave it open for now. I can think of a non-crude fix for this, but it's quite involved and won't happen for a while. Leave the issue here to remind me :)

klepp0906 commented 5 years ago

will do, sounds good - and sincere thank you!

klepp0906 commented 5 years ago

@jdgleaver

whelp, i tested it. My shortcuts in steam are in fact displaying the thumbnail now. The problem is - theyre still adding a second entry to the history lol. Half way there!

jdgleaver commented 5 years ago

Aw crap...

This may still be a symlink issue - I thought the resolve_realpath() function handled this properly for Windows, but now I'm not so sure (I can only test Linux)

Can you upload your history playlist again?

I'll try and look at this tomorrow...

klepp0906 commented 5 years ago

I dont believe it is, as RA and the steam shortcuts both point to the same rom folder now. No symlink involved.

I thought it might be on the thumbnail side - so i tried a few different things. I initially had the steam shortcuts pointing to a different folder for thumbnail source. i tried the same folder as well as a symlinked folder.

All showed thumbnails, but all showed double history entries.

I assume that the thumbnail the steam shortcuts are using is irrelevant now. It will always take RA's thumbnail (from the thumbnails dir) which is ideal anyways.

If theres anything i can do to help narrow it down for ya, lmk. regardless - as usual, please take your time!

jdgleaver commented 5 years ago

Just checking - you do have Fuzzy archive matching enabled, right...? (check PR #8674 for details)

klepp0906 commented 5 years ago

you nailed it my friend. That ought to be enabled by default perhaps no? Never even crossed my mind that might be necessary. I quickly came across it but usually anything with "fuzzy" is related to inexact names (in my mind) and since everything is 1:1 (everywhere) to ensure i dont need it, assumed i did not.

but now, 1 entry, 1 thumbnail, on a per core basis in history which is exactly what i'd hoped for.

It does always use RA's thumbnails from the boxart folder irrespective of how or wehre its launched right? My steam and RA are using the same thumbnails, but they wont be for long as the steam ones are way too big for steam grids, and I dont want that to potentially cause any issues so i figured id ask now.

thanks again btw <3 good catch

jdgleaver commented 5 years ago

Ah good, glad it's working.

That ought to be enabled by default perhaps no?

The general consensus was that it shouldn't be on by default because it might (potentially) mess with users who have non-standard archived rom setups - i.e. multiple roms per archive (there's also a tiny performance hit). Also, users who run archived roms from both the menu and the command line are in a very small minority, and no one else needs the 'fuzzy matching' feature.

I guess it's debatable. I don't always get to make the decisions here :)

It does always use RA's thumbnails from the boxart folder irrespective of how or wehre its launched right?

Absolutely. It will only ever use the thumbnail path you've configured in your RetroArch options.

klepp0906 commented 5 years ago

Ah, i see. Fair enough, you are correct. It could go either way. I'll admit my use-case is likely relatively fringe.

thanks for the confirm on the thumbnail path. a weight off my shoulders and a check off my list :)

klepp0906 commented 5 years ago

@jdgleaver

for the record (im sure you know the limitations already) but just in case.)

My plan (so far ive been stuck on the first playlist for days lol) is to add my games to steam and retroarch both. The parser I use to add shortcuts to steam has an easy regex option to remove all the yucky stuff. (en,ja,it,es) or (Rev C) etc from the end of roms.

Retroarch doesnt have any means to easily sanitize the names so I'm stuck using symlinks from the images, renaming the symlinks manually in a clean fashion. Then editing the playlist file and renaming all labels to match the artwork. No small task.

I did all this only to find out that while double history still doesnt happen - if the shortcut is started from steam first, it will give me no thumbnail. I can only presume due to the label in the RA playlist being "sanitized" ugh.

So for now, it looks like im going to have to deal with either unsanitized names, or the shortcut artwork only working if started from RA first.

I put in, and have seen others put in over the years a request for that as an option in retroarch, hopefully someone does at some point. (a simple regex option in the desktop UI would be stellar) unless a GUI option that was able to remove things independently and untethered from art. (one toggle for region, one for language, one for discs, one for funny business like publisher or revision).

anyhow, digressing.

jdgleaver commented 5 years ago

@klepp0906 Yeah, we may at some point have separate label/thumbnail ID entries in playlists, but it's not as entirely straightforward as you might think (and there are plenty of other priorities!)

But it kinda sounds like you're making a meal of this. Why don't you rename your actual ROM files however you like, and then manually create RetroArch playlists (using the QT GUI, a script, or any one of the existing tools you can find online). It's only the content scanner that adds the full 'yucky' name (from a database) - if you build the playlist yourself then you can have the label match the filename.

(and to clarify - when you launch from the command line, the generated history entry uses the filename as a label - so it won't show a thumbnail if you have a mishmash of different custom labels/thumbnail names)

klepp0906 commented 5 years ago

you are correct on all counts :)

and trust me, ive considered that. Even began doing it at some point but changed my mind before I got too far in for several reasons.

Ultimately there will be "repercussions" no matter how I do it. But keeping the roms themselves with the same name has the least drawbacks. Considering I use them via several mediums, and whenever I scrape them, or update them with a dat file etc, having edited names has the potential to create trouble across the board. Then I have to edit the media in RA as well to match.

Everything else I use has a substantially easier route to cleaning up the file names so at the cost of having issues everywhere else and complicating every update across everything, leaving them as is makes everything just work, and stay uniform everywhere but RA.

With RA ill simply have to get used to editing out the "&" in media, and having unsanitized labels. It will be my "informational" gui when I use it lol ;p

in the end, I just went through (am going through) every single path and romset to all conform to the no intro spec, as well as media, 16000 or so items, all to have uniformity so i never have to do this again.

So yea, im kinda stuck in a not getting all my cake situation but it is what it is.

I'm sure RA will get the quality of life polish eventually, it can do a substantial amount that other frontends cant do - and i dont intend on using anything else so /shrug.

fwiw - no illusions that its simple to add, I just figured adding a regex modifier for labels, something that can do all of them in a playlist file at once as opposed to manually, might be the easiest route and the only drawback being having to rename the media (which can be done much easier with a text editor than labels (but only on one line of each entry) within a playlist file.)

EDIT - i take that back. another drawback (until delved into much more deeply as you mentioned) to a regex label rename, is it would still leave the situation that brought me to make this post, media linkage being broke across launch medium) So basically I just wrote a book for nil ;p

still - until next time! (ill try not to harass you!)

klepp0906 commented 5 years ago

@jdgleaver I must harass you again my friend. Not sure if this is fixable, or perhaps you can suggest a workaround.

Your fix has served me perfectly thus far. However I've ran into an issue with NeoGeo roms. As im sure your aware, they require the romname to be unchanged. And without touching anything, as you would assume - everything works.

Unfortunately I cant live with roms named things like 3countb. lol.

The minute i change the rom label in RA (and as required, the artwork name) is when things break down. Steam generates a history entry, but its artless and using the old icky name.

I was under the assumption that RA's art was used, and that as long as the path to the rom was the same on both fronts, and in a folder named like RA's artwork folder - it'd be golden.

Does not seem to be the case here. How is label name tied into this with RA?

In steam they dont seem to be attached as I keep all my names sanitized there, and they arent sanitized in RA - yet no problems. Its only with this neo geo fiasco.

I'm likely missing something but ive been vetting, renaming, and configuring this neo geo set for days and my brain is fried.

klepp0906 commented 5 years ago

ftr i made the label and my steam title 1:1 just to test. both use the same rom and path to rom, in the same folder. problem still presents.

What am i missing?

image

yet no issue if i keep my steam title like the above, and remove the custom label in RA and return it and the artwork to 3countb.

For now, im going to have to live with it. ive tried all possible combo's of naming and artwork naming and I think the only solution on my end would be to keep everything with the mame-name which leaves this as being a lesser evil.

i'm sure you know where the hang up is, and i know you said its gonna take a bit more work at some point. Figured id mention this in case theres something that can be done on my end im not seeing.

jdgleaver commented 5 years ago

Ah, yeah, that's a nuisance...

Basically, for content loaded via the command line (which is what Steam is doing here), all RetroArch gets is the rom file path.

For detecting thumbnails, RetroArch uses the playlist label first, then falls back to the rom filename (if the label is empty). There's no way for Steam to pass the 'sanitised' rom label to RetroArch (i.e. 3 Count Bout ...), so content history has no label, and just a file name of 3countb.7z.

So you can see the problem here - there is no way for RetroArch to know that 3countb should correspond to the 3 Count Bout ... thumbnail image. (Not a problem if you use a RetroArch playlist to launch these games, since the correct label is automatically copied to history)

What is the solution? Hmm... It's not obvious. I guess the only way is to provide additional command line options for specifying things like rom label (but of course, this would have to be entered by hand, by the user, for each command line shortcut - which is pretty ugly and laborious).

klepp0906 commented 5 years ago

One more update to this guy. (as we already spoke about in discord). The fixes you implemented seem to not translate to the desktop menu.

If a game is initiated via command line through something like steam, the history list in the desktop menu shows a path to the game instead of the game title, and as such - no associated image.

dont hate me too much!