-
-
Notifications
You must be signed in to change notification settings - Fork 693
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Quicksave #1070
base: develop
Are you sure you want to change the base?
Quicksave #1070
Conversation
These checks are failing due to reasons other than my change. The first one fails due to a timeout of 45 minutes (!) while building RealmSwift. Another PR is able to get a bit farther than RealmSwift before timing out, but not much) The second fails due to an error in Microsoft's Xcode Build Task which may be the same problem as this similar open issue from 9 days ago. Both checks fail for the same reasons in this other PR. Please advise :) |
@mymikemiller Don't worry about the checks, all the CIs are breaking for their own reasons. I just haven't' had time to dig into this PR. |
Rather than an always visible button, I think something less obtrusive and quicker to access would be preferred. All the space that isn't an on-screen control could be used for gestures. Maybe something like a 3 finger double tap. Or twice finger drag from outer edge towards each other over 150ish pixels, which could be used by quickly moving your thumbers off the on screen controls and dragging them from the edge towards the middle, then you wouldn't have to even adjust your grip for standard holding styles. Also, looks like the behaviour is to delete the last manual save. I think that's too likely to delete something someone wanted to keep. It would be better to just not delete any previous saves. We only ever auto delete saves with autosaves, which are specifically flagged as such, and there's some other conditions such as time interval since last au And for the button layout, it should use anchors instead of calculating pixel offsets to reduce the likely hood of layout bugs if the layout function isn't specifically called on events like frame changes due to picture in picture, incoming call banners, rotation etc. @sevdestruct What you think? |
I haven't taken a look at this yet, but…
I agree: might not be best as an always visible button in its current execution. I've had something in mind for UI 2.0 for a new custom touch pause menu/UI and how to get quicksave and fast forward and such to be quickly accessible, but haven't drafted designs for it yet…
Update: Edge swipe quick action bar UI option would work, and good solution for now that we can put other stuff in like fast forward toggle. As for other gestures… for these types of controls will conflict with the upcoming free placing stick a/o d-pad and other revised touch controls i've got planned for 2.0. Apart from that, gestures need to be intuitive to what they do, and if not would require some onscreen tutorial/edu UI to establish it or if not that some visual feedback when using gestures (like if you were pinching having it shrink the screen as a snapshot confirmed saved and have it show that it saved that moment with something like a confirmation at the end but still…think a better UI to house these quick controls would be the way to go. Another option would be to revise how the pause button works—I'm thinking something that slides out from the main button and you can slide to quickly, and release which will call that action instead of pausing… real quick action, but out of the way.
As for layout issues, I'll take a look when I've got a chance.
Gotta agree with Joe on this, quicksave should have it's own type, and only write to it, even if it's a and extra backup save slot or whatever, similar Autosaves. Taking a page from modern games. You don't want to be overwriting manual saves. For a cleaner UI been thinking to change this screen to a vertical scrolling list, with auto and quicks at the top, and manuals under, with small/thin seperator and visual way to codify them or mark them, vs the grid used now, to avoid gaps in design layout due to orientation and devices, taking som inspiration from Skyrim
|
I misread Joe's comment earlier. We could, as he mentioned use an edge swipe — like make a narrow quick action bar UI that slides out via edge with button icons. I've updated my reply above. |
Thank you for your feedback, guys! Updating PVSaveState I started work on separating quicksaves into their own section of the save state picker and ran into all kinds of problems trying to get PVSaveState to store the type of save using an enum [auto, quick, manual] instead of the boolean isAutoSave it uses now. I kept crashing during the migration because the new saveType enum wasn't showing up in newObject in migrationBlock of realmConfig. I guess Realm just doesn't work well with enums. I'm sure I was messing up something simple. In any case, I reverted that attempt and switched to simply adding another boolean: Save state picker design
Maybe it makes the most sense to have them in reverse chronological order, with types all mixed together (as long as they show what type they are, as in your design). That way you can quickly scan through the thumbnails from top to bottom and stop at whichever one is far back enough, regardless of how it was created. |
Button Because of the frequency with which I tend to quicksave, I much prefer an always-available button. I press quicksave more often than I press For my personal build, I plan to add quickload by long-pressing the quicksave button. Since I have "Show Quicksave Button" as an option under Here are some screenshots of the button as it is now. It stays nicely out of the way in both landscape and portrait and is easy to access with the right thumb (It's important that I can perform a quicksave/quickload while keeping my left thumb on the d-pad so my character instantly moves in the desired direction once the state is loaded). |
Realm does work with enums but they have to be backed by a realm compatible associated value and you have to do the conversion yourself. https://medium.com/it-works-locally/persisting-swift-enumerations-with-realm-io-dab37cd98bcd Also you can't just remove an iVar from a Realm object without adding a migration function to the init of the database. You'd have to take all the old objects that have isAutosave and convert their values to the new enum associated value containing iVar on the newObject in the migration method. You're not updating the save state's themselves so you don't need to worry about backwards compatibility, it's just the reference to those save states and extra metadata that's in the database, the saves themselves are raw files that are created and read by the cores which don't know anything about the Realm references. |
I think the reasoning to need a quick save button on screen because you want to constantly tap it incase you die at any instance, is really a case for adding rewind functionality. Franticlly using quick save in rapid succession seems like a hack that fills the niche that rewind serves. In either case, I'm not sure still that an always visible icon is the way to go, considering no one has ever made a feature request for quick saves before, I'd wager it's not a particularly high demand feature to warrant permanent real-estate. I suppose as long as it's an option that's off by default it would be ok. |
Points well taken. None of the emulators I grew up with (mainly snes9x) had a rewind feature, so I guess I just got used to spamming quicksave. It still does come in handy for cases when you know you might want to return to a specific point, like when you have a choice of two directions to go and you know the one you choose might lead to a dead end or nothing of interest. Saves the time of walking all the way back, and rewinding through everything you did on the wrong path would be tedious. I'm looking forward to the rewind feature - you may just get me to give up save states entirely :) Thanks for the link re: Realm and enums. I'll see if I can get my attempt to compile. |
The recent changes address your suggestions. "Quicksave" is now available in the menu, and the quicksave button can be toggled on in the settings (it is off by default). Quicksaves are separated from auto and manual saves in the Save States screen. Like auto saves, your 5 most recent quicksaves are kept. Long-pressing the quicksave button performs a quickload (loading the most recent quicksave). I made a change to the way autosaves (and now quicksaves) are auto-deleted when there are more than 5. Previously, Another change ensures that the .svs.json file is deleted when a PVSaveState is deleted. Previously these files were left around indefinitely. A couple things I'd like to call out:
Please let me know your thoughts on this PR. I've been enjoying using the quicksave feature, and I hope other users will find it handy too. |
Just my non-dev/user two cents: I use an MFi controller which eliminates the on screen button overlay. I would like gestures (as well as the on-screen save button) or a button combo to access functionality while using a controller. I do love the basic idea of quick manual saves/loading. |
Using realm.delete as was previously done does not delete the save state files
This re prevents the emulator from freezing when loading a quicksave by long-pressing the quicksave button when anothercontroller button is held down
Quicksave now only appears for cores that support save states. Also resolves a bug where using Quicksave from the pause menu caused video to freeze.
This commit made a lot of formatting changes that made merging this PR non-trivial. It has been rebased on top of those changes to be easier to diff and merge :) I hope you'll accept this implementation of quicksave, and if not, let me know what other changes you'd like to see made. |
I got a lot going on at the moment, I'll try to get to this soon but it may take a week or so. I want to test this and fix a crash and put a new build out so I need to find a few hours. Just been swamped lately, apologies. |
Adds a quicksave feature in the form of a save icon on the top right side of the screen. Tapping this icon overwrites your most recent manual save with a new save.
Request for feedback:
How do you feel about this implementation?