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

Add "Mute Game" toggle in Game view #99555

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Meorge
Copy link
Contributor

@Meorge Meorge commented Nov 22, 2024

This should close godotengine/godot-proposals#1066.

It adds a button to the Game view's toolbar that disables all audio output from the game when toggled off.

Godot.Mute.Audio.Button.mp4

Things to consider:

  • The code was structured to mimic the camera override implementation. It seems to be working well, but I'm feeling a little iffy about having separate audio_enabled members for EditorDebuggerNode and ScriptEditorDebugger, rather than a single source of truth for whether or not audio is enabled.
  • Currently, it defaults to being enabled, and stays in sync with the AudioServer simply by being the only object to modify it. We may want to add a signal or something to the AudioServer that emits whenever the mute variable is changed, so that the button's toggled state can update appropriately if mute is changed from elsewhere.
  • The current icon may be a bit confusing. It could be good to create a crossed-out version of the AudioStreamPlayer icon to indicate that sound is muted.
  • Similarly, the meaning of the button is a bit mixed. It's technically an "enable audio" button (as in, when toggled on, audio plays). The addition to AudioServer, on the other hand, is a mute variable (when true, audio doesn't play.) I did this to keep it consistent with set_bus_mute, but I'm wondering if one direction (true = mute, or true = audible) should be chosen for this whole flow instead.

@YeldhamDev
Copy link
Member

The button should disable audio when toggled on, since the audio being enabled is the default state. Having buttons be in a pressed state as the default unnecessarily draws attention to them.

@Meorge
Copy link
Contributor Author

Meorge commented Nov 24, 2024

It sounds like Unity's equivalent of this is also a "mute audio" button and not an "enable audio" button. Between that (i.e., standardizing) and the point you brought up about muted audio not being the default behavior, I agree that it'd make more sense for the button's pressed state to make the audio not play.

It also occurred to me that this should perhaps be made into either a project-level or editor-level setting, so that it is kept on closing and re-opening the editor. At present it always defaults to not muting, which I could see being annoying. I'll look more into how to save its state.

@tetrapod00
Copy link
Contributor

It also occurred to me that this should perhaps be made into either a project-level or editor-level setting, so that it is kept on closing and re-opening the editor. At present it always defaults to not muting, which I could see being annoying. I'll look more into how to save its state.

I'm not an expert in the norms for editor code but I believe this is a good use case for using project metadata. The editor can remember the toggled state of the mute button, across multiple tests of the game and multiple editor sessions, without the need for a project or editor setting that the user has to go menus to change.

@Calinou
Copy link
Member

Calinou commented Nov 24, 2024

  • The current icon may be a bit confusing. It could be good to create a crossed-out version of the AudioStreamPlayer icon to indicate that sound is muted.

Here you go 🙂

mute_audio_icons.zip

MuteAudio2 is my favorite personally, but MuteAudio3 may look better at 16x16.

@Meorge
Copy link
Contributor Author

Meorge commented Nov 24, 2024

Thank you very much for the icons! 😄

Currently, I have it set up so that it displays the regular AudioStreamPlayer icon when not enabled to indicate that audio will play, and then switches to the muted icon when it is toggled on:

Godot.Mute.Audio.Button.with.Icons.mp4

The icons themselves look great, but unfortunately the blue tint that Godot applies makes it a bit hard to read the red strikethrough, in my opinion... What are your thoughts?

@tetrapod00
Copy link
Contributor

IMO the strikethrough does not need to be red; it can be whatever the color of the surrounding icon is. The toggled style and difference in icon is already sufficient to distinguish the states.

@Meorge
Copy link
Contributor Author

Meorge commented Nov 24, 2024

I suppose we can see what others think as well, but I personally like the red "highlight" of the strikethrough - it draws my attention to the fact that this isn't an ordinary audio icon, that something is very different about its current state.

Perhaps Godot's icon support already has something like this, and/or perhaps it doesn't and it would be considered unnecessary or infeasible. But it would be neat if certain portions of an SVG could be marked as "don't recolor for the selected variation", so that in spots like these, the highlighted parts would retain their contrast against the rest of the icon.

@Meorge Meorge requested a review from a team as a code owner November 24, 2024 23:35
@Meorge
Copy link
Contributor Author

Meorge commented Nov 24, 2024

There's currently a bug where, if the "mute audio" toggle is activated before the game is run, you can hear the game for a split second before the mute takes effect. I have a feeling this may have to do with the messaging system that it uses to send events to the game taking a frame to process, and if so I'm not entirely sure how this would be gotten around 😅

@Meorge
Copy link
Contributor Author

Meorge commented Nov 25, 2024

Following Calinou's suggestions in the contributor chat, I changed the way the button works to match the Skip Breakpoints button in the debugger. This looks a lot better to me!

Godot.Mute.Audio.Button.with.Icons.No.Tint.Adaptive.Tooltips.mp4

The bright red strikethrough feels much more prominent, and indicates that the game is currently muted to me.

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally, it works as expected (including if you stop the project and run it again with game audio muted).

Note that the option doesn't persist across editor restarts (only game restarts), which is good since it avoids accidentally forgetting about it and wondering why the game doesn't play any audio.

@Meorge
Copy link
Contributor Author

Meorge commented Nov 26, 2024

Thanks for reviewing and approving! 😄

it works as expected (including if you stop the project and run it again with game audio muted).

I'm glad this is working for you, but on my end when I run it with game audio muted, I can hear it for a split second. Is it completely silent for you the whole time?

Godot.Mute.Audio.Button.Gives.Frame.of.Audio.At.Beginning.mp4

Note that the option doesn't persist across editor restarts (only game restarts), which is good since it avoids accidentally forgetting about it and wondering why the game doesn't play any audio.

I agree that forgetting about it, then being confused why no audio is playing is a valid concern - I think that may have happened to me in the past with Unity, actually. Given that, perhaps it makes sense to leave the behavior that way for now, and consider possibilities for saving in a follow-up proposal/discussion. I could see there being cases, though, where someone may want to keep the setting off and not need to disable it every time they start the editor. Perhaps we could do something like have a toast notification appear on project startup warning the user that the mute setting is enabled?

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

Successfully merging this pull request may close these issues.

Add a "Mute Audio" toggle to the editor
5 participants