RetroPie / EmulationStation

A Fork of Emulation Station for RetroPie. Emulation Station is a flexible emulator front-end supporting keyboardless navigation and custom system themes.
Other
850 stars 340 forks source link

Changes to maintain and persist <folder/> information in a gamelist: #841

Closed Gemba closed 5 months ago

Gemba commented 7 months ago
  1. folder elements are loaded when present in a gamelist and if they match the filesystem representation
  2. missing optional folder elements are rendered empty in theme (and not "unknown")
  3. As before non-existing folder elements are generated by ES, with mandatory elements <path/> and <name/>, but with this PR, whenever metadata is edited for this folder object (FileData class) it is persisted
  4. Editiing metadata on existing <folder/> gets persisted too
  5. UI metadata edit: layout fix that renders entered metadata not centered when returning from editing
  6. UI metadata edit: format tooltip for date format in release date field
Gemba commented 7 months ago

As it involved several files I will comment on the crucial changes.

pjft commented 5 months ago

@Gemba pinging this, in case you were waiting for me to make progress here. I am ok with the changes, just had the couple minor questions I added there. No pressure though - just wanted to make sure this wasn't in a limbo because of me. Have a great weekend.

Gemba commented 5 months ago

Thanks. I have read through this issue twice. Did I miss a question to answer? I you can recap the one(s) which escaped my attention it would be nice.

pjft commented 5 months ago

Sure. Main questions I had:

Gemba commented 5 months ago
* Gamelist.cpp: std::string relative = Utils::FileSystem::removeCommonPath(path, root->getPath(), contains, true); - root->getPath() should be systempath, or am I missing something?

You are right. Well spotted. Done.

* Why don't we allow ratings for the folders?

It was more an assumption of mine. I don't stick to that. Enabled the rating edit on folders. Thanks for nudging me.

pjft commented 5 months ago

Thanks. I just tested this and, from what I was able to test on my end, I'm good with merging. Just a minor question I asked regarding a label, but I'm good to go next. I don't have folders on my setup, though, so it should be test run by more in the community if you can share it there!

Gemba commented 5 months ago

Will ask for more testers in the forum. Just let me know: Before the merge or after the merge?

pjft commented 5 months ago

Afterwards is fine, it's just to make sure the different scenarios get covered.

Do check my comment on the variable and squash as appropriate. Let me know when it's good to go.

Gemba commented 5 months ago

Afterwards is fine, it's just to make sure the different scenarios get covered.

Ok.

Do check my comment on the variable and squash as appropriate. Let me know when it's good to go.

I sure will. But I am really sorry to bother you again, I cannot spot your comment on the label/variable. Would you be so kind to highlight/repeat it? Thanks.

pjft commented 5 months ago

GuiMetaDataEd.cpp:41 - why ROM and not GAME?

Gemba commented 5 months ago

GuiMetaDataEd.cpp:41 - why ROM and not GAME?

Ah. Thanks. I put "ROM" here deliberatly as the filename with extension will be shown. The ROM's display title is shown first in the edit fields.

pjft commented 5 months ago

If I'm reading this correctly, then we should probably keep GAME for a few reasons:

A bit long, apologies, but hopefully will relay where I'm coming from here.

Gemba commented 5 months ago

I am ok with your justification, better avoid "surprises" for the end-user and step-back a little on technical preciseness. Did adjust the subtitle to "GAME: ..."

pjft commented 5 months ago

Thank you.