#2898 closed enhancement (fixed)
[PATCH] audio notification system when your nick is mentioned in chat
Reported by: | Moul | Owned by: | Andy Alt |
---|---|---|---|
Priority: | Nice to Have | Milestone: | Alpha 21 |
Component: | Music & Sound FX | Keywords: | patch |
Cc: | Patch: |
Description (last modified by )
Get a notification system when someone talk to you (write exactely the psedo) in the multiplayer lobby. Like for XMPP clients. Because when the 0ad windows is not fullscreen you need to watch each time if someone talk to you.
Attachments (7)
Change History (35)
comment:1 by , 9 years ago
Keywords: | chat removed |
---|---|
Milestone: | Alpha 18 → Backlog |
comment:2 by , 8 years ago
Keywords: | chat sounds audio alerts lobby host added |
---|
comment:3 by , 8 years ago
Description: | modified (diff) |
---|---|
Type: | task → enhancement |
comment:4 by , 8 years ago
Description: | modified (diff) |
---|
comment:6 by , 8 years ago
Keywords: | chat sounds audio alerts lobby host removed |
---|---|
Milestone: | Backlog → Alpha 21 |
Owner: | set to |
comment:7 by , 8 years ago
Status: | new → assigned |
---|
follow-up: 11 comment:8 by , 8 years ago
Milestone: | Alpha 21 → Backlog |
---|
Cool work :)
I listened to the sound and I don't think it fits though: the piano sounds way too modern for the game and it contrasts with other sounds that are more "nature-feeling".
Would you have another idea? I really like the rhythm you chose, you should keep this.
Important: you should not use Trac for submitting art. Please create a topic in the Official Tasks forum and don't forget to post to the Legal Waiver forum thread.
This ticket can be used for proposing changes to the code in order to use the new sound.
comment:9 by , 8 years ago
Keywords: | patch review added |
---|---|
Milestone: | Backlog → Alpha 21 |
Summary: | Get notification system on lobby chat when someone talk to you → [PATCH] audio notification system when your nick is mentioned in chat |
comment:10 by , 8 years ago
The patch not only issues an audible alert if your nick is mentioned in the lobby, but also the game setup (host) menu, and during a game (session).
comment:11 by , 8 years ago
Replying to Itms:
Cool work :)
I listened to the sound and I don't think it fits though: the piano sounds way too modern for the game and it contrasts with other sounds that are more "nature-feeling".
Would you have another idea? I really like the rhythm you chose, you should keep this.
Important: you should not use Trac for submitting art. Please create a topic in the Official Tasks forum and don't forget to post to the Legal Waiver forum thread.
This ticket can be used for proposing changes to the code in order to use the new sound.
Presently the sound used in the patch is a pre-existing 0 A.D. clip, used as a placeholder. The sound of selecting a metal deposit.
As per your suggestion about the art part, I hear you. I'll use the forum for that. Thanks.
I posted in an existing forum thread to ask for suggestions regarding the sound clip. Sounds to create
follow-up: 13 comment:12 by , 8 years ago
Keywords: | review removed |
---|
As mentioned on IRC, the patch has style issues (whitespace, bad capitalization - we use camelCase, we use let
over var
etc.). Please read wiki:Coding_Conventions thoroughly.
Regarding the code, I really don't think getting the username from the configuration files is the proper way to do it. Also you should use != -1
instead of >= 0
for consistency.
Per our new SubmittingPatches system you should use the rfc queue since this is not a ready-to-commit proposal.
The sound used is far too aggressive for a simple notification IMO, so I'm afraid this placeholder can't be used. Also I'm afraid the forum thread you posted to is not useful since LAVS has been AWOL for years.
comment:13 by , 8 years ago
Milestone: | Alpha 21 → Backlog |
---|---|
Owner: | changed from | to
Status: | assigned → new |
Replying to Itms:
As mentioned on IRC, the patch has style issues (whitespace, bad capitalization - we use camelCase, we use
let
overvar
etc.). Please read wiki:Coding_Conventions thoroughly.Regarding the code, I really don't think getting the username from the configuration files is the proper way to do it. Also you should use
!= -1
instead of>= 0
for consistency.Per our new SubmittingPatches system you should use the rfc queue since this is not a ready-to-commit proposal.
The sound used is far too aggressive for a simple notification IMO, so I'm afraid this placeholder can't be used. Also I'm afraid the forum thread you posted to is not useful since LAVS has been AWOL for years.
A 4-page thread with multiple subscribers is useless because the originating poster has been AWOL for years? Will the thread be locked or deleted?
I'm afraid I don't have time or desire to learn, understand, and comply with all the required standards (especially when my work so far has been pro-bono), so I'll unassign myself from this ticket and won't bother you with anymore patches. I hope at some point you'll be able to find some of this code useful and it will improve the gaming experience for the players.
comment:14 by , 8 years ago
Milestone: | Backlog → Alpha 21 |
---|---|
Owner: | changed from | to
Status: | new → assigned |
After some words by elexis, I decided I'd like to finish this patch.
I read about the differences between let
and var
and will absorb that info as I practice it more.
I reviewed the coding conventions page. I typically don't remember things I read, but I'll be sure to reference it more often.
I know the sound isn't correct, and don't know what to do about it other than what I've done. Shall I start a separate thread on the forum to make a request for users to submit sound clips? It was also discussed on IRC earlier today so maybe niektb will be able to provide something.
I don't know the proper way to get the username in gamesetup.js and messages.js. I started learning javascript 3 or 4 days ago, and after 2 hours was unable to figure out a different way, that's why I finally chose to use the method I did. I am open to suggestions or cheats on how to implement a different way.
I believe I've made all the changes regarding caps and whitespace. I'm submitting the patch for comments again, to ensure I've set geany to deal with my whitespace problem.
comment:15 by , 8 years ago
Keywords: | rfc added |
---|
comment:16 by , 8 years ago
(Had mentioned some of these things in irc on 2016-07-13)
In case you want to find out what a variable contains, add a warn(uneval(variableName));
and execute that code.
functions_utility.js
:
- L311 else not needed (in case the if-statement is true, it won't execute the code below anyway. if it's false, it won't return)
- L312 use
indexOf
instead ofsearch
since we use that everywhere - Variable
nickNotify
is used only once, so it can be inlined (as inif (!Engine...) return
) - There should be a comment stating what the function is good for (i.e. playing a sound if someone types your nick.). It should be a JSdoc comment, i.e. start with
/**
gamesetup.js
and messages.js
:
- Get the username from
g_PlayerAssignments[Engine.GetPlayerGUID() || "local"]
(.Name or .name, don't recall). (Since the uploaded patch only checks for multiplayername while it could be singleplayer and for lobby games we have the rating in the name currently) - Not fully convinced that we need a global for that name since it's used only in one place and we should avoid copies.
- Perhaps the check in L703 can be moved to that new notification function as an early return (if !text return;)
lobby.js
:
- Don't add whitespace to that emptyline
options.json
:
- Since the option is used for gamesetup and ingame (session), it's not a lobby option anymore but a general one.
- You need to add an entry to
default.cfg
As usual, I'm happy to answer any kind of stupid questions in irc or here and don't worry about learning the coding conventions - getting a patch rejected due to some minor style issues and having to update it again and again is itself painful enough so that they will burn in at some point anyway ;)
by , 8 years ago
Attachment: | userNotify.2.patch added |
---|
comment:17 by , 8 years ago
Keywords: | rfc removed |
---|
follow-up: 19 comment:18 by , 8 years ago
I seem to get pinged now even if my name is not mentioned. Will submit another patch soon.
comment:19 by , 8 years ago
Replying to andy5995:
I seem to get pinged now even if my name is not mentioned. Will submit another patch soon.
It was happening because my local name got changed when I was testing with 2 different instances using the same config directory.
comment:20 by , 8 years ago
Keywords: | rfc added |
---|
comment:21 by , 8 years ago
Keywords: | rfc removed |
---|
comment:24 by , 8 years ago
You should use let
instead of var
.
I think you should remove L317-318 in functions_utility.js and change L320 to if (!notifyUser.lastPing || timeNow > notifyUser.lastPing + 3000)
.
Great feature btw ;)
comment:25 by , 8 years ago
After I switched to using a global variable rather than a function object, it doesn't work correctly during session chat anymore. The user only gets pinged once. (userNotify.3.patch)
by , 8 years ago
Attachment: | userNotify.4.patch added |
---|
by , 8 years ago
Attachment: | userNotify.5.patch added |
---|
use of function object as static keyword in this patch
by , 8 years ago
Attachment: | userNotify.4.2.patch added |
---|
Moved the option to the sound options, as it's not only a lobby option anymore. Chose the patch with that global since it doesn't bug here. Added the target filename of the new sound. Tested and approved.
comment:26 by , 8 years ago
Keywords: | review added; rfc removed |
---|
That sound https://wildfiregames.com/forum/applications/core/interface/file/attachment.php?id=11824 seems well suited. It's a harp, so it's an instrument that fits and it consists of only 2 notes, which is exactly the right amount (not too annoying when repeated often).
comment:27 by , 8 years ago
Component: | Multiplayer lobby → Music & Sound FX |
---|---|
Keywords: | review removed |
Resolution: | → fixed |
Status: | assigned → closed |
Fixes by r18545.
Thanks andy5995 for spending a lot of energy to understand the fallacies of our code and javascript, creating a bunch of sound files that were rejected and the one accepted and taking all the hurdles!
Also shout-outs to niektb, Imarok, Itms, sanderd17, feneur, fabio, Lion.Kanzen (and likely others I forgot to mention) that gave feedback on the patch and the soundfile in irc and the thread: https://wildfiregames.com/forum/index.php?/topic/20967-nick-notification-sound/
comment:28 by , 8 years ago
elexis, thank you for the recognition and acknowledgements. I'd like to add a few names to that list: sanderd17, echotangoecho, and elexis.
I wanted to submit this for a feature request.
An audible alert when someone types your name (or enters your name using tab-autocomplete).
It would be great if this were implemented in the host menu and the chat during gameplay.
It helps draw attention to a chat message that you may otherwise miss. A short sound clip that's unique from the other sounds within the game.
There should be a simple, quick way to disable it with a checkbox - perhaps near the chat console itself.