Allow for Game-Specific Menu Music #11241

ExeVirus · 2021-05-04T02:31:41Z

Goal of the PR

Allow games to provide their own menu music theme ~~, via a file of the same name as the game name in /menu~~. For example, The game labyrinth would have a music file at "/games/labyrinth/menu/theme.ogg"

Implementation

Old menu music could be provided via a "/sounds/main_menu.ogg" file (or "main_menu.#.ogg") by users if they desired.

That functionality still exists in this implementation, but now in the single-player tab, a game can provide it's own music to be played when the tab is open. If the game does not provide it's own music, no music is played.

~~To safely stop main menu music when switching to this tab or vice versa, I implemented a function that I thought has been missing from minetest for sometime, and was needed in this case:~~

~~#### sound_stop_all()~~

~~This function stops all currently playing sounds, without a user needing to provide a handle~~

How to test

Download an example game with a menu music file: Labyrinth on ContentDB and click on that game in the singleplayer tab. You will hear some music playing. To test the main menu music, feel free to copy that music file to minetest_folder/sounds/ and rename it to "main_menu.ogg". I recommend using a different sound file to see the differences of when music plays or doesn't.

Relevant Issues

#8596

Ready for Review.

builtin/mainmenu/tab_local.lua

src/gui/guiEngine.cpp

src/server.cpp

ExeVirus · 2021-05-16T14:21:26Z

Sure I'll get to it within the next few days, all good suggestions, wasn't aware of game.id I believe. Let me know if you see anything else Joker

…

On Sun, May 16, 2021, 8:31 AM SmallJoker ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/server.cpp <#11241 (comment)>: > + for (std::unordered_set<session_t>::const_iterator si = + psound.clients.begin(); + si != psound.clients.end(); ++si) { How about for (session_t peer_id : psound.clients) { ? That's a bit shorter. Same for the m_playing_sounds loop. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#11241 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKT7N2WJH5DDDMUK4ZJ5ZNTTN63LZANCNFSM44B3NSWA> .

ExeVirus · 2021-05-18T02:54:32Z

Okay, good to go, much cleaner implementation, thanks for the examples. 👍

builtin/mainmenu/tab_local.lua

SmallJoker · 2021-05-19T12:23:53Z

Directory structure:

????/games/minetest_game/menu$ ls
header.png  icon.png  minetest.ogg

Log output:

2021-05-19 14:18:38: [Main]: Playing sound: devtest
2021-05-19 14:18:39: [Main]: Playing sound: minetest

The sound file is not played back (using a renamed carts_cart_moving.1.ogg). Also Playing sound appears even when the selected game did not change. I presume it would restart the sound, which is not ideal.

Also: Menu content uses consistent names. Wouldn't it be possible to name the file theme.ogg and play that in the mainmenu, rather than fetching all possible sound locations?

ExeVirus · 2021-05-20T01:22:06Z

So a few things:

The name of minetest_game is minetest_game, not minetest. I ask that dev's use the game.id (game name) as the menu theme.
Naming every game's theme to be "theme.ogg" results in name collisions, and my current implementation doesn't make sorting that out easy.
It does restart the sound, not ideal. But again, how many people double click the game they want to play when selecting it? I'll see if I can make it not restart on double click.
As for your last question, yes this can be supported. Looking at the texture code, this can be supported if I change how the lua code is loading it in built-in. Give me a bit to rework.

Edit: My mistake, minetest's id is different from it's directory name, this is why I made the change originally I believe from using game.id. But I will make it work so that theme.ogg is the name not <game_name>.ogg

ExeVirus · 2021-05-20T02:02:30Z

Not quite as "clean" of a local function as before, but now menu music can simply be called "theme.ogg" / "theme.1.ogg" as you suggested. I have also made it so that re-clicking the game button does not restart the theme from the beginning. Thanks for the push

ExeVirus · 2021-06-09T02:10:09Z

Okay, sound_stop_all is no more.

Instead added a music_player.lua to the main_menu builtin, it provides a single function that manages the current music handle. When you provide a new file to play, the current is stopped and the new one is played on loop. If you try to play the same theme.ogg again (or main_menu.ogg) nothing occurs and play continues.

ShadowNinja

I think the way that the files are loaded should be cleaned up a bit, also there are some style things.

ShadowNinja · 2021-06-09T23:51:20Z

builtin/mainmenu/music_player.lua

@@ -0,0 +1,32 @@
+--Minetest
+--Copyright (C) 2014 sapier


This copyright notice should be updated.

To what, particularly? That was copied from the other main menu lua files. Should we instead leave that for a future PR cleaning up the copyrights for all of them?

Just update the author and the date.

builtin/mainmenu/music_player.lua

builtin/mainmenu/tab_local.lua

src/gui/guiEngine.cpp

ExeVirus · 2021-06-11T02:42:35Z

Okay besides the potentially postponed copyright update and possibly more efficient directory search, the requested changes have been committed.

P.S. thanks for the timely review 👍

builtin/mainmenu/music_player.lua

builtin/mainmenu/tab_local.lua

src/gui/guiEngine.cpp

ShadowNinja · 2021-09-22T00:54:11Z

builtin/mainmenu/music_player.lua

@@ -0,0 +1,32 @@
+--Minetest
+--Copyright (C) 2014 sapier


Just update the author and the date.

Allows Menu music to be provided for indivudal games in games/gamename/menu/gamename.ogg Add sound_stop_all() lua_api function, no handles required Conflicts: src/gui/guiEngine.cpp

- Use pkgmgr.games[j].id correctly - clean-up gui/guiEngine.cpp to use local function - use C++ goodies to clean up src/server.cpp

…or full paths for menu music

ExeVirus · 2021-09-24T18:35:40Z

Alrighty, @ShadowNinja @SmallJoker ready for hopefully final review/approval.

ShadowNinja

Looks good other than some tiny nits that could be fixed when merging.

src/gui/guiEngine.cpp

rubenwardy · 2021-10-31T20:58:25Z

When you open a dialog, the game theme still applies (header.png / background.png) but the music stops.

I'd consider changing textures.lua to game_theme.lua, and merging the sound stuff into it - allowing both to be controlled at the same time

ExeVirus · 2021-10-31T21:06:47Z

Sounds reasonable, I'll have to take a look at of that would still work without breaking the current main menu music system (which plays for the multiplayer tab for example). Or are you saying just remove the main menu music system we have and use this instead for all tabs/dialogs?

…

On Sun, Oct 31, 2021, 4:58 PM rubenwardy ***@***.***> wrote: When you open a dialog, the game theme still applies (header.png / background.png) but the music stops. I'd consider changing textures.lua to game_theme.lua, and merging the sound stuff into it - allowing both to be controlled at the same time — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#11241 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKT7N2SMMXZCAYLCV6L5HBTUJWUXXANCNFSM44B3NSWA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

rubenwardy · 2021-10-31T21:29:57Z

I'll have to take a look at of that would still work without breaking the
current main menu music system (which plays for the multiplayer tab for
example).

The game theming already shows on the local tab, but not the multiplayer tab. So you'd just play the global mainmenu music when leaving the local tab, when you hide the game theme

- rename the internal textures.lua table to mm_game_theme - move the music play code to game_theme to fix dialogs stopping music - Fix Errant \n removal - Fix guiEngine extraneous \n

ExeVirus · 2021-11-19T14:42:26Z

@rubenwardy

Tested on local machine(working) and updated with your suggested changes, feel free to merge if you are satisfied with it

rubenwardy · 2021-11-19T22:29:59Z

@ShadowNinja: does your approval on this still hold?

ShadowNinja · 2021-11-20T22:41:24Z

Yep, looks good.

ExeVirus force-pushed the GameSpecificMusic branch from ccf67cf to fd2a69e Compare May 4, 2021 02:39

SmallJoker reviewed May 16, 2021

View reviewed changes

builtin/mainmenu/tab_local.lua Outdated Show resolved Hide resolved

SmallJoker reviewed May 16, 2021

View reviewed changes

src/gui/guiEngine.cpp Outdated Show resolved Hide resolved

SmallJoker reviewed May 16, 2021

View reviewed changes

src/server.cpp Outdated Show resolved Hide resolved

SmallJoker added @ Client / Audiovisuals @ Mainmenu Feature ✨ PRs that add or enhance a feature labels May 16, 2021

SmallJoker reviewed May 18, 2021

View reviewed changes

builtin/mainmenu/tab_local.lua Outdated Show resolved Hide resolved

ExeVirus changed the title ~~Allow for Game-Specific Menu Music and Add needed sound_stop_all()~~ Allow for Game-Specific Menu Music Jun 9, 2021

ExeVirus requested a review from SmallJoker June 9, 2021 02:22

ShadowNinja requested changes Jun 10, 2021

View reviewed changes

ExeVirus requested a review from ShadowNinja June 13, 2021 17:41

ShadowNinja reviewed Sep 22, 2021

View reviewed changes

ExeVirus added 8 commits September 24, 2021 11:55

Allow for Game-Specific Menu Music and Add sound_stop_all()

36524ba

Allows Menu music to be provided for indivudal games in games/gamename/menu/gamename.ogg Add sound_stop_all() lua_api function, no handles required Conflicts: src/gui/guiEngine.cpp

SmallJoker corrections

f2f3f4f

- Use pkgmgr.games[j].id correctly - clean-up gui/guiEngine.cpp to use local function - use C++ goodies to clean up src/server.cpp

Use game.id instead of pattern matching

19674d0

Update menu music names to "theme.ogg" and similar

783e386

Update to use music_player.lua for mainmenu to manage music handles

f9287ee

Remove sound_stop_all from codebase. May be useful later, but not now.

b3122a0

ShadowNinja Requested Changes (Comments, Cleanup)

99fb9ad

Remove async send information.

805e419

ExeVirus force-pushed the GameSpecificMusic branch from 38839b4 to 805e419 Compare September 24, 2021 16:48

Update copyright, remove redudant current playing file check, allow f…

98dd502

…or full paths for menu music

ExeVirus force-pushed the GameSpecificMusic branch from 9b67399 to 98dd502 Compare September 24, 2021 18:19

ShadowNinja approved these changes Oct 9, 2021

View reviewed changes

src/gui/guiEngine.cpp Outdated Show resolved Hide resolved

src/gui/guiEngine.cpp Outdated Show resolved Hide resolved

ShadowNinja added the One approval ✅ ◻️ label Oct 16, 2021

rubenwardy added the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Oct 31, 2021

ExeVirus force-pushed the GameSpecificMusic branch from a347450 to 341e85f Compare November 5, 2021 13:58

- Rename textures.lua to game_theme.lua

3e9e8c5

- rename the internal textures.lua table to mm_game_theme - move the music play code to game_theme to fix dialogs stopping music - Fix Errant \n removal - Fix guiEngine extraneous \n

ExeVirus force-pushed the GameSpecificMusic branch from 341e85f to 3e9e8c5 Compare November 19, 2021 14:10

rubenwardy removed the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Nov 19, 2021

rubenwardy self-requested a review November 19, 2021 17:50

rubenwardy approved these changes Nov 19, 2021

View reviewed changes

rubenwardy added >= Two approvals ✅ ✅ and removed One approval ✅ ◻️ labels Nov 19, 2021

rubenwardy merged commit 52bfbf6 into minetest:master Nov 22, 2021

rubenwardy mentioned this pull request May 15, 2022

Game-provided menu music #8596

Closed

Desour mentioned this pull request Jul 13, 2023

Fix sound manager not being stepped by GUIEngine #13661

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow for Game-Specific Menu Music #11241

Allow for Game-Specific Menu Music #11241

ExeVirus commented May 4, 2021 •

edited

ExeVirus commented May 16, 2021 via email

ExeVirus commented May 18, 2021

SmallJoker commented May 19, 2021

ExeVirus commented May 20, 2021 •

edited

ExeVirus commented May 20, 2021 •

edited

ExeVirus commented Jun 9, 2021

ShadowNinja left a comment

ShadowNinja Jun 9, 2021

ExeVirus Jun 11, 2021

ShadowNinja Sep 22, 2021

ExeVirus commented Jun 11, 2021 •

edited

ShadowNinja Sep 22, 2021

ExeVirus commented Sep 24, 2021

ShadowNinja left a comment

rubenwardy commented Oct 31, 2021

ExeVirus commented Oct 31, 2021 via email

rubenwardy commented Oct 31, 2021

ExeVirus commented Nov 19, 2021

rubenwardy commented Nov 19, 2021

ShadowNinja commented Nov 20, 2021

Allow for Game-Specific Menu Music #11241

Allow for Game-Specific Menu Music #11241

Conversation

ExeVirus commented May 4, 2021 • edited

Goal of the PR

Implementation

How to test

Relevant Issues

Ready for Review.

ExeVirus commented May 16, 2021 via email

ExeVirus commented May 18, 2021

SmallJoker commented May 19, 2021

ExeVirus commented May 20, 2021 • edited

ExeVirus commented May 20, 2021 • edited

ExeVirus commented Jun 9, 2021

ShadowNinja left a comment

Choose a reason for hiding this comment

ShadowNinja Jun 9, 2021

Choose a reason for hiding this comment

ExeVirus Jun 11, 2021

Choose a reason for hiding this comment

ShadowNinja Sep 22, 2021

Choose a reason for hiding this comment

ExeVirus commented Jun 11, 2021 • edited

ShadowNinja Sep 22, 2021

Choose a reason for hiding this comment

ExeVirus commented Sep 24, 2021

ShadowNinja left a comment

Choose a reason for hiding this comment

rubenwardy commented Oct 31, 2021

ExeVirus commented Oct 31, 2021 via email

rubenwardy commented Oct 31, 2021

ExeVirus commented Nov 19, 2021

rubenwardy commented Nov 19, 2021

ShadowNinja commented Nov 20, 2021

ExeVirus commented May 4, 2021 •

edited

ExeVirus commented May 20, 2021 •

edited

ExeVirus commented May 20, 2021 •

edited

ExeVirus commented Jun 11, 2021 •

edited