Skip to content
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

GCMemcard/MemcardManager: Make creating a new card more explicit and UX-friendly. #7996

Open
wants to merge 4 commits into
base: master
from

Conversation

3 participants
@AdmiralCurtiss
Copy link
Contributor

AdmiralCurtiss commented Apr 13, 2019

The current way is pretty poor UX -- try it yourself, manually type something in the Memory Card Manager's path boxes and see what happens. If we want this feature, we should add an explicit 'create new file' button or dialog.

Note that this is the only place where GCMemcard is ever instantiated without first checking if the file exists, so barring file system races it's the only place that can actually reach the 'create new file' branch.

Thoughts? The actual new UI here is pretty slapdash, but I figured I should collect some opinions before actually putting effort into making a nice dialog.

@spycrab

This comment has been minimized.

Copy link
Contributor

spycrab commented Apr 13, 2019

Could you maybe provide a screenshot of the UI changes? That would be swell :)

@AdmiralCurtiss AdmiralCurtiss force-pushed the AdmiralCurtiss:memcard-manager-create-new branch from 286c348 to 201b286 Apr 13, 2019

@AdmiralCurtiss

This comment has been minimized.

Copy link
Contributor Author

AdmiralCurtiss commented Apr 13, 2019

The old way was when you typed a path into the Slot A or Slot B path field, you'd get this dialog for every character you typed:

image

If you pressed 'Yes' you then got:

image

It feels unintended, to be honest.

What is currently in the PR is just a button that opens a regular file save dialog, but I'm not really sold on that. It'd be nicer to have a separate window where you could select things like encoding and card size.

image

@spycrab

This comment has been minimized.

Copy link
Contributor

spycrab commented Apr 13, 2019

Heh, I'd been fine with a screenshot of showing how it looks with your PR

Anyway, if you want to create a proper prompt best to create a new QDialog with multiple QComboBoxes for options (in a separate file).

@BhaaLseN
Copy link
Member

BhaaLseN left a comment

About time, thanks :D

UX is still a bit meh with GCMemCard directly showing alerts, but thats for another PR.

Regarding settings, I could see the "Create new" button being a dropdown that, when clicked directly, creates the default sized (which may or may not be the previous 16MB) western memcard; or when opened presents a list of common sizes that make sense. Not sure where to put that SJIS option tho; maybe a checkbox at the end of that dropdown?

@@ -30,22 +30,14 @@ static void ByteSwap(u8* valueA, u8* valueB)
*valueB = tmp;
}

GCMemcard::GCMemcard(const std::string& filename, bool forceCreation, bool shift_jis)
: m_valid(false), m_filename(filename)
GCMemcard::GCMemcard(const std::string& filename) : m_valid(false), m_filename(filename)
{
// Currently there is a string freeze. instead of adding a new message about needing r/w

This comment has been minimized.

Copy link
@BhaaLseN

BhaaLseN Apr 13, 2019

Member

This comment seems heavily outdated...

This comment has been minimized.

Copy link
@AdmiralCurtiss

AdmiralCurtiss Apr 13, 2019

Author Contributor

Yeah I noticed that too. I think I want to redesign the entire way this is constructed anyway due to the PanicAlerts so this will probably go away with that.

@AdmiralCurtiss AdmiralCurtiss force-pushed the AdmiralCurtiss:memcard-manager-create-new branch from 201b286 to bebdcb4 Apr 13, 2019

@AdmiralCurtiss

This comment has been minimized.

Copy link
Contributor Author

AdmiralCurtiss commented Apr 13, 2019

Okay, pushing the 'Create New...' button now pops up this little menu to let you set options:

image

I'm still not too happy about the actual placement of the button though, it doesn't really make sense next to the buttons that deal with the currently highlighted item in the save file list. Better ideas?

Maybe two separate buttons around the Card A/B Browse buttons that then automatically select the newly created card for copying saves onto?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.