Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change the order of types (SoundManager) #6753

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wooseok123
Copy link

@wooseok123 wooseok123 commented Feb 29, 2024

Since the default audio context is Web Audio in Phaser, I believe many developers will predominantly use Web Audio. I encountered an issue while trying to use the decodeAudio() as described in Phaser.Sound.WebAudioSoundManager. The tsc throws an error because the type of Phaser.Game.Sound starts with Phaser.Sound.NoAudioSoundManager.

Property 'decode' does not exist on type 'NoAudioSoundManager | HTML5AudioSoundManager | WebAudioSoundManager'.
Property 'decode' does not exist on type 'NoAudioSoundManager'.

While I understand that this is not mandatory, I believe it would be more helpful for developers if the order of the types was reversed. 馃憤

Please consider the following change:

// Current Order
sound: Phaser.Sound.NoAudioSoundManager | Phaser.Sound.HTML5AudioSoundManager | Phaser.Sound.WebAudioSoundManager;

// Proposed Order
sound: Phaser.Sound.WebAudioSoundManager | Phaser.Sound.HTML5AudioSoundManager | Phaser.Sound.NoAudioSoundManager;

This adjustment prioritizes WebAudioSoundManager while developing, which contains the decodeAudio(), potentially reducing issues related to the mentioned tsc error or etc.

Thank you!

@samme
Copy link
Contributor

samme commented Mar 3, 2024

I think the error is correct because the actual Sound Manager may be Web Audio, HTML5, or No Audio depending on the device.

@wooseok123 wooseok123 closed this Mar 4, 2024
@wooseok123 wooseok123 reopened this Mar 4, 2024
@wooseok123
Copy link
Author

wooseok123 commented Mar 4, 2024

I think the error is correct because the actual Sound Manager may be Web Audio, HTML5, or No Audio depending on the device.

Actually it is not a real error of tsc during the build. While developing on typescript environment, i think tsc implicitly infer the first index of enumerated strings ( NoAudioSoundManager | HTML5Audio | WebAudio ), so its default SoundManager on the dev environment is NoAudioSoundManager.

So linter or tsc throws an type error if i try to use decodeAudio of WebAudioSoundManager.

I do believe it's not necessary because as you said, SoundManager type will be correctly assigned accoring to their devices after builded.

But it'll be more helpful for developers to change the order while developing with typescript. (Because then tsc implicitly infer the WebAudioSoundManager, which is default context of Phaser, preventing potential issues when they try to use WebAudioSoundManager. )

@wooseok123
Copy link
Author

wooseok123 commented Mar 4, 2024

Here's the situation i mentioned above while developing Phaser with Typescript.

image

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.

None yet

2 participants