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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switched music paths to ensure windows compatibility #214

Merged
merged 13 commits into from
Jul 23, 2022

Conversation

lxgr-linux
Copy link
Owner

Please test @MaFeLP .

@lxgr-linux lxgr-linux added the bug Something isn't working label Jun 29, 2022
@lxgr-linux lxgr-linux requested a review from MaFeLP June 29, 2022 22:38
@lxgr-linux lxgr-linux self-assigned this Jun 29, 2022
@lxgr-linux lxgr-linux linked an issue Jun 29, 2022 that may be closed by this pull request
Copy link
Collaborator

@MaFeLP MaFeLP left a comment

Choose a reason for hiding this comment

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

Does not work on every map.

Tested works in Pokete Center.
Does not work in NiceTown or Fightmap:


Stacktrace for NiceTown

    Error 305 for command:
        open "C:\Users\maxfe\AppData\Local\Temp\PS_4klm1ho.mp3"
    Cannot specify extra characters after a string enclosed in quotation marks.

    Error 263 for command:
        close "C:\Users\maxfe\AppData\Local\Temp\PS_4klm1ho.mp3"
    The specified device is not open or is not recognized by MCI.
Failed to close the file: "C:\Users\maxfe\AppData\Local\Temp\PS_4klm1ho.mp3"
Process Process-3:
Traceback (most recent call last):
  File "C:\Users\maxfe\AppData\Local\Programs\Python\Python310\lib\multiprocessing\process.py", line 315, in _bootstrap
    self.run()
  File "C:\Users\maxfe\AppData\Local\Programs\Python\Python310\lib\multiprocessing\process.py", line 108, in run
    self._target(*self._args, **self._kwargs)
  File "C:\Users\maxfe\pokete\pokete_classes\audio.py", line 18, in audio_fn
    playsound(MUSIC_PATH / song)
  File "C:\Users\maxfe\AppData\Local\Programs\Python\Python310\lib\site-packages\playsound.py", line 44, in _playsoundWin
    _playsoundWin(tempPath, block)
  File "C:\Users\maxfe\AppData\Local\Programs\Python\Python310\lib\site-packages\playsound.py", line 72, in _playsoundWin
    winCommand(u'open {}'.format(sound))
  File "C:\Users\maxfe\AppData\Local\Programs\Python\Python310\lib\site-packages\playsound.py", line 64, in winCommand
    raise PlaysoundException(exceptionMessage)
playsound.PlaysoundException:
    Error 305 for command:
        open "C:\Users\maxfe\AppData\Local\Temp\PS_4klm1ho.mp3"
    Cannot specify extra characters after a string enclosed in quotation marks.

@lxgr-linux
Copy link
Owner Author

WTF

try:
from playsound import playsound
except ModuleNotFoundError:
from .dummy_playsound import playsound
from .settings import settings


MUSIC_PATH = __file__.replace("pokete_classes/audio.py", 'assets/music/')
MUSIC_PATH = Path(__file__).parent.parent / 'assets' / 'music'
Copy link
Contributor

Choose a reason for hiding this comment

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

just FYI, should be able to do Path(__file__).parents[1]

Copy link
Owner Author

Choose a reason for hiding this comment

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

Oh yes, that's a good tip.

Co-authored-by: Mustafa Quraish <[email protected]>
@lxgr-linux
Copy link
Owner Author

"C:\Users\maxfe\AppData\Local\Temp\PS_4klm1ho.mp3"

I wonder where this path comes from.

@lxgr-linux
Copy link
Owner Author

And there seams to be a problem with playsound: TaylorSMarks/playsound#98

@lxgr-linux
Copy link
Owner Author

Best option would be to enforce playsound=1.2.2, the best alternative pydub needs ffmpeg installed, which would be a hassle for the windows users.

@MaFeLP MaFeLP marked this pull request as draft July 21, 2022 17:26
I now went through the pain of setting up a windows vm and finly fixing
audio for windows
@lxgr-linux
Copy link
Owner Author

I now went through the pain of setting up a windows vm and finally fixing
audio for windows. Also take a look at the new installation instructions @MaFeLP .

@lxgr-linux lxgr-linux marked this pull request as ready for review July 22, 2022 13:20
@lxgr-linux lxgr-linux requested a review from MaFeLP July 22, 2022 13:21
@lxgr-linux
Copy link
Owner Author

It now does not rely on pynput anymore. At all.

@lxgr-linux
Copy link
Owner Author

This will also fix #217 and #124.

@MaFeLP
Copy link
Collaborator

MaFeLP commented Jul 23, 2022

Review status:

  • Music working on Windows
  • keys not on terminal after exit on Windows1
  • Music working on macOS
  • keys not on terminal after exit on macOS
  • Code review

Footnotes

  1. Interesting observation: when running .\pokete.py instead of python \.pokete.py, pokete will be launched in a separate window!

@lxgr-linux
Copy link
Owner Author

  1. Interesting observation: when running .\pokete.py instead of python \.pokete.py, pokete will be launched in a separate window! leftwards_arrow_with_hook

That looks like common windows BS and may not be related to Pokete at all.

Copy link
Collaborator

@MaFeLP MaFeLP left a comment

Choose a reason for hiding this comment

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

Apart from this one comment, everything looks good!

pokete.py Outdated Show resolved Hide resolved
Co-authored-by: MaFeLP <[email protected]>
README.md Outdated Show resolved Hide resolved
@lxgr-linux lxgr-linux requested a review from MaFeLP July 23, 2022 22:05
@lxgr-linux lxgr-linux merged commit 9e6db99 into master Jul 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Freeze the terminal screen Audio not working on windows in game keys still on terminal after exit
3 participants