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

New save/load chooser #260

Merged
merged 33 commits into from Aug 12, 2012
Merged

New save/load chooser #260

merged 33 commits into from Aug 12, 2012

Conversation

lordhoto
Copy link
Contributor

This is a improved version of pull request #250. The grid based chooser now also supports saving. It still does not support deleting saves however.

Some screenshots:

Loading: http://lordhoto.users.sourceforge.net/scummvm-thumbnail-1.png

Saving:
http://lordhoto.users.sourceforge.net/scummvm-thumbnail-2.png
http://lordhoto.users.sourceforge.net/scummvm-thumbnail-3.png
http://lordhoto.users.sourceforge.net/scummvm-thumbnail-4.png

The "New Save" button will automatically use the smallest unused slot available.

The grid based dialog can be disabled via defining DISABLE_SAVELOADCHOOSER_GRID, which is for now defined when DISABLE_FANCY_THEMES is defined. Porters will need to check whether this dialog will work with their ports, since it requires more memory and then can add DISABLE_SAVELOADCHOOSER_GRID to their flags.

Johannes Schickel added 30 commits June 14, 2012 03:01
GuiObject::removeWidget only removes the widget from the widget list, but
doesn't delete it. Oops.
Formerly it was enabled for everything above 320x200, but resolutions below
640x400 feature not enough space.
Now it is possible to add sub widgets to a ContainerWidget and allow for these
to get events too.
Now the thumbnail button and the descriptions are sub widgets of the
container widget.
@lordhoto lordhoto mentioned this pull request Jul 24, 2012
@bluegr
Copy link
Member

bluegr commented Jul 24, 2012

I like it!
However, would it be possible to change the red backgrounds in each screenshot to black? That red isn't nice, IMHO

@lordhoto
Copy link
Contributor Author

However, would it be possible to change the red backgrounds in each screenshot to black? That red isn't nice, IMHO

Red is our default button color. It may be possible to add another color for this in the modern theme. But this would be inconsitent with all the other buttons, so I am not sure I like it.

@lordhoto
Copy link
Contributor Author

Unless there's any objections I'll merge this tomorrow (29.7) evening.

@bluegr
Copy link
Member

bluegr commented Aug 1, 2012

Were there any objections then? (cause this hasn't been merged)

@lordhoto
Copy link
Contributor Author

lordhoto commented Aug 2, 2012

Since it's summer time and the pull request had only been open for a few days I decided to wait a bit longer.

@bluegr
Copy link
Member

bluegr commented Aug 2, 2012

I still don't like the red border around the pictures. I realize that it's the color that all the other buttons are using, but it looks ugly in this case, as the contrast with the screenshot colors is quite high. A dark color (i.e. black) would be more fitting, IMHO

@lordhoto
Copy link
Contributor Author

lordhoto commented Aug 2, 2012

Black buttons: http://lordhoto.users.sourceforge.net/scummvm-thumbnail-5.png
Looks ugly and doesn't fit the coloring of our modern theme.

@clone2727
Copy link
Contributor

The red looks good to me.

@salty-horse
Copy link
Member

I think the page numbering should be closer to the prev/next buttons. Maybe directly above the buttons, centered to be above the space between them?

@lordhoto
Copy link
Contributor Author

lordhoto commented Aug 8, 2012

I think the page numbering should be closer to the prev/next buttons. Maybe directly above the buttons, centered to be above the space between them?

Above the buttons would sadly not work for 640x400 anymore, since then there would be not enough space for two rows. But maybe left to them would be possible.

@djwillis
Copy link
Member

djwillis commented Aug 8, 2012

Just reviewing this and playing with it on a few small screens. Is there a good reason not to merge it and tweak things afterwards? Especially as we are so early in the release cycle, maybe a few porters will even build the code before the next 1.6.* release ;).

It looks good (to my eyes) with the red borders on a small screen (840*480) but I have not had a good look at the screens when scaled up to higher resolutions.

It may also be nice (but I am being picky) to default this to disabled for all the smaller backends that are low on RAM or < 640*400.

Of the smaller/low spec device backends the only ones I would leave this enabled on are Maemo and OpenPandora, it would have to be off for GPH, DS, PSP etc. and I suspect it would be an extra load the DC could do without.

@lordhoto
Copy link
Contributor Author

lordhoto commented Aug 8, 2012

Just reviewing this and playing with it on a few small screens. Is there a good reason not to merge it and tweak things afterwards?

The main reason why I did not merge this yet, is not because it will still require some tweaks. It's rather because there didn't seem to be much feedback about the general design.

It may also be nice (but I am being picky) to default this to disabled for all the smaller backends that are low on RAM or < 640*400.

Adding the define for those backends should be easy.

lordhoto pushed a commit that referenced this pull request Aug 12, 2012
New save/load chooser

Conflicts:
	gui/saveload.cpp
@lordhoto lordhoto merged commit 71daae7 into scummvm:master Aug 12, 2012
@lordhoto
Copy link
Contributor Author

Merged now. I also wrote a mail to -devel about the define used to disable the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants