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

Show splash screen earlier and reintroduce intro music #6412

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

Conversation

bunnybot
Copy link

@bunnybot bunnybot commented Mar 14, 2024

tothxaMirrored from Codeberg
Created on Thu Mar 14 15:41:15 CET 2024 by Tóth András (tothxa)


Type of change
New feature / Refactoring

Issue(s) closed

New behavior

  • Cleaned up intro music related code
  • Cleaned up splash screen handling in MainMenu
  • Always play the menu music in the main menu
  • Added a new hourglass mouse cursor that is used for the splash screen, the game loading screen, and some other lengthy operations. Always use SDL cursor mode during startup to keep it visible.
  • Don't switch music for the game/map loading screens (it doesn't take that long ;)
  • Added more (verbose) logging points for the start-up (for timing info), moved OpenGL features logging to verbose.
  • Added a rect() method for class Image and converted found applicable places

Possible regressions
Start up, music switching, mouse cursor initialisation and switching

Additional context
This is the alternative PR promised in #6409 / https://codeberg.org/wl/widelands/issues/4759#issuecomment-1660296

I'd turn all log_info() calls to verb_log_info() at least for the start-up, but maybe that'd be better in a separate PR (also implementing multiple choices instead of just verbose or not).

@bunnybot bunnybot added this to the v1.3 milestone Mar 14, 2024
@bunnybot bunnybot self-assigned this Mar 14, 2024
@bunnybot
Copy link
Author

Assigned to tothxa

@bunnybot bunnybot added enhancement New feature or request cleanup & refactoring Improving our code quality sounds & music Sound effects, music tracks, sound handler labels Mar 14, 2024
@bunnybot
Copy link
Author

frankystoneMirrored from Codeberg
On Thu Mar 14 17:36:24 CET 2024, frankystone wrote:


When using a theme add-on this loads the default splash screen image and not the one of the add-on. In result the default splash is shown and close after that the splash from the add-on is shown and then the main menu.

Is there a way to slow down the startup for testing purposes? Some when i noticed that filling the image cache slows down the startup, e.g. with minimalistic theme a black screen is shown for a short time (~1 sec.) before the splash screen of the addon is shown.

@bunnybot
Copy link
Author

NordfrieseMirrored from Codeberg
On Thu Mar 14 17:47:41 CET 2024, Benedikt Straub (Nordfriese) wrote:


[173/524] Building CXX object src/CMakeFiles/widelands_ball_of_mud.dir/wlapplication.cc.o
FAILED: src/CMakeFiles/widelands_ball_of_mud.dir/wlapplication.cc.o 
/home/benedikt/wl/g/src/wlapplication.cc:1015:24: error: suggest parentheses around assignment used as truth value [-Werror=parentheses]
 1015 |         assert(ev.type = SDL_WINDOWEVENT);

@bunnybot
Copy link
Author

NordfrieseMirrored from Codeberg
On Thu Mar 14 17:55:20 CET 2024, Benedikt Straub (Nordfriese) wrote:


After fixing the =/== typo it compiled, but I am still not really happy with this change. In my opinion the intro music doesn't really fit to the gameplay and is better suited to the first startup.

Big +1 for the other cleanup and refactoring changes though :)

@bunnybot
Copy link
Author

tothxaMirrored from Codeberg
On Thu Mar 14 19:52:17 CET 2024, Tóth András (tothxa) wrote:


When using a theme add-on this loads the default splash screen image and not the one of the add-on. In result the default splash is shown and close after that the splash from the add-on is shown and then the main menu.

Oops, I was a bit confused about that. I'm afraid that's very hard to solve, as I don't think we could get this picture from the configured add-on theme early enough.

Is there a way to slow down the startup for testing purposes?

You can add SDL_Delay(<in_millisec>) where you would like to have some...


<@>Nordfriese Thanks, fixed. Why didn't the workflow checks got this?

@bunnybot
Copy link
Author

tothxaMirrored from Codeberg
On Thu Mar 14 22:28:01 CET 2024, Tóth András (tothxa) wrote:


<@>frankystone The last commit should fix the custom splash.jpg problem for most normal use cases, except for the first time it is run. (on first install it should work fine, because in that case there should be no addons) For consecutive runs it should work as long as the user doesn't tamper with the config file and the addons directory.

I can imagine another failure case: if the theme is changed but the game crashes without saving the config.

So this is a simple, mostly cleanish pseudo-solution for 99% of the cases. I hope that suffices...

@bunnybot
Copy link
Author

frankystoneMirrored from Codeberg
On Sat Mar 16 10:29:55 CET 2024, frankystone wrote:


<@>frankystone The last commit should fix the custom splash.jpg problem for most normal use cases, except for the first time it is run. (on first install it should work fine, because in that case there should be no addons) For consecutive runs it should work as long as the user doesn't tamper with the config file and the addons directory.

I can imagine another failure case: if the theme is changed but the game crashes without saving the config.

So this is a simple, mostly cleanish pseudo-solution for 99% of the cases. I hope that suffices...

Yes, that's ok, imho.

@bunnybot
Copy link
Author

bunnybot commented Jun 1, 2024

NordfrieseMirrored from Codeberg
On Sat Jun 01 15:44:29 CEST 2024, Benedikt Straub (Nordfriese) wrote:


Perfect for the keystroke handling now :)

The cursor still jumps erratically, but now to an arbitrary position somewhere in the window. Perhaps this is again the auto-maximizing of the window manager, and Widelands retrieves the mouse coordinates in the window directly after creation and then warps the cursor to keep the in-window coordinates of the mouse pointer constant after the automatic window size change? (just a guess)

@bunnybot bunnybot added the ci:cancelled CI checks were cancelled label Jun 1, 2024
@bunnybot
Copy link
Author

bunnybot commented Jun 1, 2024

tothxaMirrored from Codeberg
On Sat Jun 01 16:35:46 CEST 2024, Tóth András (tothxa) wrote:


<@>Nordfriese <@>hessenfarmer Could you please post the console output / stdout.txt with the last commit?

@bunnybot bunnybot removed the ci:cancelled CI checks were cancelled label Jun 1, 2024
@bunnybot
Copy link
Author

bunnybot commented Jun 1, 2024

NordfrieseMirrored from Codeberg
On Sat Jun 01 16:41:33 CEST 2024, Benedikt Straub (Nordfriese) wrote:


[00:00:00.730 real] DEBUG: Initializing the mouse cursor in SDL mode
[00:00:00.730 real] DEBUG: reported mouse position: 0 0
[00:00:00.730 real] DEBUG: window position: 704 368
[00:00:00.730 real] DEBUG: calculated mouse position: 860 633
[00:00:00.733 real] DEBUG: mouse position after warping: 860 633
[00:00:00.744 real] INFO: Font handler created
[00:00:00.769 real] INFO: Style Manager: Reading style templates took 25ms
[00:00:00.777 real] INFO: Loaded default styles
[00:00:00.777 real] DEBUG: handling mouse motion: 1564 945
[00:00:00.777 real] DEBUG: handling mouse motion: 859 632
[00:00:00.777 real] DEBUG: handling mouse motion: 860 633
[00:00:00.777 real] INFO: Handled 10 SDL window events at start up
[00:00:00.956 real] INFO: Splash screen shown
[00:00:00.956 real] INFO: Rebuilding texture atlas
[00:00:05.228 real] INFO: Texture atlas rebuilt
[00:00:05.228 real] INFO: Cleaning up temporary files
[00:00:05.252 real] INFO: Loading songsets
[00:00:05.253 real] INFO: Initializing Add-Ons
[00:00:05.266 real] INFO: Done initializing Add-Ons
[00:00:05.266 real] INFO: Developer tools are enabled.
[00:00:05.267 real] INFO: WLApplication created
[00:00:05.269 real] INFO: Setting logic thread.
[00:00:05.582 real] INFO: Registering the descriptions took 168ms
[00:00:05.602 real] INFO: Initializing economy serial
[00:00:05.752 real] INFO: lastserial: 0
[00:00:05.984 real] INFO: Registering the descriptions took 166ms
[00:00:06.003 real] INFO: Initializing economy serial
[00:00:06.090 real] INFO: lastserial: 0
[00:00:06.105 real] INFO: Initiating splash screen fade out
[00:00:10.138 real] INFO: Splash screen ended

@bunnybot
Copy link
Author

bunnybot commented Jun 1, 2024

tothxaMirrored from Codeberg
On Sat Jun 01 17:37:09 CEST 2024, Tóth András (tothxa) wrote:


Thanks.

I suppose the initial mousemove event (1564, 945) is generated to update the position when the window is resized and moved. This makes sense.

What doesn't make sense is how dropping that event or setting mouse_position_ based on it (or at all) can make any difference in SDL mode...

Let's wait for <@>hessenfarmer's report, then I'll try moving init_mouse_cursor() for SDL mode after the initial event loop.

@bunnybot
Copy link
Author

bunnybot commented Jun 1, 2024

tothxaMirrored from Codeberg
On Sat Jun 01 18:51:58 CEST 2024, Tóth András (tothxa) wrote:


Sorry for not waiting after all. But if this doesn't solve it, I don't have any idea what could...

@bunnybot bunnybot added the ci:success CI checks succeeded label Jun 1, 2024
@hessenfarmer
Copy link
Contributor

hessenfarmer commented Jun 1, 2024

Well i tested the last commit in all 4 modes resulting from system Mouse cursor and widelands mouse cursor and Full screen and windowed mode.
cursor position and switch was fine everywhere (except the little flick in full screen) so the position is fixed.
3 little observations regarding the option to use system cursor.

  • when the option is selcted the widelands cursor styles are displayed although the option text suggests it the other way round.
  • if deselcted the windows waiting cursor is shown but for the last 5 seconds before the widelands cursor is shown (the splash can be clicked away) no cursor is shown.
  • I never deselected the option before so I do not know whether the usage of the widelands cursor in game with the option deselected is a bug.

but these should not uphold this PR. for completeness here my log

This is Widelands version 1.3~git26710 (12a81cb@mirror/tothxa/widelands/splash-early) Release
[00:00:00.000 real] INFO: Set home directory: C:\Users\Stephan.widelands
[00:00:00.001 real] INFO: Set configuration file: C:\Users\Stephan.widelands\config
[00:00:00.005 real] INFO: Adding data directory: D:\development\Widelands_Git\tothxa\data
[00:00:00.006 real] WARNING: No locale translations found in D:\development\Widelands_Git\tothxa\data\i18n\translations
[00:00:00.014 real] INFO: Setting initializer thread.
[00:00:00.014 real] INFO: Byte order: little-endian
[00:00:00.020 real] INFO: **** SOUND REPORT ****
[00:00:00.020 real] INFO: SDL version: 2.30.3
[00:00:00.020 real] INFO: SDL_mixer version: 2.8.0
[00:00:00.020 real] INFO: **** END SOUND REPORT ****
[00:00:00.153 real] INFO: Songset: Loaded song "music\intro.ogg"
[00:00:00.272 real] INFO: Graphics: Try to set Videomode 1480x877
[00:00:00.359 real] INFO: Graphics: OpenGL: Version "4.6.0 NVIDIA 456.71"
[00:00:00.360 real] INFO: Graphics: SDL_GL_RED_SIZE is 8
[00:00:00.360 real] INFO: Graphics: SDL_GL_GREEN_SIZE is 8
[00:00:00.360 real] INFO: Graphics: SDL_GL_BLUE_SIZE is 8
[00:00:00.360 real] INFO: Graphics: SDL_GL_ALPHA_SIZE is 0
[00:00:00.361 real] INFO: Graphics: SDL_GL_BUFFER_SIZE is 24
[00:00:00.361 real] INFO: Graphics: SDL_GL_DOUBLEBUFFER is 1
[00:00:00.361 real] INFO: Graphics: SDL_GL_DEPTH_SIZE is 24
[00:00:00.361 real] INFO: Graphics: SDL_GL_STENCIL_SIZE is 0
[00:00:00.361 real] INFO: Graphics: SDL_GL_ACCUM_RED_SIZE is 16
[00:00:00.361 real] INFO: Graphics: SDL_GL_ACCUM_GREEN_SIZE is 16
[00:00:00.362 real] INFO: Graphics: SDL_GL_ACCUM_BLUE_SIZE is 16
[00:00:00.362 real] INFO: Graphics: SDL_GL_ACCUM_ALPHA_SIZE is 16
[00:00:00.362 real] INFO: Graphics: SDL_GL_STEREO is 0
[00:00:00.362 real] INFO: Graphics: SDL_GL_MULTISAMPLEBUFFERS is 0
[00:00:00.362 real] INFO: Graphics: SDL_GL_MULTISAMPLESAMPLES is 0
[00:00:00.362 real] INFO: Graphics: SDL_GL_ACCELERATED_VISUAL is 1
[00:00:00.363 real] INFO: Graphics: SDL_GL_CONTEXT_MAJOR_VERSION is 2
[00:00:00.363 real] INFO: Graphics: SDL_GL_CONTEXT_MINOR_VERSION is 1
[00:00:00.363 real] INFO: Graphics: SDL_GL_CONTEXT_FLAGS is 0
[00:00:00.363 real] INFO: Graphics: SDL_GL_CONTEXT_PROFILE_MASK is 0
[00:00:00.363 real] INFO: Graphics: SDL_GL_SHARE_WITH_CURRENT_CONTEXT is 0
[00:00:00.363 real] INFO: Graphics: SDL_GL_FRAMEBUFFER_SRGB_CAPABLE is 1
[00:00:00.363 real] INFO: Graphics: OpenGL: Double buffering enabled
[00:00:00.363 real] INFO: Graphics: OpenGL: Max texture size: 32768
[00:00:00.363 real] INFO: Graphics: OpenGL: ShadingLanguage: "4.60 NVIDIA"
[00:00:00.373 real] INFO: **** GRAPHICS REPORT ****
[00:00:00.373 real] INFO: VIDEO DRIVER windows
[00:00:00.373 real] INFO: Display #0: 1920x1080 @ 120hz SDL_PIXELFORMAT_RGB888
[00:00:00.373 real] INFO: **** END GRAPHICS REPORT ****
[00:00:00.440 real] DEBUG: Ignoring window event: 1
[00:00:00.440 real] DEBUG: Ignoring window event: 12
[00:00:00.440 real] DEBUG: Ignoring SDL event 0x0302
[00:00:00.440 real] DEBUG: Ignoring window event: 10
[00:00:00.440 real] DEBUG: handling mouse motion: 338 637
[00:00:00.440 real] DEBUG: Ignoring window event: 4
[00:00:00.440 real] DEBUG: Ignoring window event: 6
[00:00:00.440 real] DEBUG: Handling window resize event: 1920x1080
[00:00:00.440 real] DEBUG: handling mouse motion: 338 637
[00:00:00.440 real] DEBUG: Ignoring window event: 3
[00:00:00.440 real] WARNING: Ignored 1 non-mousemove SDL events at start up
[00:00:00.440 real] INFO: Handled 7 SDL window events at start up
[00:00:00.440 real] DEBUG: Initializing the mouse cursor in SDL mode
[00:00:00.440 real] DEBUG: reported mouse position: 338 637
[00:00:00.464 real] INFO: Font handler created
[00:00:00.495 real] INFO: Style Manager: Reading style templates took 30ms
[00:00:00.505 real] INFO: Loaded default styles
[00:00:00.626 real] INFO: Splash screen shown
[00:00:00.626 real] INFO: Rebuilding texture atlas
[00:00:06.539 real] INFO: Texture atlas rebuilt
[00:00:06.539 real] INFO: Cleaning up temporary files
[00:00:06.663 real] INFO: Loading songsets
[00:00:06.673 real] INFO: Initializing Add-Ons
[00:00:06.747 real] INFO: Done initializing Add-Ons
[00:00:06.748 real] INFO: WLApplication created
[00:00:06.748 real] INFO: Setting logic thread.
[00:00:07.390 real] INFO: Registering the descriptions took 609ms
[00:00:07.390 real] INFO: Initializing economy serial
[00:00:07.556 real] INFO: lastserial: 0
[00:00:08.193 real] INFO: Registering the descriptions took 603ms
[00:00:08.193 real] INFO: Initializing economy serial
[00:00:09.306 real] INFO: lastserial: 0
[00:00:11.689 real] INFO: Splash screen ended
[00:00:12.311 real] INFO: Songset: Loaded song "music\menu.ogg"
[00:00:17.832 real] INFO: SoundHandler: Closing 1 time, 44100 Hz, format 32784, 2 channels
[00:00:17.832 real] INFO: SoundHandler: SDL_AUDIODRIVER wasapi`

@bunnybot
Copy link
Author

bunnybot commented Jun 1, 2024

NordfrieseMirrored from Codeberg
On Sat Jun 01 21:52:17 CEST 2024, Benedikt Straub (Nordfriese) wrote:


Mouse position works for me now :)

@bunnybot bunnybot removed the ci:success CI checks succeeded label Jun 1, 2024
@bunnybot
Copy link
Author

bunnybot commented Jun 1, 2024

tothxaMirrored from Codeberg
On Sat Jun 01 23:05:51 CEST 2024, Tóth András (tothxa) wrote:


Thanks to both of you for testing! :)

On Sat Jun 01 21:28:39 CEST 2024, hessenfarmer wrote:
(except the little flick in full screen)

We'd need some backtraces to debug that, but since it only manifests in fullscreen mode, I don't think we can get it by running Widelands in a debugger... We'd need a breakpoint at Graphic::refresh, and backtrace the second invocation, but I'm afraid that window switching to the debugger after the first break would ruin the context that causes the problem. I thought of making a branch to log the backtraces of the first 3 calls in debug builds based on <@>Nordfriese's earlier attempt at self debugging...

3 little observations regarding the option to use system cursor.

  • when the option is selcted the widelands cursor styles are displayed although the option text suggests it the other way round.
  • if deselcted the windows waiting cursor is shown but for the last 5 seconds before the widelands cursor is shown (the splash can be clicked away) no cursor is shown.
  • I never deselected the option before so I do not know whether the usage of the widelands cursor in game with the option deselected is a bug.

That's exactly the expected behaviour. :) We can set our own custom cursor in both modes. SDL mode means that SDL or the system handles drawing the cursor, otherwise we have to do it ourselves when it's inside our window. We do that in Panel::do_redraw_now(), so it's position and looks are only updated when that is called, which doesn't happen during startup. That's why I chose to only set up the mouse cursor in software mode at the end of WLApplication(), to have a visible cursor for as long as possible. Then it disappears until we start actually refreshing the window.

But now, writing this, I've realised that I could always init the cursor in SDL mode first to have our own graphics, then only switch to soft mode at the end of WLApplication(). I guess at worst those who use soft mode for compatibility would get no cursor at startup. Maybe I should still set system wait cursor for them before our own cursor init, to give it a chance? For everybody else it would mean they get our own hourglass, but it would then disappear for a few seconds. Or maybe I can postpone setting the mode until menu in WLApplication::run() is created? (that's the other slow part while last save and last replay are found)... Let's try it!

[00:00:00.440 real] DEBUG: Ignoring SDL event 0x0302

Hmmm, that's SDL_TEXTEDITING... what does that mean and why do you get that at program initialisation?

@hessenfarmer
Copy link
Contributor

That's exactly the expected behaviour. :) We can set our own custom cursor in both modes. SDL mode means that SDL or the system handles drawing the cursor, otherwise we have to do it ourselves when it's inside our window. We do that in Panel::do_redraw_now(), so it's position and looks are only updated when that is called, which doesn't happen during startup. That's why I chose to only set up the mouse cursor in software mode at the end of WLApplication(), to have a visible cursor for as long as possible. Then it disappears until we start actually refreshing the window.

But now, writing this, I've realised that I could always init the cursor in SDL mode first to have our own graphics, then only switch to soft mode at the end of WLApplication(). I guess at worst those who use soft mode for compatibility would get no cursor at startup. Maybe I should still set system wait cursor for them before our own cursor init, to give it a chance? For everybody else it would mean they get our own hourglass, but it would then disappear for a few seconds. Or maybe I can postpone setting the mode until menu in WLApplication::run() is created? (that's the other slow part while last save and last replay are found)... Let's try it!

well I just did not and still do not get what this option really does. and I wanted to just share that at least the option text and description might be misleading.

[00:00:00.440 real] DEBUG: Ignoring SDL event 0x0302

Hmmm, that's SDL_TEXTEDITING... what does that mean and why do you get that at program initialisation?

I have no idea. but it is displayed each time

@bunnybot
Copy link
Author

bunnybot commented Jun 1, 2024

tothxaMirrored from Codeberg
On Sat Jun 01 14:14:14 CEST 2024, Tóth András (tothxa) wrote:


There's another bug now: The cursor jumps to the top-left corner of the Widelands window when the splashscreen ends.

Is that in SDL or soft mode? (or both?) It was only an issue for me in soft mode (also in master), but I enabled the tap dance that solved it for me in SDL mode too now. Let's see if it helps either of you...

I also still think the behaviour for fading out in combination with a keystroke is not intuitive.

OK, the code was getting more trouble than it's worth anyway. It's always abort now. (that also means that multiple keypresses in the loading phase may still cause trouble, but solving that is best left to a separate PR at this point, my approach only partially addressed it anyway)

This is why I think that any keypress that is accepted as a shortcut by the main menu should skip the slow transition entirely right away.

Well, handling only known ones would take some more refactoring...

@bunnybot bunnybot added the ci:success CI checks succeeded label Jun 1, 2024
@bunnybot bunnybot removed the ci:success CI checks succeeded label Jun 1, 2024
@bunnybot bunnybot added ci:fail CI checks failed and removed ci:fail CI checks failed labels Jun 2, 2024
Copy link
Author

@bunnybot bunnybot left a comment

Choose a reason for hiding this comment

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

NordfrieseMirrored from Codeberg
On Sun Jun 02 11:16:51 CEST 2024, Benedikt Straub (Nordfriese) commented.

/** TRANSLATORS: tooltip text for the sdl_cursor option */
_("If in doubt, leave this enabled. When disabled, cursor updates may be slow "
"and the cursor appears frozen during long operations. Disable it only if "
"the cursor doesn't appear right, or if you want it to be visible in "
Copy link
Author

Choose a reason for hiding this comment

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

NordfrieseMirrored from Codeberg
On Sun Jun 02 11:19:17 CEST 2024, Benedikt Straub (Nordfriese) wrote:


'

@@ -701,6 +706,8 @@ void Panel::set_visible(bool const on) {
if (Panel* cm = find_context_menu(); cm != nullptr) {
cm->die();
}
} else if (name_ == "progresswindow") {
Copy link
Author

Choose a reason for hiding this comment

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

NordfrieseMirrored from Codeberg
On Sun Jun 02 11:16:51 CEST 2024, Benedikt Straub (Nordfriese) wrote:


Fragile, please make this a panel flag

@bunnybot bunnybot added ci:success CI checks succeeded and removed ci:fail CI checks failed labels Jun 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:success CI checks succeeded cleanup & refactoring Improving our code quality enhancement New feature or request graphics Animations, graphics files, graphics engine, OpenGL sounds & music Sound effects, music tracks, sound handler ui User interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reinstate the Intro Music
3 participants