Issue 5250 in pharo: Clicking on sound in Setting does not anything

Status: Accepted
Owner: [hidden email]
CC: [hidden email]
Labels: Milestone-1.4

New issue 5250 by [hidden email]: Clicking on sound in Setting does  
not anything
http://code.google.com/p/pharo/issues/detail?id=5250

in 1.3 I clicked on the sound setting in the setting browser and nothing  
happened.

I had to set the sound by hand using.
BaseSoundSystem soundEnabled: true



_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #1 on issue 5250 by [hidden email]: Clicking on sound in Setting  
does not anything
http://code.google.com/p/pharo/issues/detail?id=5250

It doesn't work because of the change to SoundService soundEnabled.

See SoundService #soundEnabled:/#soundEnabled which are invoked from the  
browser.

Like I told you back when the change was made, I still don't understand why  
a global switch for whether sound is enabled is not good enough.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #2 on issue 5250 by [hidden email]: Clicking on sound in Setting  
does not anything
http://code.google.com/p/pharo/issues/detail?id=5250

Here's what I propose:
1) Revert SoundService #soundEnabled / remove it from AbstractSoundSystem.
2) Revert api to the 1.0 -1.2 behaviour, so you write SoundService default  
beep, instead of SoundService default new beep. (This also makes it work  
with the external PharoSound package again.
3) Achieve 2 by moving the methods class-side. Since it's really a utility  
API class delegating to other parts of the same SoundSystem, it should not  
require any state.
4) Do the same for PharoSound, this eliminates the, I think,  
original "uglyness" objection that an AppRegistry which is supposed to used  
for classes, have instances registered to them.

If the above is acceptable, I'll make changes to base for 1.3/1.4, and  
after moving the methods/change the registration in PharoSound, we can have  
the same Package working on all versions of Pharo.



_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Labels: Type-Bug

Comment #3 on issue 5250 by [hidden email]: Clicking on sound in  
Setting does not anything
http://code.google.com/p/pharo/issues/detail?id=5250

(No comment was entered for this change.)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #4 on issue 5250 by [hidden email]: Clicking on sound in  
Setting does not anything
http://code.google.com/p/pharo/issues/detail?id=5250

Issue 4901 has been merged into this issue.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #5 on issue 5250 by [hidden email]: Clicking on sound in  
Setting does not anything
http://code.google.com/p/pharo/issues/detail?id=5250

Issue 3805 has been merged into this issue.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #6 on issue 5250 by [hidden email]: Clicking on sound in  
Setting does not anything
http://code.google.com/p/pharo/issues/detail?id=5250

In 1.4 the reason is that the package for sound is not loaded.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #7 on issue 5250 by [hidden email]: Clicking on sound in Setting  
does not anything
http://code.google.com/p/pharo/issues/detail?id=5250

No, it's not.

The reason is the setting uses the setter for  SoundService's SoundEnabled  
class var, but the getter accesses the classvar of the currently installed  
subclass. So you are modifying a variable which is never used when querying  
if sound is actually enabled.

See comment 1/2.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #8 on issue 5250 by [hidden email]: Clicking on sound in  
Setting does not anything
http://code.google.com/p/pharo/issues/detail?id=5250

Ok I will have a look. I remember our old discussion but not its exact  
contents.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Status: FixReviewNeeded

Comment #9 on issue 5250 by [hidden email]: Clicking on sound in  
Setting does not anything
http://code.google.com/p/pharo/issues/detail?id=5250

These are the pieces of code in question:

soundEnabled: aBoolean
        DefaultSoundEnabled := aBoolean



soundEnabled
        "By default we expect zero or one sound provider. When there is one we  
delegate to it."
        | system |
        system := self registeredClasses.
        system ifEmpty: [^self defaultSoundEnabled].
        ^ system first soundEnabled


soundEnabled
        "By default we expect zero or one sound provider. When there is one we  
delegate to it."
        | system |
        system := self registeredClasses.
        system ifEmpty: [^self defaultSoundEnabled].
        ^ system first soundEnabled


what about changing it to:

soundEnabled
         ^ self defaultSoundEnabled

?

Attachments:
        issue 5250.1.cs  225 bytes


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Status: Integrated

Comment #10 on issue 5250 by [hidden email]: Clicking on sound in  
Setting does not anything
http://code.google.com/p/pharo/issues/detail?id=5250

in 14423


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker