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

Display world screenshot previews and progress #2349

Open
wants to merge 28 commits into
base: master
Choose a base branch
from

Conversation

Vankata453
Copy link
Member

@Vankata453 Vankata453 commented Dec 4, 2022

WorldSetMenu and SortedContribMenu now inherit the new WorldPreviewMenu class, which allows displaying profile-specific world previews and progress. A preview for a world is generated each time the user leaves its worldmap. Only worldmap levelsets support this feature.
Supported also are optional placeholder world previews, which have to be named preview.png, and stored in the relative to levels directory for the world.

image

FileSystemMenu also now shows previews of images, when hovering over an image file.

image

@mrkubax10 mrkubax10 added type:feature category:code status:needs-review Work needs to be reviewed by other people labels Dec 6, 2022
@Vankata453 Vankata453 marked this pull request as draft June 24, 2023 14:47
@Vankata453 Vankata453 marked this pull request as ready for review August 5, 2023 23:46
solved(solved_),
total(total_)
{}
uint32_t solved;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m_solved
m_total

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't think an m_ prefix is needed here, since this struct is only used to store 2 variables together and has just one simple function to calculate percentage using them.

@tobbi
Copy link
Member

tobbi commented Oct 27, 2023

/home/runner/work/supertux/supertux/src/worldmap/worldmap_state.cpp:139:30: error: empty expression statement has no effect; remove unnecessary ';' to silence this warning [-Werror,-Wextra-semi-stmt]
WORLDMAP_STATE_SECTOR_GUARD;
^
/home/runner/work/supertux/supertux/src/worldmap/worldmap_state.cpp:172:30: error: empty expression statement has no effect; remove unnecessary ';' to silence this warning [-Werror,-Wextra-semi-stmt]
WORLDMAP_STATE_SECTOR_GUARD;
^
/home/runner/work/supertux/supertux/src/worldmap/worldmap_state.cpp:200:30: error: empty expression statement has no effect; remove unnecessary ';' to silence this warning [-Werror,-Wextra-semi-stmt]
WORLDMAP_STATE_SECTOR_GUARD;
^
/home/runner/work/supertux/supertux/src/worldmap/worldmap_state.cpp:256:30: error: empty expression statement has no effect; remove unnecessary ';' to silence this warning [-Werror,-Wextra-semi-stmt]
WORLDMAP_STATE_SECTOR_GUARD;
^
/home/runner/work/supertux/supertux/src/worldmap/worldmap_state.cpp:347:30: error: empty expression statement has no effect; remove unnecessary ';' to silence this warning [-Werror,-Wextra-semi-stmt]
WORLDMAP_STATE_SECTOR_GUARD;
^
/home/runner/work/supertux/supertux/src/worldmap/worldmap_state.cpp:361:30: error: empty expression statement has no effect; remove unnecessary ';' to silence this warning [-Werror,-Wextra-semi-stmt]
WORLDMAP_STATE_SECTOR_GUARD;
^
/home/runner/work/supertux/supertux/src/worldmap/worldmap_state.cpp:382:30: error: empty expression statement has no effect; remove unnecessary ';' to silence this warning [-Werror,-Wextra-semi-stmt]
WORLDMAP_STATE_SECTOR_GUARD;
^
/home/runner/work/supertux/supertux/src/worldmap/worldmap_state.cpp:410:30: error: empty expression statement has no effect; remove unnecessary ';' to silence this warning [-Werror,-Wextra-semi-stmt]
WORLDMAP_STATE_SECTOR_GUARD;

@Vankata453
Copy link
Member Author

This PR is now ready for review.

@Vankata453 Vankata453 added this to the 0.7.0 milestone Jul 14, 2024
Copy link
Member

@MatusGuy MatusGuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is completing all levels normally supposed to count as 100%? What about getting all collectibles?

On small screens, the help text is not fully readable because it's not aligned to the screen but rather to the menu.
image

I don't see any animations when navigating a Filesystem menu.

Aside from that, nice work!

src/gui/menu.cpp Outdated
#include "video/video_system.hpp"
#include "video/viewport.hpp"

#include "supertux/error_handler.hpp"
const Sizef Menu::s_preview_size(426.f, 240.f);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcoding the preview size squishes pictures that are too big. Any reason as to why you're doing this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the image and the menu could both fit on the screen. Maybe I should check the resolution before determining this though.

@Vankata453
Copy link
Member Author

Is completing all levels normally supposed to count as 100%? What about getting all collectibles?

That is only supposed to indicate the progress through levels on the map. If the percentage isn't perceived well, I can remove it.

On small screens, the help text is not fully readable because it's not aligned to the screen but rather to the menu.

True, it should stay centered regardless.

I don't see any animations when navigating a Filesystem menu.

Do you have transitions enabled?

@Vankata453 Vankata453 requested a review from MatusGuy July 15, 2024 11:58
@MatusGuy
Copy link
Member

Do you have transitions enabled?

Yes, but now that I think about it, this might be because the loading time overrides the animation. That can be something for another pr. Will do some further investigation later.

The preview still gets squished. I cloned this PR and am currently trying to write a fix for this

@Vankata453
Copy link
Member Author

this might be because the loading time overrides the animation

Yes, if you go to the next item too quickly, there will be no time for animations.

The preview still gets squished.

It does, but only if you change your resolution to be different from the one when the screenshot was taken.

Comment on lines +699 to +700
Rectf preview_rect(Vector(static_cast<float>(context.get_width()) * 0.73f - preview_size.width / 2 + (width_diff > 0 ? width_diff / 2 : 0),
static_cast<float>(context.get_height()) / 2 - preview_size.height / 2 + (height_diff > 0 ? height_diff / 2 : 0)),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use std::max

Suggested change
Rectf preview_rect(Vector(static_cast<float>(context.get_width()) * 0.73f - preview_size.width / 2 + (width_diff > 0 ? width_diff / 2 : 0),
static_cast<float>(context.get_height()) / 2 - preview_size.height / 2 + (height_diff > 0 ? height_diff / 2 : 0)),
Rectf preview_rect(Vector(static_cast<float>(context.get_width()) * 0.73f - preview_size.width / 2 + std::max(width_diff / 2, 0),
static_cast<float>(context.get_height()) / 2 - preview_size.height / 2 + std::max(height_diff / 2, 0)),

if (valid_last_index)
{
// Draw progress preview of current item.
const Sizef preview_size(static_cast<float>(SCREEN_WIDTH) / 2.5f, static_cast<float>(SCREEN_HEIGHT) / 2.5f);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the context functions.

Suggested change
const Sizef preview_size(static_cast<float>(SCREEN_WIDTH) / 2.5f, static_cast<float>(SCREEN_HEIGHT) / 2.5f);
const Sizef preview_size(static_cast<float>(context.get_width()) / 2.5f, static_cast<float>(context.get_height()) / 2.5f);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:code status:needs-review Work needs to be reviewed by other people type:feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants