Page MenuHomeWildfire Games

Notify players with a sound when a player is defeated
Needs ReviewPublic

Authored by Stan on Jul 3 2020, 2:37 PM.

Details

Reviewers
Angen
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Summary

Currently there is only a text notification when a player is defeated this patch a sound will also be played.

Open Questions:

  • Better defeat sound
Test Plan

Play a game with multiple players, and change perspective to make them lose, or use cheats. Check that the sound is heard

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Vulcan added a comment.Jul 3 2020, 2:42 PM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'new'.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Player.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Player.js
| 443| 443| 		// Sound must be have <Omnipresent>1</Omnipresent> and not have <HeardBy>owner</HeardBy>
| 444| 444| 		// TODO: Support <HeardBy>othersonly</HeardBy> so that the losing player doesn't hear it.
| 445| 445| 		if(cmpSoundManager && cmpSound)
| 446|    |-			cmpSoundManager.PlaySoundGroupAtPosition(cmpSound.GetSoundGroup("defeated"),  new Vector3D(0, 0, 0))
|    | 446|+			cmpSoundManager.PlaySoundGroupAtPosition(cmpSound.GetSoundGroup("defeated"), new Vector3D(0, 0, 0))
| 447| 447| 	}
| 448| 448| 
| 449| 449| 	this.state = newState;
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Player.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Player.js
| 443| 443| 		// Sound must be have <Omnipresent>1</Omnipresent> and not have <HeardBy>owner</HeardBy>
| 444| 444| 		// TODO: Support <HeardBy>othersonly</HeardBy> so that the losing player doesn't hear it.
| 445| 445| 		if(cmpSoundManager && cmpSound)
| 446|    |-			cmpSoundManager.PlaySoundGroupAtPosition(cmpSound.GetSoundGroup("defeated"),  new Vector3D(0, 0, 0))
|    | 446|+			cmpSoundManager.PlaySoundGroupAtPosition(cmpSound.GetSoundGroup("defeated"),  new Vector3D(0, 0, 0));
| 447| 447| 	}
| 448| 448| 
| 449| 449| 	this.state = newState;

binaries/data/mods/public/simulation/components/Player.js
| 346| »   for·(var·type·in·amounts)
|    | [NORMAL] JSHintBear:
|    | 'type' is already defined.

binaries/data/mods/public/simulation/components/Player.js
| 446| »   »   »   cmpSoundManager.PlaySoundGroupAtPosition(cmpSound.GetSoundGroup("defeated"),··new·Vector3D(0,·0,·0))
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2577/display/redirect

Stan updated this revision to Diff 12551.Jul 4 2020, 5:04 PM

Play a different sound depending on whether you were defeated, an ally was or an enemy was.

Vulcan added a comment.Jul 4 2020, 5:09 PM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Player.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Player.js
| 449| 449| 				continue;
| 450| 450| 
| 451| 451| 			if (playerEnt === this.entity)
| 452|    |-				cmpSoundManager.PlaySoundGroupAtPosition(cmpSound.GetSoundGroup("defeat"), dummyPosition)
|    | 452|+				cmpSoundManager.PlaySoundGroupAtPosition(cmpSound.GetSoundGroup("defeat"), dummyPosition);
| 453| 453| 			else if (this.IsAlly(i))
| 454| 454| 				cmpSoundManager.PlaySoundGroupAtPosition(cmpSound.GetSoundGroup("ally_defeat"), dummyPosition)
| 455| 455| 			else
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Player.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Player.js
| 451| 451| 			if (playerEnt === this.entity)
| 452| 452| 				cmpSoundManager.PlaySoundGroupAtPosition(cmpSound.GetSoundGroup("defeat"), dummyPosition)
| 453| 453| 			else if (this.IsAlly(i))
| 454|    |-				cmpSoundManager.PlaySoundGroupAtPosition(cmpSound.GetSoundGroup("ally_defeat"), dummyPosition)
|    | 454|+				cmpSoundManager.PlaySoundGroupAtPosition(cmpSound.GetSoundGroup("ally_defeat"), dummyPosition);
| 455| 455| 			else
| 456| 456| 				cmpSoundManager.PlaySoundGroupAtPosition(cmpSound.GetSoundGroup("enemy_defeat"), dummyPosition)
| 457| 457| 		}
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Player.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Player.js
| 453| 453| 			else if (this.IsAlly(i))
| 454| 454| 				cmpSoundManager.PlaySoundGroupAtPosition(cmpSound.GetSoundGroup("ally_defeat"), dummyPosition)
| 455| 455| 			else
| 456|    |-				cmpSoundManager.PlaySoundGroupAtPosition(cmpSound.GetSoundGroup("enemy_defeat"), dummyPosition)
|    | 456|+				cmpSoundManager.PlaySoundGroupAtPosition(cmpSound.GetSoundGroup("enemy_defeat"), dummyPosition);
| 457| 457| 		}
| 458| 458| 	}
| 459| 459| 

binaries/data/mods/public/simulation/components/Player.js
| 346| »   for·(var·type·in·amounts)
|    | [NORMAL] JSHintBear:
|    | 'type' is already defined.

binaries/data/mods/public/simulation/components/Player.js
| 452| »   »   »   »   cmpSoundManager.PlaySoundGroupAtPosition(cmpSound.GetSoundGroup("defeat"),·dummyPosition)
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

binaries/data/mods/public/simulation/components/Player.js
| 454| »   »   »   »   cmpSoundManager.PlaySoundGroupAtPosition(cmpSound.GetSoundGroup("ally_defeat"),·dummyPosition)
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

binaries/data/mods/public/simulation/components/Player.js
| 456| »   »   »   »   cmpSoundManager.PlaySoundGroupAtPosition(cmpSound.GetSoundGroup("enemy_defeat"),·dummyPosition)
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2581/display/redirect

Angen added a subscriber: Angen.Jul 4 2020, 5:12 PM
Angen added inline comments.
binaries/data/mods/public/simulation/components/Player.js
432

Why not to listen to player defeated message and every player will handle sound by itself, instead of looping here ?

Angen added inline comments.Jul 4 2020, 5:13 PM
binaries/data/mods/public/simulation/components/Player.js
446

also here is hard duplication;
set sound group to variable and use one line to call play sound

Angen requested changes to this revision.Jul 10 2020, 3:47 PM
This revision now requires changes to proceed.Jul 10 2020, 3:47 PM
Stan updated this revision to Diff 12645.Jul 12 2020, 4:47 PM

Use message instead of loop, add soundmanager function to play sound for a specific player

Stan marked 2 inline comments as done.Jul 12 2020, 4:48 PM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/simulation2/components/ICmpSoundManager.cpp
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2017"

source/simulation2/components/ICmpSoundManager.h
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2017"

source/simulation2/components/ICmpSoundManager.h
|  28| class·ICmpSoundManager·:·public·IComponent
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classICmpSoundManager:' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '='.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Player.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Player.js
| 937| 937| 	this.startingTechnologies = techs;
| 938| 938| };
| 939| 939| 
| 940|    |-Player.prototype.OnGlobalPlayerDefeated  = function(msg)
|    | 940|+Player.prototype.OnGlobalPlayerDefeated = function(msg)
| 941| 941| {
| 942| 942| 	let cmpSound = Engine.QueryInterface(this.entity, IID_Sound);
| 943| 943| 	if (!cmpSound)
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Player.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Player.js
| 945| 945| 
| 946| 946| 	let cmpSoundManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_SoundManager);
| 947| 947| 	let soundGroup = cmpSound.GetSoundGroup(this.playerID === msg.playerId ? "defeat" : this.IsAlly(msg.playerId) ? "ally_defeat" : "enemy_defeat");
| 948|    |-	cmpSoundManager.PlaySoundGroupForPlayer(soundGroup, this.entity)
|    | 948|+	cmpSoundManager.PlaySoundGroupForPlayer(soundGroup, this.entity);
| 949| 949| };
| 950| 950| 
| 951| 951| 

binaries/data/mods/public/simulation/components/Player.js
| 346| »   for·(var·type·in·amounts)
|    | [NORMAL] JSHintBear:
|    | 'type' is already defined.

binaries/data/mods/public/simulation/components/Player.js
| 948| »   cmpSoundManager.PlaySoundGroupForPlayer(soundGroup,·this.entity)
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2624/display/redirect

Angen added a comment.Jul 13 2020, 6:10 PM

I did not try yet but looks reasonable.
One question I have. How this works for observers?

binaries/data/mods/public/simulation/templates/special/player/player.xml
93

defeated sounds better, what do you think? like trained referring to status change

Stan added inline comments.Jul 13 2020, 6:12 PM
binaries/data/mods/public/simulation/templates/special/player/player.xml
93

Yeah, I went back and forth a few times, I have no strong opinion

Stan updated this revision to Diff 12667.Jul 14 2020, 10:46 AM

Style fixes, defeat to defeated

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1005/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2637/display/redirect

Angen requested changes to this revision.Jul 16 2020, 2:09 PM

jenkins is sad
svn: E195011: 'svn:eol-style' is not a valid Subversion property name

This revision now requires changes to proceed.Jul 16 2020, 2:09 PM
Stan added a comment.Jul 17 2020, 2:45 PM

That's because jenkins is stupid... I think it's an arcanist bug.

Stan updated this revision to Diff 12752.Jul 17 2020, 2:51 PM
Stan edited the summary of this revision. (Show Details)

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2683/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/2150/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1051/display/redirect

Stan updated this revision to Diff 12753.Jul 17 2020, 2:59 PM

I can't do better than this, the patch applies fine on my end...

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2684/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1052/display/redirect

Angen added a comment.EditedJul 18 2020, 5:36 PM

Nice feature and it works but.
1.) I think we need another sounds for these situations and to they should be a bit more different among themselves, mainly when enemy is defeated and ally/me is defeated, also ally and me is defeated could be better distinguishable.
2.) When player wins, it should play winning sound and not enemy defeated sound as that will be played likely during the match as well so I would rather want to hear something else, maybe something more fancy.
3.) Also I would add option for player to disable these sounds, for someone they could be annoying or distracting, mainly enenmy/ally defeated.

Stan updated this revision to Diff 12781.Jul 19 2020, 1:03 PM

Try With config borkne
...

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/2171/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1072/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2704/display/redirect

Stan updated this revision to Diff 12788.Jul 20 2020, 1:18 AM

Revert jsinterface changes

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2709/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1077/display/redirect

Stan updated this revision to Diff 12789.Jul 20 2020, 1:21 AM

Try crlf

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2710/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1078/display/redirect

Stan added a subscriber: Imarok.Jul 20 2020, 1:26 AM

Tried with @Imarok suggestion using the GUIInterface to access the config but it didn't work. The problem is you actually have to call the actual gui to get the config value, and that also allows you to call the playuisound. There are however a lot of issues.

  • That function doesn't accept soundgroups only sound files (.ogg)
    • Bad for modding
    • Hardcoding in js files
  • That function is played regardless of the caller, so while all the players play a different sound, they all hear every sound played by the other players

So I do not think it's possible using that route. That also means one will not be able to disable the sounds without disabling all the other ui sounds. Here is the code if someone has a better idea

Stan updated this revision to Diff 12863.Jul 22 2020, 10:43 PM

Fix broken behavior reported by Angen

Stan updated this revision to Diff 12865.Jul 22 2020, 10:50 PM

Revert changes

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2758/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2760/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1128/display/redirect

Freagarach added inline comments.
binaries/data/mods/public/simulation/components/Player.js
930

Since the ternary below is already huge, you can inline this as well, since it is used merely once ;)

935

Unnecessary newline.

bb added a subscriber: bb.Sep 29 2020, 9:27 PM
bb added inline comments.
source/simulation2/components/CCmpSoundManager.cpp
98

If am I parsing the code right, by setting that last argument to true, the sound will be heard by everyone, i.e., ALL sounds will be heard by everyone. Since everyone runs the simulation, everyone runs ALL the player.js code, for ALL players, hence this gets called for all players, and I don't see where the call gets nuked.

HowToFix: as you can see in the PlaySoundGroup function above, we can set the last variable in the call depending on the currently viewed player. It might be fruitful to not pass the playerEnt but the playerID int (and pass INVALID_ENTITY furtheron).

Stan added inline comments.Sep 29 2020, 9:44 PM
source/simulation2/components/CCmpSoundManager.cpp
98

There is a specific player specified there so since the sound are owner only you only hear one sound although all are played.

Stan updated this revision to Diff 13580.Sep 29 2020, 11:07 PM

Update, for some reason I got confused. Now works with @bb's suggestion

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/3265/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1613/display/redirect

Freagarach added inline comments.Sep 30 2020, 8:35 AM
source/simulation2/components/ICmpSoundManager.cpp
27

Why is this _const?

Stan added inline comments.Sep 30 2020, 8:37 AM
source/simulation2/components/ICmpSoundManager.cpp
27

Cause it doesn't alter the CCmpSoundComponent

bb added inline comments.Sat, Oct 10, 5:18 PM
source/simulation2/components/CCmpSoundManager.cpp
68

why do we use something different here than below?

94

Why do we pass the playerEntity? querying cmpVisual down the line should return null anyway. Pass INVALID_ENTITY down.

96

This disables the "heardby" tag of the sound, probably better to pass g_Game->GetViewedPlayerID() == playerId as last argument below.

Stan updated this revision to Diff 13620.Sun, Oct 11, 3:55 PM
Stan marked 8 inline comments as done.

Fix notes.

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/3291/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1639/display/redirect