Skip to content

Conversation

@msem-me
Copy link

@msem-me msem-me commented Nov 16, 2025

Work done

This PR introduces a new audio notification that plays for teammates when an allied player requests energy or metal. This provides clear, non-visual feedback for resource requests, which at present are only posted to chat and can be easily missed in a busy game.

This widget could be extended in the future to include additional audio and/or visual indicators for ally player requests. For example chat wheel functionality to allow quick team messages such as "Push now", "Building T2 lab" or "Fall back".

These could have visual and/or audio alert notifications, for now this only implements sound effects in addition to the existing chat messages.

It is implemented by hooking into the existing notification system:

  1. gui_advplayerslist.lua: Modified to send a new, unique alert:allyRequest:energy (or :metal) message when a player clicks the resource icons next to their own name. This is sent in addition to the existing msg:... message (which goes to chat).
  2. snd_notifications_addon_ally_alerts.lua (New Widget): A new, small, widget created to do the following:
    • Intercept Lua messages (RecvLuaMsg) broadcast by the player list.
    • Verify that the player is not a spectator and the sender is on the same AllyTeam as the local player, as well as not being a spectator (security check).
    • Pass the event to the global notification handler (WG['notifications'].queueNotification) to trigger the "AllyRequestMetal" or "AllyRequestEnergy" sounds.
  3. sounds/voice/config.lua: Added two new event definitions, AllyRequestEnergy and AllyRequestMetal. These are configured to only play a soundEffect, but could be ammended to include a voiceline.
  4. sounds/voice-soundeffects/AllyRequest.wav (New Asset): This is the new sound effect that is played.
  5. license_sounds.txt updated with license for AllyRequest.wav
  6. language/en/tips.json updated with appropriate messages.

The idea was to take a clean, modular approach using the game's existing notification system, so it will respect user settings for notification volume.

AllyRequest.wav was created by me from scratch using LMMS synthesizers. I hereby release the sound file AllyRequest.wav included in this PR under the CC0 1.0 Universal (CC0 1.0) Public Domain Dedication.

Test steps

  • Start a game with at least one teammate (e.g., in a 2v2).
  • As Player 1, open the player list.
  • Click the energy icon next to your own name (this is the 'request' action).
  • Expected Result: Player 2 (your teammate) should hear the new AllyRequest.wav sound.
  • Repeat the test by clicking the metal icon next to your own name.
  • Expected Result: Player 2 should hear the AllyRequest.wav sound again.
  • (Optional) Click the resource icons next to an ally's name.
  • Expected Result: No sound should play, as this is a "share" action, not a "request."

@sprunk
Copy link
Collaborator

sprunk commented Nov 18, 2025

This is pure UI so shouldn't involve a gadget. Use Spring.SendLuaUIMsg and widget:RecvLuaMsg for communicating with teammates. See https://recoilengine.org/docs/guides/wupget/#springsendluauimsg-and-widgetrecvluamsg

@msem-me
Copy link
Author

msem-me commented Nov 18, 2025

Thanks for the guidance, I wasn't aware of Spring.SendLuaUIMsg, it's definitely simpler.

I've refactored the PR to remove the gadget entirely. Here is the new implementation:

  • Sender: Updated gui_advplayerslist.lua to broadcast directly to allies using Spring.SendLuaUIMsg(..., 'allies').
  • Listener: Instead of modifying the core snd_notification.lua file, I created a small, dedicated widget snd_notifications_addon_ally_alerts.lua to handle RecvLuaMsg, following the naming convention of the other snd_ files. This then hooks into the existing notification queue system.
  • Cleanup: Deleted alert_notifications.lua and removed references to it.

Please let me know if this is the correct approach. I've made an assumption that queueNotification is preferable to playNotification. Particuarly if this were to become a voice prompt in the future.

Thanks for reviewing.

@sprunk
Copy link
Collaborator

sprunk commented Nov 18, 2025

I've made an assumption that queueNotification is preferable to playNotification

Probably, but this is for a sound designer to decide, I only look at code quality.

The .wav was made on https://sfxr.me/ so should be replaced with something better.

Assets should have their licence specified (somehow). I also haven't listened to the sound (also the job of a sound designer).

@Damgam
Copy link
Member

Damgam commented Nov 18, 2025

Just call WG['notifications'].queueNotification("AllyRequestEnergy") directly in gui_advancedplayerlist

You forgot to add I18N (language) def for the new announcement.

Sound effect is trash.

I like the notification itself though, I was going to add something like this myself.

@msem-me
Copy link
Author

msem-me commented Nov 18, 2025

I'll work on the sound effect, it is trash, more a placeholder for testing. Is there an existing SFX we could re-use that would make sense?

If we call WG['notifications'].queueNotification("AllyRequestEnergy") directly in gui_advancedplayerlist.lua won't it only play for the local user? If we want allies to hear it doesn't it need to be sent via Spring.SendLuaMsg somehow?

I was suggesting this would be a sound effect rather than speech-to-text notification, but keen to get others' opinions. I think most voice announcements are one shot, whereas I see this as more similar to a map ping.

I've added the local check for the sender being and ally and added the stackedDelay, thank you for catching this.

@msem-me
Copy link
Author

msem-me commented Nov 18, 2025

I've added a slightly improved sound effect. Please let me know what you think.

With regards to the I18N (language) def for the new announcement, is this required if there's no voice line associated with it, only a sound effect?

@Damgam
Copy link
Member

Damgam commented Nov 19, 2025

is this required if there's no voice line associated with it, only a sound effect?

Yes because it appears in the messages box. In current state it will show missing localisation string there. Also, We will add a voiceline for it if it gets added.

Please put it as Allied Commander, Requesting Metal. and Allied Commander, Requesting Energy.

@Damgam
Copy link
Member

Damgam commented Nov 19, 2025

I don't hate this new sound effect but please make sure all the licenses and credits are covered.

Also it's too loud. Put it in Audacity or something and compare it to other sound effects, and try to match.

@Damgam
Copy link
Member

Damgam commented Nov 19, 2025

If you don't want this to be a voiced notification then don't use voice notifications system for it, and just play the sound.

@sprunk
Copy link
Collaborator

sprunk commented Nov 19, 2025

I think the idea of putting it in the notification framework was that the sound should still be queued like other notifications so as not to play on top of other sounds, and that one can customize the sound to be voiced.

@msem-me
Copy link
Author

msem-me commented Nov 19, 2025

I hereby release the sound file AllyRequest.wav included in this PR under the CC0 1.0 Universal (CC0 1.0) Public Domain Dedication. I declare that I created this sound from scratch using LMMS synthesizers.

I have added attestation to the effect of the above in license_sounds.txt.

I have lowered the volume of AllyRequest.wav

I have added localisations to tips.json files as requested. I can only verify the english translations, so please verify the Czech, French, Russian, German, Spanish and Chinese translations are correct.

@sprunk
Copy link
Collaborator

sprunk commented Nov 20, 2025

For translations do just the English source, the other languages are handled via a dedicated translation platform.

@msem-me msem-me force-pushed the SupportRequestNotificationSound branch 3 times, most recently from 86524af to 996dbdd Compare November 20, 2025 19:58
@msem-me msem-me force-pushed the SupportRequestNotificationSound branch from 996dbdd to 141843a Compare November 20, 2025 20:01
@msem-me
Copy link
Author

msem-me commented Nov 20, 2025

Removed unnecessary language translations, left only language/en/tips.json. I've cleaned up into a single commit too. Anything else I need to address?

@msem-me
Copy link
Author

msem-me commented Nov 20, 2025

Updated the PR comment to accurate reflect the current state based on feedback.

@sprunk
Copy link
Collaborator

sprunk commented Nov 21, 2025

Something to think about in the future is that "requesting units" also exists, but adding that is not necessarily in the scope of this PR.

@msem-me
Copy link
Author

msem-me commented Nov 28, 2025

Is there anything else I can provide on this?

@sprunk
Copy link
Collaborator

sprunk commented Nov 28, 2025

Nah, just wait for senpai to notice

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants