Feature - Implemented Add sounds to barrel smashing! (patch included)

heeheehee

Developer
Staff member
Yeah, I can't see a choice override for an individual choice number being feasible: in order to figure out the choice number, you'd pretty much always have to fetch the page in the first place (for the script writer, this is essentially the initial visit_url()).

A few relevant threads to this (side) discussion:
http://kolmafia.us/showthread.php?16505 -- a recent feature request for choice override handling.
http://kolmafia.us/showthread.php?11264 -- a relay script that modularizes choice overrides (it actually does fairly well, iirc, but I also remember it cluttering the CLI with a bunch of debug messages); we'd need wider adoption from script writers, though.
 

Bale

Minion
Perhaps I am confused. The images subdirectory created by running KoLmafia does not seem to be a good place for this because the cache clear command will delete the contents, thus requiring some mechanism to detect its absence and replace it.

That's a problem I hadn't considered. Definitely not desirable.

The images directory in the KoLmafia source tree contains nothing but images unless I have made an error inspecting it.

Your eyes are working differently from my own. On my PC I see /images contains favicon.ico, actionbar.6.css, styles.20150113.css and styles.css while /images/scripts only contains many javascript files.
 

fronobulax

Developer
Staff member
That's a problem I hadn't considered. Definitely not desirable.



Your eyes are working differently from my own. On my PC I see /images contains favicon.ico, actionbar.6.css, styles.20150113.css and styles.css while /images/scripts only contains many javascript files.

Sorry. Key words in my comment were "source tree". http://sourceforge.net/p/kolmafia/code/HEAD/tree/src/images/ at sourceforge or the equivalent of C:\KoLmafia\src\images if you are building locally. The one you see with sibling directories relay, scripts, settings and sessions is the one that is cleared by cache clear.
 
Today was actually the first time I tried to use this feature - been storing up a couple hundred barrels - and I was disappointed to hear no smashing sounds :(

Was the "default off, opt-in to turn it on" feature implemented, and I missed it? How do I make this work?

r16335, running the relay browser in Firefox 40.0.3.
 

lostcalpolydude

Developer
Staff member
Today was actually the first time I tried to use this feature - been storing up a couple hundred barrels - and I was disappointed to hear no smashing sounds :(

Was the "default off, opt-in to turn it on" feature implemented, and I missed it? How do I make this work?

r16335, running the relay browser in Firefox 40.0.3.

This thread isn't marked Implemented, because it hasn't been.
 
Oh, bummer. I didn't even notice the lack of an implementation note - I thought the ongoing discussion was about tweaks, not initial implementation. Thanks!
 

fronobulax

Developer
Staff member
Oh, bummer. I didn't even notice the lack of an implementation note - I thought the ongoing discussion was about tweaks, not initial implementation. Thanks!

Sorry. As the proxy for commit I have a couple of concerns that I am waiting for someone else to address. I get that an override script would be overly complicated but how and were for KoLmafia to supply media seems unresolved.
 

Veracity

Developer
Staff member
"How" to supply media was answered for you; post 12 pointed you to RelayRequest.constructURLString, which is where we set the media type based on the file extension, and post 13 says that the appropriate media type is audio/mpeg. Post 20 has my opinion, which is that "images" is a fine place to put it. The fact that "cache clear" clears that directory seems irrelevant, since we'll just copy it back out from the JAR when needed.

Which is to say, I'm not seeing any "unaddressed concerns".
 

fronobulax

Developer
Staff member
I shall review this thread and put the patch at the top of my todo list. Hopefully Crimbo will be a gradual ramp up this year :)
 
I'm excited about this, to a probably-unreasonable degree. I've smashed a few barrels here and there, but currently have 669 saved up for a ridiculous smash party with sound.
 

fronobulax

Developer
Staff member
I tried the patch. Two things. As is, relayAddSounds showed up as global for me and it really needs to be per user. I think I can figure that out. It initially did not work for me, but I copied barrel_smash.mp3 into /images and I was rewarded with the sound of a smashed barrel. Not sure about that. Will investigate later today...
 

lostcalpolydude

Developer
Staff member
I would have thought global makes more sense. Why would someone want sound for only some of their characters?

On the other hand, I won't ever enable it so it doesn't matter to me.
 

Theraze

Active member
Well, chatBeep, the other sound I can think of, is a global. That suggests to me that sound-controls should be consistent... if you think this should be per-character, then chatBeep should be per-character as well.
 

fronobulax

Developer
Staff member
Well, chatBeep, the other sound I can think of, is a global. That suggests to me that sound-controls should be consistent... if you think this should be per-character, then chatBeep should be per-character as well.

Yeah. In my perfect world where I was in charge and veractity was willing and able to vet my ideas before I opened my mouth I would eliminate global preferences completely. But in this world I agree that consistency counts for something and it will remain a global preference defaulting to false. Tangentially I note that the OP doesn't seem to have visited since since September. If PaladinWhite weren't so enthusiastic I'd consider abandoning this too ;-)
 

fronobulax

Developer
Staff member
r16515

I think the problems I had initially were because I expected the patch to add the sound file, which it did not.

Global setting, can be toggled in the GUI.
Sound file is in src/images in the source tree which is as good a place as any if there are more sounds in the future.
 
Top