Aloshi / EmulationStation

A flexible emulator front-end supporting keyboardless navigation and custom system themes.
MIT License
2.07k stars 905 forks source link

Don't strip data in parenthesis for Mame/Arcade games #323

Open joolswills opened 9 years ago

joolswills commented 9 years ago

The information in parenthesis is pretty useful. And it's hard to know which version is which when you have a list of 4 "pacman" entries etc. How about:

diff --git a/es-app/src/FileData.cpp b/es-app/src/FileData.cpp
index a1b625d..87a49ef 100644
--- a/es-app/src/FileData.cpp
+++ b/es-app/src/FileData.cpp
@@ -64,9 +64,12 @@ std::string FileData::getCleanName() const
 {
        std::string stem = mPath.stem().generic_string();
        if(mSystem && mSystem->hasPlatformId(PlatformIds::ARCADE) || mSystem->hasPlatformId(PlatformIds::NEOGEO))
+       {
                stem = PlatformIds::getCleanMameName(stem.c_str());
-
-       return removeParenthesis(stem);
+               return stem;
+       }
+       else
+               return removeParenthesis(stem);
 }

 const std::string& FileData::getThumbnailPath() const

Is the reason for the clean up related to the fact the name is used for the scapers or ? I guess the scraper would do best with a clean name, but the displayed name should include the additional information. That would be ideal for most mame users I should think (and when scraping the original name with data is kept and not overwritten by the scraper)

BTW, the current mame rom -> name mapping doesn't work too well on old mame (for example mame4all on the pi which is based off mame 0.37b5) due to rom renames. Would you be happy to receive a patch against the mapping file that includes missing entries (just new (old) ones, no changes to existing ones).

Aloshi commented 9 years ago

You got it right, it was originally to deal with scrapers not working (though a workaround could certainly be added). I'm making progress on the gamelist database rewrite (see the gamelistdb branch), after that I plan on adding "region" metadata that can be guessed through trailing parenthesis. Not sure how I want to display it yet, though. I've been holding off on a lot of simple fixes while I work on that (since the code has changed considerably).

Regarding the MAME rom name mapping, I'm not sure. ES is now used on platforms other than the Pi, where using the most recent version of MAME makes sense. I don't know what including both tables would do to executable size - a good compiler might be able to collapse a lot of the duplicate strings to the same pointers. From there, a setting could determine where it pulls the name from.

joolswills commented 9 years ago

the gameslist sounds excellent. Look forward to that. I think I may just manually patch up emulationstation for retropie for the arcade list for now then.

I was thinking to just merge missing entries into the current table. There may be cases where there are duplicate strings, but I was going to skip them, and just add missing strings. this would improve older mame, whilst not affecting the mame version the table was originally meant for.

Aloshi commented 9 years ago

Ah, okay, that makes sense. If you submit a pull request with the missing names for MameNameMap.cpp, I'll merge it in (assuming you don't find any conflicting strings between versions).

joolswills commented 9 years ago

cheers. if there is a conflicting name, I won't overwrite it, so the list will be valid for the version you used, but with some older stuff in there.

joolswills commented 9 years ago

Should I include them at the end (after a comment saying they are roms from mame 0.37b5). Would seem tidier perhaps than mixing them in perhaps - but up to you

joolswills commented 9 years ago

Looks like this appended.

http://malus.exotica.org.uk/~buzz/MameNameMap.cpp.diff

joolswills commented 9 years ago

Stripped the parenthesis is also unhelpful for disk based systems like the Amiga where common naming is to put the disk number in brackets. Could have have a change where it is a user option whether to display the entire string, and just strip it before it it sent to the scrapers ?