Fix persistence of new collections, either from theme or custom name

[Gemba]

Fixes the bug in this scenario:

1. Select in Main Menu -> Game Collection Settings -> Create New Custom Collection from Theme or Create New Custom Collection
2. Notice the toast which states that you are editing the custom collection now
3. Add at least one game with Y-Button to the collection
4. Select in Main Menu -> Game Collection Settings -> Finish Editing 'xyz' Collection
5. Notice the toast which states editing has ended
6. Quit EmulationStation via the Main Menu (do not use F4)
7. Restart EmulationStation

Result

The custom 'xyz' collection is not created, or if it was a theme based custom collection it is no longer present on the carousel.

Expected result

After restart/reboot, the new collection is still available/present in ES and contains the games added before.

Additional Notes

• The collection is persisted as soon as end editing is selected and not only in the destructor of the CustomCollectionManager class.
• It fixes also a cornercase when the filename index on collection name collisions has two digits (index 10 or above) (the added infix " (n)" is then longer than 4 characters.
• It inhibits also the editing of second, third, ... custom collection before the first one is closed (end editing): The end editing is now shown instead of the new custom collection entries to emphasise this.

Hope you can reconstruct the issue. Any Qs, please let me know.

[pjft]

Thanks. I apologize, but I won't be able to look into this in the coming weeks for professional commitments, so don't read too much into the lack of response.

I'm wondering, is this in a situation where metadata saving on exit is off, or does it happen all the time? Even though it is not a frequent use case, I am very much under the impression that this was working when it was implemented for my setup at the time, though there might have been cases that clearly didn't work. I likely didn't test without saving on exit.

Just curious. Thanks for putting this together!

[Gemba]

There is no rush to address this.

I found out it only happens in the main/head, but not in the version 2.11.2.

From a cursory look, I assume it is because the sysdata.isPopulated is no longer set true in the main/head branch when the file for the new collection does not exist (CollectionSystemManager::populateCustomCollection()). But I did not follow it into all details. However, if a collection is not marked populated, it is later not considered for storing any changes. In master/head it can therefore be reproduced regardless of the setting for the metadata save.

The change makes the creation of a new collection more robust (not depending on the position of sysdata.isPopulated = true; line.

There is a workaround available: touch the new collection file on the filesystem (custom-<collectionname>.cfg in ~/.emulationstation/collections), restart ES, enable the Collection in the menu and then select add/remove games to this collection when in the collection system view from the menu.

[pjft]

Oh my, thanks for the detailed report. I suppose I must have broken it in one of the recent changes of the collections handling - likely the random collections. Thanks for catching that.

[pjft]

Sorry for the delay. All looks good. Just wondering if we can't stick to writing the files in the CollectionSystemManager class rather than having the same logic in the GuiCollectionSystemsOptions class.

Thanks!

[pjft]

Gently pinging this in case there's still interest in merging this. Happy Monday!

[Gemba]

Nvm. Thanks for your patience, also on my side life happens. You are right it is almost the same code. Leave it with me for a few days, I will rework it a bit and have it put into saveCustomCollection().

[pjft]

No worries whatsoever - just wanted to make sure this wasn't lost because of my lack of response.

Take your time - I'll also be off for the better part of the coming few months, so it's alright.

Have a great week!

[Gemba]

I spared some time and keep the file create logic in the saveCustomCollection(). I used an enum class for the different flags a collection data set can have (as otherwise it would have been another boolean value in the method, plus the first one had a default value). The enum class is more verbose but also is more precisse as boolean flags IMHO.

I have put everywhere toUpper() for the collection name in the toasts (Gui Popups), it was missing on setEditMode().

Review whenever you want/can. I will watch this space to followup.

Great week also for you!

[pjft]

Thanks. All looks good!

Would you comment the enum for the meaning of each value, for future reference? The "Keep in Index" in particular isn't quite clear, so might as well add for all in case someone else looks into this code in the future :)

[Gemba]

Makes sense, updated.

[pjft]

Thanks.