Improve Save Manager #2951
Improve Save Manager #2951
Conversation
|
@flash-fire, thanks for your PR! By analyzing the history of the files in this pull request, we identified @Nekotekina, @TomiYuga and @Inviuz to be potential reviewers. |
…redundant settitle.
|
I'm done with this for now. Feel free to squash on merge because I don't trust my rebase squashing abilities. |
|
I can do it for you.... |
flash-fire
changed the title
… I could see on stack overflow. Also, add an error dialog if you have no entries. Simplify the logic slightly for the selected since with the no data case handled, I can make more assumptions about the return value.
|
I changed my mind. I want to make some other changes while I'm modifying the save manager to fix some things people mentioned. |
flash-fire
changed the title
…nothing. Having both isn't needed. Yay merges resulting in doing stupid things.
|
This works just fine. No more new games/errors when no save is loaded. |
|
Good for me now in 4k screen |
|
For example if i resize the title , next time reopen the game , the save interface will back to default .
|
|
Save manager doesn't save its dialog state into settings so this is expected behavior. |
|
I see |
| { | ||
| fs::file icon = fs::file(base_dir + entry.name + "/ICON0.PNG"); | ||
| u32 iconSize = icon.size(); | ||
| void* iconData = new uchar[iconSize]; |
… (tested with disgaea and renaming icon file)
|
I'm pretty sure that fixes the leak, but normally I'm not making std::shared_ptr of casted data. :P A standard pointer didn't work because the vector copies the struct to add it or something (I kept getting double free errors and was feeling too lazy to investigate rigorously). |
|
you could remove the header highlighting. I did it with gamelist |
| fs::file icon = fs::file(base_dir + entry.name + "/ICON0.PNG"); | ||
| u32 iconSize = icon.size(); | ||
| uchar* iconData = new uchar[iconSize]; | ||
| icon.read(iconData, iconSize); |
There was a problem hiding this comment.
vector<uchar> would probably be nicer..
There was a problem hiding this comment.
What would you do with the struct? I concede the std::shared_ptr isn't very clean. But, using a vector would mean the vector gets copied when save_entries.push_back is called. To me, it's not ideal copying data and immediately deleting the other copy. I'm welcome to suggestions as I would probably do a std::shared_ptr myself or maybe a std::shared_ptr<std::vector>, but the original comment had a void* pointer, and I tried to respect that.
There was a problem hiding this comment.
@flash-fire why it's shared and not unique? Is it really shared anywhere? Also, why not do reset before read? Feels cleaner to me.
Though if it's just a helper struct, there is no need to have a ptr at all. You can just replace pointer + size with a vector (as paulsapps suggested).
Also, if you worry about copying vector, you can just implement move constructor or use emplace_back.
|
@Megamouse I'll do that later. This PR is a bit of a mess as is tbh. |
Ready for merge