Display background & moving progress bar on shutdown screen #14597

chmodsayshello · 2024-04-27T18:45:20Z

This Pull Request adds the main menu background to the shutdown screen, and makes the progress bar constantly refill itself indefinitely as long as the game is still shutting down. It does so by using a load screen instead of an overlayMessage.
Fixes #14571

Ready for Review.

shutdown.webm

How to test

launch the game
join either a server or a singleplayer world
quit
verify that said changes apply to the shutdown screen

Zughy · 2024-04-28T13:12:58Z

Assigning @grorp due to being the author of the issue (unsubscribing)

appgurueu

Works. Simple and effective. I like this. This may not be an ideal "striped" progress bar (and hence slightly confusing when it wraps around), but it's definitely good enough and much better than the status quo.

grorp · 2024-04-28T16:39:30Z

and hence slightly confusing when it wraps around

To avoid the confusion without making things overly complicated, I'd suggest this:

progress.bar.mp4

(My implementation of this is at https://github.com/grorp/minetest/commits/shutdownScreen, but the patch is horrible and not meant to be used as is.)

grorp

The far edges of the clouds on the shutdown screen are black or sometimes green-ish for me. This doesn't happen on the loading screen.

src/client/game.cpp

chmodsayshello · 2024-04-28T21:03:57Z

and hence slightly confusing when it wraps around

To avoid the confusion without making things overly complicated, I'd suggest this:
progress.bar.mp4

(My implementation of this is at https://github.com/grorp/minetest/commits/shutdownScreen, but the patch is horrible and not meant to be used as is.)

I've only taken a very brief look at your commits as of right now, but I think at the point where you are modifying the rendering engine itself, it makes more sense to pass a boolean, which is a false by default in order to control wether the loading bar will do it's indefinate loading animation instead of providing the option of drawing a partional bar only.
That way we can avoid having loading animations that look different in case this is done somewhere else at some point in the future.

Anyway, I'll try to actually read through the entire thing and implement your desired changes tomorrow :)

inspired by group's implementation

chmodsayshello · 2024-04-29T14:28:34Z

The far edges of the clouds on the shutdown screen are black or sometimes green-ish for me. This doesn't happen on the loading screen.

I have not been able to reproduce that, is it still the case on my most recent commit?

src/client/game.cpp

src/client/renderingengine.cpp

grorp

In principle, your approach is also ok, but it looks bad at the start and the end of the progress bar because of the way the texture is mapped to the slider. Example:

I think it would look best if both ends of the slider would always use the ends of the texture.

It's also irritating that the progress bar doesn't start at the beginning but at its last position if you start and shut down multiple games. It should start at the beginning again.

I have not been able to reproduce that, is it still the case on my most recent commit?

Yes, it is.

src/client/game.cpp

src/client/renderingengine.h

chmodsayshello · 2024-05-09T16:44:38Z

I have not been able to reproduce that, is it still the case on my most recent commit?

Yes, it is.

I have now (finally) been able to reproduce this behavior! I noticed that the color of the distant clouds is always the color the sky was before the shutdown, for example, if you run /time 19:00and then exit the game, they will be orange-ish.

Any chance it was night in your game session?

src/client/game.cpp

grorp · 2024-05-09T19:04:05Z

It's also irritating that the progress bar doesn't start at the beginning but at its last position if you start and shut down multiple games. It should start at the beginning again.

not fixed

EDIT: also misclicked

grorp · 2024-05-09T19:07:56Z

This needs an update too:

minetest/src/client/game.cpp

Line 1426 in 553ea89

showOverlayMessage(N_("Shutting down..."), dtime, 0, false);

chmodsayshello · 2024-05-09T19:08:09Z

It's also irritating that the progress bar doesn't start at the beginning but at its last position if you start and shut down multiple games. It should start at the beginning again.

not fixed

EDIT: also misclicked

The only solution I can think of is passing a pointer to a variable the position value is stored in then, are you fine with that?

src/client/clientlauncher.cpp

chmodsayshello · 2024-05-09T21:20:29Z

At this point, I've made way more commits than I feel like is reasonable for a PR of this scale, so, now may be a good time for a squash.
Thanks a lot for your effort by the way :)

chmodsayshello · 2024-05-09T21:45:43Z

To be honest, I think in the long term it might be better altering Game::showOverlayMessage (see https://github.com/chmodsayshello/minetest/blob/shutdownScreen/src/client/game.cpp#L4369) so by default it shows the sky and uses the indefinite loading added by this PR if no percentage >= 0 is given... Though, on the other hand, that might be something I'd do in another PR, as that would slowly push this pull request from it's purpose of well, "adding something that moves to the shutdown screen"… On the other hand, some of the changes I made around line 1200 wouldn't be necessary anymore.

Edit: here is a patch (I had to change the file extension so GitHub allows me to upload it) for the said change, since it ended up being a minor change, maybe it's worth applying it to the PR?
0001-make-overlayMessage-support-indefinite-loading.patch.txt

grorp · 2024-05-10T12:43:41Z

src/client/game.cpp

+
+		fps_control.reset();
+		f32 timer = 0.0f;
+
 		while (!client->isShutdown()) {


Interesting, this loop will never run anyway. client->Stop() above sets m_shutdown to true ...

minetest/src/client/client.cpp

Lines 314 to 316 in 07fe8d4

void Client::Stop()

{

m_shutdown = true;

... and client->isShutdown() returns true as soon as m_shutdown is true:

minetest/src/client/client.cpp

Lines 331 to 334 in 07fe8d4

bool Client::isShutdown()

{

return m_shutdown || !m_mesh_update_manager->isRunning();

}

Anyway, that's out of scope for this PR to investigate and fix.

Accordingly, I've removed the changes to this loop again.

In that case, assuming that boolean is not volatile, shouldn't the entire loop be removed?

I'd rather not remove it without investigating first:

what's the purpose of the code?

did it ever work?

why doesn't it work (anymore)?

is there a bug because the code doesn't work?

which is likely out of scope for this PR

grorp

I've applied some cleanup to this PR, also including your patch for showOverlayMessage. It looks good to me now.

At this point, I've made way more commits than I feel like is reasonable for a PR of this scale, so, now may be a good time for a squash.

You don't have to squash, the PR will be squashed automatically when merging.

display background & moving progress bar on shutdown screen

98200bb

chmodsayshello force-pushed the shutdownScreen branch from 311e745 to 98200bb Compare April 27, 2024 18:54

remove timer variable

97f7a52

Zughy added @ Startup / Config / Util Feature ✨ PRs that add or enhance a feature UI/UX Supported by core dev Not on the roadmap, yet some core dev decided to take care of this PR labels Apr 28, 2024

Zughy assigned grorp Apr 28, 2024

appgurueu approved these changes Apr 28, 2024

View reviewed changes

appgurueu added the One approval ✅ ◻️ label Apr 28, 2024

grorp requested changes Apr 28, 2024

View reviewed changes

src/client/game.cpp Show resolved Hide resolved

src/client/game.cpp Outdated Show resolved Hide resolved

grorp added the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Apr 28, 2024

chmodsayshello added 3 commits April 29, 2024 15:52

add sliding load animation

d80618c

inspired by group's implementation

fix shutdown translation

ca521f6

avoid delay in shutdown screen

5a0b338

chmodsayshello requested a review from grorp April 29, 2024 14:28

sfan5 reviewed Apr 29, 2024

View reviewed changes

src/client/game.cpp Outdated Show resolved Hide resolved

sfan5 reviewed Apr 29, 2024

View reviewed changes

src/client/renderingengine.cpp Outdated Show resolved Hide resolved

grorp reviewed Apr 29, 2024

View reviewed changes

src/client/game.cpp Outdated Show resolved Hide resolved

src/client/game.cpp Outdated Show resolved Hide resolved

src/client/renderingengine.h Outdated Show resolved Hide resolved

chmodsayshello added 2 commits May 9, 2024 13:52

fix stray change

6232062

fix indefinite loading animation

67b5a8c

fix far clouds being darker

a428632

chmodsayshello requested a review from grorp May 9, 2024 18:51

undo unnecessary changes

553ea89

chmodsayshello commented May 9, 2024

View reviewed changes

src/client/game.cpp Outdated Show resolved Hide resolved

grorp closed this May 9, 2024

grorp reopened this May 9, 2024

'support' multiplayer shutdown screen

005d5e7

grorp reviewed May 9, 2024

View reviewed changes

src/client/clientlauncher.cpp Outdated Show resolved Hide resolved

chmodsayshello added 2 commits May 9, 2024 23:12

don't retain infdef_pos between shutdowns

eeba1f5

set fog color instead of disabling

fea3d17

chmodsayshello requested a review from grorp May 9, 2024 21:17

chmodsayshello force-pushed the shutdownScreen branch from 1cb0286 to 23f77c8 Compare May 9, 2024 21:37

replace an overlayMessage with loading screen

6bc06fd

chmodsayshello force-pushed the shutdownScreen branch from 23f77c8 to 6bc06fd Compare May 9, 2024 21:38

grorp added 3 commits May 10, 2024 14:27

Some cleanup

5d6b6eb

Clean up draw_sky argument (always true after this PR)

88aa81f

Fix milliseconds / seconds confusion

4bd88bc

grorp reviewed May 10, 2024

View reviewed changes

Undo changes to loop that never runs anyway

22017ab

grorp approved these changes May 10, 2024

View reviewed changes

grorp removed the Action / change needed Code still needs changes (PR) / more information requested (Issues) label May 10, 2024

Avoid progress bar jump after pressing ESC during "Creating server..."

9eb5629

Zughy added >= Two approvals ✅ ✅ and removed One approval ✅ ◻️ labels May 20, 2024

grorp merged commit ab783b9 into minetest:master May 21, 2024
13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Display background & moving progress bar on shutdown screen #14597

Display background & moving progress bar on shutdown screen #14597

chmodsayshello commented Apr 27, 2024 •

edited

Zughy commented Apr 28, 2024 •

edited

appgurueu left a comment

grorp commented Apr 28, 2024 •

edited

grorp left a comment

chmodsayshello commented Apr 28, 2024

chmodsayshello commented Apr 29, 2024

grorp left a comment

chmodsayshello commented May 9, 2024 •

edited by grorp

grorp commented May 9, 2024 •

edited

grorp commented May 9, 2024

chmodsayshello commented May 9, 2024 •

edited

chmodsayshello commented May 9, 2024 •

edited

chmodsayshello commented May 9, 2024 •

edited

grorp May 10, 2024

grorp May 10, 2024

chmodsayshello May 10, 2024

grorp May 10, 2024

grorp left a comment

	bool Client::isShutdown()
	{
	return m_shutdown \|\| !m_mesh_update_manager->isRunning();
	}

Display background & moving progress bar on shutdown screen #14597

Display background & moving progress bar on shutdown screen #14597

Conversation

chmodsayshello commented Apr 27, 2024 • edited

How to test

Zughy commented Apr 28, 2024 • edited

appgurueu left a comment

Choose a reason for hiding this comment

grorp commented Apr 28, 2024 • edited

grorp left a comment

Choose a reason for hiding this comment

chmodsayshello commented Apr 28, 2024

chmodsayshello commented Apr 29, 2024

grorp left a comment

Choose a reason for hiding this comment

chmodsayshello commented May 9, 2024 • edited by grorp

grorp commented May 9, 2024 • edited

grorp commented May 9, 2024

chmodsayshello commented May 9, 2024 • edited

chmodsayshello commented May 9, 2024 • edited

chmodsayshello commented May 9, 2024 • edited

grorp May 10, 2024

Choose a reason for hiding this comment

grorp May 10, 2024

Choose a reason for hiding this comment

chmodsayshello May 10, 2024

Choose a reason for hiding this comment

grorp May 10, 2024

Choose a reason for hiding this comment

grorp left a comment

Choose a reason for hiding this comment

chmodsayshello commented Apr 27, 2024 •

edited

Zughy commented Apr 28, 2024 •

edited

grorp commented Apr 28, 2024 •

edited

chmodsayshello commented May 9, 2024 •

edited by grorp

grorp commented May 9, 2024 •

edited

chmodsayshello commented May 9, 2024 •

edited

chmodsayshello commented May 9, 2024 •

edited

chmodsayshello commented May 9, 2024 •

edited