Music system improvements

Naprogramuji commented 4 years ago

Download more royalty free music
Shuffle music
Support for music from mods placed in "music" subfolder (should be great for traditional soundtrack from nation mod for example)

rh-github-2015 commented 4 years ago

... or a nation 'theme' that only gets played on the player picker where one selects who to play. Yes, had similar ideas again.

Unfortunately, sound is very much hardcoded (Sound is an enum) and limited right now and I wouldn't want to presume how to expand the current system. When I noticed unit attacksound is dead I did get them to play (including from a mod, see attack-sounds-experimental ), but that's an ugly hack, will probably break on android due to threading issues, and, well, the decision on an appoach is political and in my view up to yair?

Naprogramuji commented 4 years ago

Nice idea with nation theme. I got this idea from Civ5 nation mod, where there was classic songs from nation's best artists from history.

rh-github-2015 commented 4 years ago

I tried something yesterday, and for me it's 'sexy', but I'm not sure about implementation details. If someone's curious, go ahead and take a look.

I went to the homepages of the two main composers for civ5, one of them has a bunch of tracks freely accessible. Added some stuff from the 'Thatched Thingies' composer. Moved Inca-specific files into mods/Inca/music. Renamed nation-specific files to prefix . Prefixed generic tracks with 'Ambient'. Made sure there's a suffix _Theme if track is good for player picker, _War if good for the declare war popup, _Peace if applicable for first contact or trade while at peace...

Music selection in Trade screen not done, and needs some way to leave currently playing track alone if there's no match at all (so we'd be free do allow any 'war' mood track to start in a war declaration if a nation-specific one is missing, but leave music alone on first contact if there's no nation-specific file of any mood)...

Also not yet decided about fade-out duration, whether to put that in settings, or whether to begin fading in a new track while the old is still fading out.

Oh, and it searches all mods, not just the currently selected, or I would have needed a way to pass the new ruleset from the new game screen to the track randomizer. Civname_ prefix was enough for first draft.

rh-github-2015 commented 4 years ago

Technical: I'd love to hear from the gurus.

That ticker firing 20 times per second is ugly, but I saw no other way to get a fade-out.
thread("Music") is gone - Music.play() thread-decouples anyway as I understand the dox, so startSound() always returned quickly - or not? Breakpoints in the debugger seem to corroborate. Now timer("musicTimer") is the new decoupled thread, and all Music.play() calls are there anyway, so everything's fine, right?
On the other hand, with unchanged calling site code, I managed to have the same track play twice in parallel with a shift of half a second - never with the new code (continuing from a breakpoint in the separate thread did that).
Lastly, I' not convinced this is thread-safe in all situations, especially some of the !! operators. Maybe ...

            if (<should stop>) {
                val temp = currentMusic!!
                currentMusic = null
                temp.stop()
                temp.dispose()
            }

would be safer than

            if (<should stop>) {
                currentMusic!!.stop()
                currentMusic!!.dispose()
                currentMusic = null
            }

...??? Are there atomic read-and-update patterns in kotlin?

Naprogramuji commented 4 years ago

I tried something yesterday, and for me it's 'sexy', but I'm not sure about implementation details. If someone's curious, go ahead and take a look.

I'll test it in free time. You are amazing :)

Naprogramuji commented 4 years ago

I went to the homepages of the two main composers for civ5, one of them has a bunch of tracks freely accessible. Added some stuff from the 'Thatched Thingies' composer. Moved Inca-specific files into mods/Inca/music. Renamed nation-specific files to prefix . Prefixed generic tracks with 'Ambient'. Made sure there's a suffix _Theme if track is good for player picker, _War if good for the declare war popup, _Peace if applicable for first contact or trade while at peace...

You mean Inca mod on GitHub? I can' find it for testing.

Naprogramuji commented 4 years ago

Also not yet decided about fade-out duration, whether to put that in settings, or whether to begin fading in a new track while the old is still fading out.

No need for settings I thing. Gentle length should be enough. 3-4 seconds before end of previous maybe?

Naprogramuji commented 4 years ago

Oh, and it searches all mods, not just the currently selected, or I would have needed a way to pass the new ruleset from the new game screen to the track randomizer. Civname_ prefix was enough for first draft.

It's bad. Maybe in future. But it's small price for music system right now.

Naprogramuji commented 4 years ago

That ticker firing 20 times per second is ugly, but I saw no other way to get a fade-out.

20 times per second isn't much for call. In Unity for example, you check it per frame. So 60 times + per second.

rh-github-2015 commented 4 years ago

Thanks for the comments.

You mean Inca mod on GitHub? I can' find it for testing. Maybe Yair already removed it because he merged them into main? The post meant only to quickly illustrate where it looks and how it chooses file names - without writing clear instructions. IOW, structure like this:

Allows some flexibility, as e.g. Greece_War.mp3 has the same frequency to be picked as Greece_is_hot_in_summer!_even_without_War.flac, and where in the (modcount+1) music folders it is is irrelevant. Best approach? Don't know. Just a quick hack. And I got distracted or the hooks in the trade dialogs would be in by now.

Naprogramuji commented 4 years ago

This happened to me when first civ meet: `Music queued: mods/Lands of the Bohemian Crown/music/Ambient_The High Castle_Peace.mp3 AL lib: (WW) FreeDevice: (0x1316d4000) Deleting 6 Buffer(s) Exception in thread "LWJGL Application" com.badlogic.gdx.utils.GdxRuntimeException: Error reading audio data. at com.badlogic.gdx.backends.lwjgl.audio.Mp3$Music.read(Mp3.java:91) at com.badlogic.gdx.backends.lwjgl.audio.OpenALMusic.fill(OpenALMusic.java:251) at com.badlogic.gdx.backends.lwjgl.audio.OpenALMusic.update(OpenALMusic.java:235) at com.badlogic.gdx.backends.lwjgl.audio.OpenALAudio.update(OpenALAudio.java:227) at com.badlogic.gdx.backends.lwjgl.LwjglApplication.mainLoop(LwjglApplication.java:222) at com.badlogic.gdx.backends.lwjgl.LwjglApplication$1.run(LwjglApplication.java:128) Caused by: java.lang.NullPointerException at com.badlogic.gdx.backends.lwjgl.audio.Mp3$Music.read(Mp3.java:82) ... 5 more Exception in thread "musicTimer" java.lang.UnsatisfiedLinkError: org.lwjgl.openal.AL10.nalBufferData(IIJII)V at org.lwjgl.openal.AL10.nalBufferData(Native Method) at org.lwjgl.openal.AL10.alBufferData(AL10.java:1099) at com.badlogic.gdx.backends.lwjgl.audio.OpenALMusic.fill(OpenALMusic.java:268) at com.badlogic.gdx.backends.lwjgl.audio.OpenALMusic.play(OpenALMusic.java:91) at com.unciv.UncivGame.musicTimerTask(UncivGame.kt:148) at com.unciv.UncivGame$startMusic$$inlined$timer$1.run(Timer.kt:149) at java.util.TimerThread.mainLoop(Timer.java:555) at java.util.TimerThread.run(Timer.java:505)

`

Ambient_The High Castle_Peace.mp3.zip

Naprogramuji commented 4 years ago

When ambient music is playing with _Peace and first civ contacted, it should continue playing because _Peace music is playing already.

Naprogramuji commented 4 years ago

Theme should start when nation mod selected and nation was picked to empty player slot. (Depending on civ added).
Music should stops when opening menu. Or new game menu at least.
Lands of the Bohemian Crown_Ktož sú boží boží bojovníci_Theme.mp3 is playing even when picking another civ without theme music. Should stop a music.

Naprogramuji commented 4 years ago

List of my songs here for more info.

rh-github-2015 commented 4 years ago

1. GdxRuntimeException: Error reading audio data.
Normally this would clearly say file is broken. Maybe I should catch that, rename the file so it doesn't get attempted again, log to console and move on...

But - is it broken? Can VLC play it? Can your non-VLC distro play it with its default media player (or MPC should you be on windoze)? As a Mint-ie I remember the Distro gives you a choice to include non-open codecs, and I always pull them... DL'ing for a quick mediainfo... mp3 looks OK, I see no reason why GDX wouldn't handle it. That said, we should probably go for lower resource usage than 192kbps 44kHz - look at the Thatched Villages the game pulls now: 22kHz <100kbps. Probably on purpose. Reminds me to actually test with flac's and AAC v2 HE...

2 - true, was too lazy. Should go into the 'don't repeat too early' logic which at the moment is ~~crap~~ too dumb. Btw, the lazyness hit when I would have needed to g00gl the kotlin way to do a limited depth fifo... 3 - yes I stuck the trigger into the playerpicker, which the auto-select doesn't need to open. Good catch. 4 - You think? Hmmm. For now unless the track pick function is called it just continues, through newgame map editor whatnot. (Hey should I put a nation pick call in the editor when one selects a starting location? - Oh, the other branch needs to check whether the map editor handles modded nations - it probably does) Somewhere in the comments of the branch I did already say it needs the ability to leave currently playing track when match isn't good enough. That should address this. 5 - Well it's a first-hit pick from a list sort. Sorting is by prefix match yes->0/no->1 then suffix match same way, then (is-it-the-same) then random. If I remember correctly. Default next-track should ask for prefix 'Ambient' so with enough of these (>=2 because the don't-repeat 'queue' is only 1 entry deep) others should not be picked by default calls. So - must be a reason? Maybe I should log the filters, not only the result, to the console to enable some rudimentary diagnostic for such a case.

Naprogramuji commented 4 years ago

VLC can play it. I'll test it further but it seems that this happens at random.

rh-github-2015 commented 4 years ago

Well my Unciv can play your file. Mint 19.3. And you might want to check my branch for a more current state. Next thing I need to check is how best to tell it to finish playing one track, but afterwards not move to the next but stop. The small commit for TileEditorOptionsTable might be problematic - I inserted one line with copy& paste and three '.first()' calls way further down suddenly flagged red. A 'build' fails, but starting the debugger not - and it executes my line. Very strange.

Oh, I finally tested with more file types. GdX outright rejects m4a, flac and opus. Darn. And it does so early, and the message more or less says it only checks the extension. Doesn't even try to pass it to the next api which would probably be able to play those... Well, it tries be be multiplatform.

Naprogramuji commented 4 years ago

Works
Works
Works
Works. I like fading out feature :) When you must go something for example, you pause game using menu and no music is playing. Which is great. But music can be paused instead of stopping it completely and playing next.
I don't understand much. This song version is with lyrics so for theme only. I have another version without lyrics for ambient when war.

This is how i understand it for "Lands of the Bohemian Crown_Ktož sú boží boží bojovníci_Theme" - not ambient (no prefix), play when nation is "Lands of the Bohemian Crown", song name, theme (triggers when choosing nation)

This is how i understand it for "Lands of the Bohemian Crown_Ktož sú boží boží bojovníci_War" - not ambient (no prefix), play when nation is "Lands of the Bohemian Crown", song name, war (triggers when war starts)

And when there is no theme music for selected civ, It should fade out and stop at least in my opinion. Right now, you select another civ and previous still playing. And when disabling of nation mod, music continue playing too.

When launch a game and starts with another civ (autostart), mod civ related music is played. I though, that when mod name is before song name, it will play only when playing as civ from mod.

So

"Ambient_Mod-name_Song-name.mp3" should play should play for Mod-name civ only

and

"Ambient_Song-name.mp3" for every civ (when this mod is activated)

When starting game without activated mod or game is loaded without activated mod, nothing plays and music controller stays shut down right now.

https://mab.to/RHy4lJHnr

rh-github-2015 commented 4 years ago

mab.to: "Prior using the service you are required to provide your consent to the processing of your personal data." - no thanks Headaches - trouble deciphering your english - but I think I get the gist: Mostly the answer is: Yes, this is how it's currently designed, and I tried to explain it: Put all music files including all present mods into a list, sort by prefix math, suffix match, nonpresence in recent history, then random and pick first. No fuzzy logic (which I considered).

No, I haven't included any provision to truly pause a playing track yet. Just a quick draft. You convinced me it needs to ignore mod folders not currently in the ruleset. Later.

Don't get me wrong: I asked for feedback, and you delivered. I'm very thankful. But I'm not getting a clear idea where this should go - is the name matching a useful approach or should we drop it and go for something needing explicit declaration in some new json to map context to tracks? Perhaps when my head is clearer...

Naprogramuji commented 4 years ago

mab.to: "Prior using the service you are required to provide your consent to the processing of your personal data." - no thanks

Or you know better service for sending of files? This is just nonsese from EU. You can't run even personal website without this consent.

Naprogramuji commented 4 years ago

Headaches - trouble deciphering your english - but I think I get the gist: Mostly the answer is: Yes, this is how it's currently designed, and I tried to explain it: Put all music files including all present mods into a list, sort by prefix math, suffix match, nonpresence in recent history, then random and pick first. No fuzzy logic (which I considered).

Sorry for my english. I'm not native speaker I can't deciper my english sometimes too. :D

Naprogramuji commented 4 years ago

Don't get me wrong: I asked for feedback, and you delivered. I'm very thankful. But I'm not getting a clear idea where this should go - is the name matching a useful approach or should we drop it and go for something needing explicit declaration in some new json to map context to tracks? Perhaps when my head is clearer...

Your idea with suffixes and affixes is just brilliant. Simple and working solution. Don't get me wrong too… I know it looks like I'm commanding you, but I'm not. I just testing it to pulp. These tasks was for me primary. After when I get to them. But you are hero for me. You are not afraid of any programming challenge. I'm completely new in Kotlin. It's hellish language from my point of view :D So I believe, after few weeks, I'll be more beneficial then actually I am :)

rh-github-2015 commented 4 years ago

Heh. Three or four weeks ago I had'n even heard the name.

The mod system opens holes once you look at getting them into the mod editor, and that's what interests me more right now. I think I'm getting somewhere, but I still get a situation where mod lists are out of synch.

What I'd really like to know - When you have several independent branches in your local git and its github fork, and the fork gets far behind its master, the branches would be presentable as pull requests without red flags if the master hadn't moved but would get red flags now, how do you get everything synchronized properly? The only way I know that would work reliably for me is to make a local file copy of your branches, delete all branches offline and online, then bring the fork up to date, then reopen branches and merge stuff in with an external merge tool.... I did that too often already.

rh-github-2015 commented 4 years ago

Say, in the current codebase (not my fork), what happens if you

open the new game screen, ensure some mods are selected

ensure some players pick a modded nation and leave the new game screen

open the map editor to a new map and close it

open the new game screen again

for me, the mod and player nation selection is preserved, but the icons in the player picker are black.

Or, start a game with mods, open the map editor, close it, and open civilopedia -> crash, right?

The ImageGetter ruleset isn't reset properly, but how do I catch all ways of leaving the map editor to update it?

I think I fixed all that...

rh-github-2015 commented 4 years ago

service for sending of files

We shouldn't endorse such here, but the most hassle-free I know is we.tl. Or does that play nice with me only because I have already let it set some cookies ages ago? (...) Oh, it does have one such button. So, no, I know of no bugme-free file services.

rh-github-2015 commented 4 years ago

Another push, this time I wrote instructions.

I'll probably delete the whole thing soon, to re-create it based on a freshly synced fork master. If that goes smoothly, I'll pull-request it.

Ideas for a json-controlled system to support downloading music files are there, some notes, but will have to wait.

Naprogramuji commented 4 years ago

I like it. I'll prepare my mods for it after you release first draft :) You are God :)

rh-github-2015 commented 4 years ago

God

Definitely not. I don't speak to her and he doesn't speak to me. It's a SEP.

Anyway, re-merge done. Only change - newgame mod sel checkboxes try to select by mod name first, and if that's got no match and there is a nation in there, then it retries with the nation name. With your mod no change, my 'mars attacks' one needed it 'cuz I prepended 'Example' to the folder name....... Damn, it answers the PR requesting with a Unicorn...

rh-github-2015 commented 4 years ago

Possibly resolved by #2341? Please test and comment.

AdityaMH commented 3 years ago

This outdated?

SomeTroglodyte commented 3 years ago

This outdated

Looking at dates and issue number - definitely. I might pull it out of some backup one day - unless I've lost those..

yairm210 commented 3 years ago

Done by @SomeTroglodyte

yairm210 / Unciv

Music system improvements #2232