Page MenuHomeWildfire Games

Add a 8-bit music Easter Egg
ClosedPublic

Authored by PingvinBetyar on Dec 1 2017, 9:59 PM.

Details

Summary

Add a cheat code to play the 8-bit version of Honor Bound.

Test Plan
  1. Open Chat
  2. Type "retro me"
  3. Enjoy the retro feeling

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

PingvinBetyar created this revision.Dec 1 2017, 9:59 PM
elexis added a subscriber: elexis.Dec 1 2017, 10:27 PM
elexis added inline comments.
binaries/data/mods/public/gui/common/music.js
39 ↗(On Diff #4481)

CUSTOM? CUSTOM_PLAYLIST? So that it could be used with other tracks too

76 ↗(On Diff #4481)

I'd prefer an early return, i.e. if (foo) return

binaries/data/mods/public/gui/credits/texts/programming.json
236 ↗(On Diff #4481)

alphabetic order by nickname, prename surname

binaries/data/mods/public/simulation/helpers/Cheat.js
165 ↗(On Diff #4481)

players not needed, is it?
I'd suggest type: playmusic or something like that and a new property for the filename.
The filename would be passed from the cheat ideally.

  • Custom track list
  • Change type to play-custom-tracks
  • Custom track comes from cheat
  • Reorder my entry at contributors
PingvinBetyar added inline comments.Dec 2 2017, 12:47 AM
binaries/data/mods/public/simulation/helpers/Cheat.js
165 ↗(On Diff #4481)

If player is undefined Notifaction won't work because of this if in messages.js handleNotifications function

if (!notification.players || !notification.type || !g_NotificationsTypes[notification.type])
{
	error("Invalid GUI notification: " + uneval(notification));
	continue;
}

And here is the converted track

elexis accepted this revision.Dec 2 2017, 11:17 AM

This diff looks much better than the previous one.

When testing the thing, we can see in the replay file (commands.txt) that this is indeed now able to play arbitrary tracks.
{"type":"cheat","action":"retroMe","player":1,"parameter":"","templates":[{"Type":"custom","File":"0_8Bit_Bonus_Track.ogg"}],"selected":[]}
Perhaps this could also be used to crash the game when sending such a command manually - but that would only be possible in games that allow cheats.

  • It's still weird to have a GUI-only interaction in the simulation, but if we would address that, we would have two different kinds of cheats and I believe we wanted to avoid that.
  • A second downside is that people not familiar with the music codebase might forget that there is a music filename in the cheats and not only in music.js when renaming files for instance. But I'd still prefer having the filename in the cheatfile, so that the cheatfile describes what it does.

If you want to add a little feature, the cheats also consume an argument, for instance iwanttopwnthem 5 spawns 5 heroes. So retrome could also optionally consume an argument of the filename. But I doubt anyone would use this.

So I'll mark it as accepted. I can do that rename and whitespace when committing, but you can upload a new version too, with or without the new feature.
(I'll also have to check that the ogg conversion was nice and sound.)

Thanks for the patch!

binaries/data/mods/public/gui/common/music.js
77 ↗(On Diff #4485)

space after if

binaries/data/mods/public/simulation/helpers/Cheat.js
161 ↗(On Diff #4485)

This should be playCustomTracks too as long as it consumes the tracks as an argument

This revision is now accepted and ready to land.Dec 2 2017, 11:17 AM
  • space after if
  • rename play-custom-tracks to playCustomTracks in the notification
elexis added inline comments.Dec 2 2017, 11:44 AM
binaries/data/mods/public/simulation/helpers/Cheat.js
161 ↗(On Diff #4485)

I meant the line above ;-)

  • rename cheat case to playCustomTracks
  • rename notification to a previous one (play-custom-tracks)
elexis added a comment.Dec 2 2017, 3:44 PM

The phrase play-custom-tracks is slightly misleading, because the tracks itself aren't custom. I'd say play-tracks should be sufficient and the custom part is only needed in music.js.

I suggest that we use the parameter argument, so that the player can type retro me to play and retro me 0 to switch back.
We can do this (possibly together with the cleanup mention in the inline comments) in a separate patch, or not at all, or all in this patch if you want.

About the ogg file:

Weird that it starts with 0_, but I won't make it inconsistent with the audio repository.

ogginfo reveals me:

Vendor: Lavf58.2.100
Channels: 2
Rate: 44100
Nominal bitrate: 112,000000 kb/s

mplayer -identify 0_8Bit_Bonus_Track.wav tells me however

ID_AUDIO_BITRATE=2116800
ID_AUDIO_RATE=44100
ID_AUDIO_NCH=2

112kbps is not the best quality. Arguably it doesn't matter for an 8Bit track, still.
The other tracks in that folder are in 160 kbps, so that should be fine for this 8bit track too then for the time being (the other tracks might be nice to have in 320kbps eventually).
I have set a fixed bitrate using ffmpeg and -b:a.

binaries/data/mods/public/gui/common/music.js
77 ↗(On Diff #4485)

(Might be counterintuitive to people who read this file the first time, especially since it has no documentation on how it really works. But that file needs refactoring altogether (I have a template for that patch, so not relevant now).)

85 ↗(On Diff #4489)

No newlines needed (except the one after the return; is good).
Also we don't have whitespace in empty lines

120 ↗(On Diff #4489)

(That .0 doesn't do anything, but ok to have it consistent with the calls above)

binaries/data/mods/public/gui/credits/texts/programming.json
170 ↗(On Diff #4489)

(I have a slight urge to remove the space from the nick, since currently noone (except {"nick": "gbish (aka Iny)", "name": "Grant Bishop"},) has spaces in the nickname, since your irc and Phabricator name also don't have that space (makes it easier to search). I assume forum nicks also don't have a space.
But then I saw Prodigal Son also has a space, so whatever. ¯\_(ツ)_/¯)

binaries/data/mods/public/simulation/data/cheats/retroMe.json
1 ↗(On Diff #4489)

I noticed the other cheat files are called acording to their action, not according to the thing one has to type.
So this one should be playRetro, play8bit or playTrack or something like that.

5 ↗(On Diff #4489)

Oh dear, that is Templates because executeCheat in messages.js only supports that argument, this should be cleaned eventually. (Not really happy about music.js requiring this Type argument either, since it should always be custom)

New functions:

  • retro me or retro me 1 enable the 8bit music and retro me 2 disable
  • Music class get a lock mechanism to prevent play list change

It would be better if retro me 0 switch off the music but parameter can't be 0 because:

if (cheat.DefaultParameter && (isNaN(parameter) || parameter <= 0))
		parameter = cheat.DefaultParameter;
fabio added a subscriber: fabio.Dec 3 2017, 2:41 PM
In D1097#43524, @elexis wrote:

The phrase play-custom-tracks is slightly misleading, because the tracks itself aren't custom. I'd say play-tracks should be sufficient and the custom part is only needed in music.js.

I suggest that we use the parameter argument, so that the player can type retro me to play and retro me 0 to switch back.
We can do this (possibly together with the cleanup mention in the inline comments) in a separate patch, or not at all, or all in this patch if you want.

About the ogg file:

Weird that it starts with 0_, but I won't make it inconsistent with the audio repository.

ogginfo reveals me:

Vendor: Lavf58.2.100
Channels: 2
Rate: 44100
Nominal bitrate: 112,000000 kb/s

mplayer -identify 0_8Bit_Bonus_Track.wav tells me however

ID_AUDIO_BITRATE=2116800
ID_AUDIO_RATE=44100
ID_AUDIO_NCH=2

112kbps is not the best quality. Arguably it doesn't matter for an 8Bit track, still.
The other tracks in that folder are in 160 kbps, so that should be fine for this 8bit track too then for the time being (the other tracks might be nice to have in 320kbps eventually).
I have set a fixed bitrate using ffmpeg and -b:a.

About ogg quality, see https://trac.wildfiregames.com/ticket/3619#comment:14
64 kb/s, maybe even less, with a proper codec should be more than enough for everything in the game.

fabio added a comment.Dec 3 2017, 2:49 PM

Also the fixed rate is inefficient, and intended for fixed size channels, e.g. ISDN were you can't go higher and you don't benefits from saving during silence, we should use vbr instead, uses higher bitrate where needed, save space on easy zones.
IIRC ffmpeg encoder is also broken with vorbis, I suggest using official oggenc on Linux or oggdrop on windows, with aotuv encoder library.

elexis added a comment.Dec 3 2017, 5:02 PM

I have tried oggenc with the quality parameter only.

For quality 0 I get a nominal bitrate of 64kbps. I believe to hear a difference at 25 seconds.
For quality 4 128kbps, quality 6 192kbps.
Maybe ogg is much smarter than mp3, but when listening to mp3s I'm really dissatisfied with 192kbps. Indeed people should hear the music on youtube or download it if they really want to however.
Since this is an 8bit track and only an easteregg we can justify reducing the quality and have a diffeent bitrate for this file to save that MB.

binaries/data/mods/public/gui/common/music.js
183 ↗(On Diff #4496)

Only one function setLocked needed

binaries/data/mods/public/gui/session/messages.js
467 ↗(On Diff #4496)

setState not needed as its updated every turn on its own, right? So the else part can be avoided afterwards

binaries/data/mods/public/simulation/data/cheats/retroMe.json
5 ↗(On Diff #4496)

nice! Surprised that it worked however.

binaries/data/mods/public/simulation/helpers/Cheat.js
166 ↗(On Diff #4496)

Since this object structure is defined by the GUI, we should construct it in the GUI too, i.e. as late as possible, just before calling the music function

172 ↗(On Diff #4496)

would be nicer to pass a boolean here, i.e parameter == 1, or 2

fabio added a comment.Dec 3 2017, 5:17 PM
In D1097#43717, @elexis wrote:

I have tried oggenc with the quality parameter only.

For quality 0 I get a nominal bitrate of 64kbps. I believe to hear a difference at 25 seconds.
For quality 4 128kbps, quality 6 192kbps.
Maybe ogg is much smarter than mp3, but when listening to mp3s I'm really dissatisfied with 192kbps. Indeed people should hear the music on youtube or download it if they really want to however.
Since this is an 8bit track and only an easteregg we can justify reducing the quality and have a diffeent bitrate for this file to save that MB.

Ogg vorbis is indeed a lot better than MP3, also for MP3 the codec used can also make a great difference. Make sure to use latest aotuv libvorbis when encoding a file, the standard library wasn't perfectly tuned. Using -q N is the proper way to use variable bit rate (vbr).

  • lock and unlock merged to setLocked
  • message.js simplifield
  • tracks object created by message.js
  • notification pass boolean instead int, as suggested
elexis added a comment.Dec 4 2017, 4:24 AM
In D1097#43723, @fabio wrote:

Make sure to use latest aotuv libvorbis when encoding a file, the standard library wasn't perfectly tuned. Using -q N is the proper way to use variable bit rate (vbr).

Since I'm using ubuntu and that doens't provide a package, I have downloaded the latest source from http://www.geocities.jp/aoyoume/aotuv/ compiled, ran make install, but ogginfo doesn't show that the codec was applied.
Anyway, shouldn't matter much for this 1.9mb 8bit file, but I agree that it would be great to reencode the entire music and sound effect folder to reduce the download size drastically eventually (_and_ save the tags while at it).

Nice patch PingvinBetyar, all those nuisances with the surrounding cheat and music code have vanished!

binaries/data/mods/public/gui/common/music.js
174 ↗(On Diff #4511)

Maybe a comment (we use JSdoc syntax) explaining why and what is locked, For instance

/**
   * Play the custom playlist when locked, otherwise plays the civ music according to the battle state.
   */
binaries/data/mods/public/gui/session/messages.js
466 ↗(On Diff #4511)

storeTracks and setState only needed when locking, right?

Might want to use a fancy map array function:
storeTracks(notification.tracks.map(track => ({ "Type": "custom", "File": track }))

Imarok added a subscriber: Imarok.Dec 4 2017, 12:54 PM

I get this when trying:

Function call failed: return value was -100032 (Function not supported)
Location: ogg.cpp:241 (Open)

Call stack:

(0x9c2fbe) binaries/system/pyrogenesis() [0x9c2fbe]
(0x969179) binaries/system/pyrogenesis() [0x969179]
(0x969545) binaries/system/pyrogenesis() [0x969545]
(0x9698c8) binaries/system/pyrogenesis() [0x9698c8]
(0x71c612) binaries/system/pyrogenesis() [0x71c612]
(0x718f9e) binaries/system/pyrogenesis() [0x718f9e]
(0x6bc606) binaries/system/pyrogenesis() [0x6bc606]
(0x6be6bc) binaries/system/pyrogenesis() [0x6be6bc]
(0x6b7e93) binaries/system/pyrogenesis() [0x6b7e93]
(0xa6963c) binaries/system/pyrogenesis() [0xa6963c]
(0x7eff269f5ff0) /media/LiExtend/0ADsvn/binaries/system/libmozjs38-ps-release.so(+0x227ff0) [0x7eff269f5ff0]
(0x7eff269ead0d) /media/LiExtend/0ADsvn/binaries/system/libmozjs38-ps-release.so(+0x21cd0d) [0x7eff269ead0d]
(0x7eff269f5cb1) /media/LiExtend/0ADsvn/binaries/system/libmozjs38-ps-release.so(+0x227cb1) [0x7eff269f5cb1]
(0x7eff269f5f3c) /media/LiExtend/0ADsvn/binaries/system/libmozjs38-ps-release.so(+0x227f3c) [0x7eff269f5f3c]
(0x7eff269f6a5f) /media/LiExtend/0ADsvn/binaries/system/libmozjs38-ps-release.so(+0x228a5f) [0x7eff269f6a5f]
(0x7eff26b84349) /media/LiExtend/0ADsvn/binaries/system/libmozjs38-ps-release.so(+0x3b6349) [0x7eff26b84349]

errno = 0 (No error reported here)
In D1097#43965, @Imarok wrote:

I get this when trying:

Because Phabricator didnt download the audio file but creates a 0 byte one

fabio added a comment.Dec 4 2017, 8:36 PM

Try:
ldd /usr/bin/oggenc
to check if oggenc links to newer libvorbis.

Imarok added a comment.EditedDec 5 2017, 5:16 PM
In D1097#43967, @elexis wrote:
In D1097#43965, @Imarok wrote:

I get this when trying:

Because Phabricator didnt download the audio file but creates a 0 byte one

Ah, sorry for the noise ><

Not sure, but maybe this cheat should also be available, when cheats are disabled...

Maybe with the mapping, the line is a little bit too long. What do you think?

Itms awarded a token.Dec 8 2017, 10:35 PM

@fabio do you happen to have that codec installed and want to upload the ogg? (I'm kind of too lazy to try again just for this file)

Thanks again for the patch again PingvinBetyar and the recommendation fabio!

I'm not going to try another oggenc compilation just for this file. I'll pick the variable bitrate version that's about 128kbps with the ubuntu packaged oggenc. That's just below the 160kbps average and 1.3MB instead of 1.6MB and we can save maybe 300kb more.
(also noticed something I reported at #4918)

fabio added a comment.Dec 30 2017, 6:05 PM

I am away now, I can do the encoding tomorrow, I suppose.

Okay, I will just commit the code for now.

  • bug: doesn't work for players who won: updateGUIObjects() doesn't update the music depending on the battlestate for observers (including players who had won).

Since winners can still send commands, we need to update the playlist each time a cheat is typed.
Neither can't it work for the other observers who aren't players who had won as these can't send cheats.
The winning players bug could be solved by refactoring the music code, which is needed sometime.

  • I've independently changed the messages.js cheat restriction and simplified the JSON structure, so that players can type "retro me" to play the 8bit track, "retro me off" to turn it off and "retro me Filename1.ogg Filename2.ogg" for custom playlists. Ive tested quickly with the other cheats and hope there is no hidden bug (its now possible to cheat negative resources).

No cheat (including (research technology code and spawn unit or whatever) should use anything else than parameter I suppose. Can simplify the json code then. Also JSON files are not good for cheats since they should contain the cheat simulation function too. (Can be implemented whenever by whomever.)

This revision was automatically updated to reflect the committed changes.
Owners added a subscriber: Restricted Owners Package.Dec 30 2017, 6:42 PM
fabio added a comment.Jan 2 2018, 11:50 AM

Here are 3 versions of the wav coded with aotuv at quality 0 (64kb/s) ,-1 (48kb/s) and -2 (32 kb/s).
Feel free to try them, if the 64 kb/s is not enough I can try with a higher setting.

elexis added a comment.Jan 2 2018, 1:04 PM

Thanks, those file sizes are reduced significantly!
It's hard to notice the quality differences. Perhaps the sound effect at 23s and 30s would be suited best to judge it.
In my opinion you can commit the one you prefer.
(Also Omri mentioned that he is remastering the current music tracks, I guess that does exclude this track)

fabio added a comment.Jan 2 2018, 4:33 PM

Committed the 32 kbps version. We may use higher bitrates for the standard musics.

elexis added a comment.Jan 2 2018, 4:35 PM

Nice, thanks for the help! :-)