Skip to content

(release_30) sound updates - bugfix and enhance#359

Merged
SlySven merged 3 commits intoMudlet:release_30from
Nyyrazzilyss:release_30-sound-updates
Dec 21, 2016
Merged

(release_30) sound updates - bugfix and enhance#359
SlySven merged 3 commits intoMudlet:release_30from
Nyyrazzilyss:release_30-sound-updates

Conversation

@Nyyrazzilyss
Copy link
Copy Markdown
Contributor

This moves sound from 4 static players into an unlimited QList,
fixing https://bugs.launchpad.net/mudlet/+bug/1645064

It also adds volume (0-100) to the LUA playSoundFile command, with 100
(maximum) substituted if not present for backwards compatability.

This moves sound from 4 static players into an unlimited QList,
fixing https://bugs.launchpad.net/mudlet/+bug/1645064

It also adds volume (0-100) to the LUA playSoundFile command, with 100
(maximum) substituted if not present for backwards compatability.
@SlySven
Copy link
Copy Markdown
Member

SlySven commented Dec 6, 2016

⚠️ The Linux-gcc-cmake build failed to configure itself correctly and died prematurely - I've tried CPR to bring it back to life but it iswas failing to obtain an updated cmake from the ppa:kalakris/cmake Ubuntu PPA and is consequently now consistentlywas failing with the default 2.8.7 version for that configuration. C.I. checks will now fail for all PRs until it gets fixed It seems it was a temporary glitch and now appears to be OK... ⚠️

@vadi2
Copy link
Copy Markdown
Member

vadi2 commented Dec 6, 2016

We've also frozen release_30 from getting new features, in an attempt to stem the flow of bugs resulting from them - so we can finally get 3.0 out the door... let's keep the improvement to development branch only.

replaced tabs with spaces
@Nyyrazzilyss
Copy link
Copy Markdown
Contributor Author

Sorry, I shouldn't have created release30 and dev commits at the same time with this. It's not really new features as much as a bug fix, it just happened that the easiest way to stop mudlet from crashing when you play 5 sounds simultaneously was to remove the limit. Adding volume at the same time (in lua) was only a line or two.

corrected location of comment
@SlySven
Copy link
Copy Markdown
Member

SlySven commented Dec 15, 2016

❓ I must confess that I did not get a crash on my Linux system with more than 4 sounds trying to be played, I think I got the most recently started sound being changed as expected from the existing code...

@Nyyrazzilyss
Copy link
Copy Markdown
Contributor Author

Taking another look, the sample code I used identifying this now works?
playSoundFile("\\tmp\\165331__ani-music__tubular-bell-of-death.wav") playSoundFile("\\tmp\\209740__yummie__minion-yahoo-2.wav") playSoundFile("\\tmp\\274736__sforsman__distort-ring-2.wav") playSoundFile("\\tmp\\85568__joelaudio__dragon-roar.wav") --playSoundFile("\\tmp\\45809__themfish__gas-fire-catch.wav")
Previously, if I uncommented the final line of that mudlet would freeze. I reinstalled epsilon, and it seems to now do what's expected/work properly. I would have sworn I was running epsilon at the time, though i'm wondering now if it was a self-built compile that i'd never changed the name of. I wouldn't have changed anything myself in regards to the code at that time, I didn't event touch it until after my script started crashing when multiple sounds got played simultaneously.

Copy link
Copy Markdown
Member

@vadi2 vadi2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works fine, thanks @Nyyrazzilyss!

@vadi2
Copy link
Copy Markdown
Member

vadi2 commented Dec 20, 2016

@SlySven I didn't get a crash either, but it adds the feature of more simultaneous sounds which is cool. Over to you

@SlySven
Copy link
Copy Markdown
Member

SlySven commented Dec 21, 2016

👍 So I'll hit the switch... buzz

@SlySven SlySven merged commit 3feb369 into Mudlet:release_30 Dec 21, 2016
@SlySven
Copy link
Copy Markdown
Member

SlySven commented Dec 21, 2016

...ouch!

I deeply regret that I messed things up here because I didn't spot that there are things here that match items on the development branch version of this PR. Although I have immediately reverted the change it has killed this PR as there isn't a way to "undo" the merge AND reactivate this PR.

@Nyyrazzilyss I think we need to resolve the error message presentation thing on #358 and any other concerns relevant to this branch and then you can re-PR a cleaned up single commit with all the right bits and none of the not-right bits based on top of the most up to date version of the release_30 branch as a new PR...

@Nyyrazzilyss
Copy link
Copy Markdown
Contributor Author

No problem, creating this at the same time I created the development branch pr was an error on my part in the first place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants