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

Taking screenshot or recording crashes the game #11

Closed
JeanAraujo opened this issue Jul 31, 2018 · 13 comments
Closed

Taking screenshot or recording crashes the game #11

JeanAraujo opened this issue Jul 31, 2018 · 13 comments

Comments

@JeanAraujo
Copy link
Contributor

My Ubuntu is in Portuguese and when I try to take a screenshot or record a gif, the game crashes trying to create a file:
File "/home/jean/Documentos/pyxel-debug/lib/python3.6/site-packages/PIL/Image.py", line 1947, in save fp = builtins.open(filename, "w+b") FileNotFoundError: [Errno 2] No such file or directory: '/home/jean/Desktop/pyxel-180731-095749.gif'

If I create a Desktop folder on my home directory the problem is solved, but I don't that's a good solution.
An alternative would be creating a subfolder inside the game's current directory. I would do a pull request, but even that being pretty simple, I think that's more like a design decision, so it's up to you ☺️

@kitao
Copy link
Owner

kitao commented Aug 1, 2018

The filename for captured images is decided in the _get_capture_filename method in app.py.
And it only takes care of Windows and Mac for now.

If there's some way to get the path to the desktop on Linux, I'll add the code for that. And if there's no common way for that, I think the home directory can be used instead of the desktop.

Do you know how to get the desktop path on Linux? (I hope it's not different for each distribution.)
Or I should use the home directory on Linux?

@JeanAraujo
Copy link
Contributor Author

JeanAraujo commented Aug 1, 2018

Both

import os
os.popen('xdg-user-dir DESKTOP').read().split('\n')[0]

and

import subprocess
subprocess.check_output(['xdg-user-dir', 'DESKTOP']).decode('utf-8').split('\n')[0]

Worked for me on Ubuntu and Fedora, I don't know if it works on all distribution though.

@kitao kitao closed this as completed in ea462c6 Aug 2, 2018
@kitao kitao reopened this Aug 2, 2018
@kitao
Copy link
Owner

kitao commented Aug 2, 2018

I fixed it and released as 0.7.4.
Could you confirm it works right?

@Godzil
Copy link

Godzil commented Aug 2, 2018

If you use that, please add a fallback, an external tool like xdg-user-dir is not necessarily installed:

godzil@box4 ~ $ xdg-user-dir
-bash: xdg-user-dir: command not found

@vesche
Copy link
Contributor

vesche commented Aug 2, 2018

@Godzil The implemented method that fixed this issue ea462c6 does not use xdg-user-dir.

@Godzil
Copy link

Godzil commented Aug 2, 2018

Well this is not the actual code:

path = os.popen('xdg-user-dir DESKTOP').read().split('\n')[0]

@JeanAraujo
Copy link
Contributor Author

@Godzil I was afraid that something like that would happen.
Two ways of solving this are:

  1. Using some other python module to do this and setting it as a dependency. (I think that there is a xdg module for python but I didn't check it out yet)
  2. Mixing my solution with @vesche 's to something like:
import subprocess
[...]
else:
    try:
        path = subprocess.check_output(["xdg-user-dir", "DESKTOP"], shell=True).decode('utf-8').split('\n')
    except subprocess.CalledProcessError:
        path = os.path.expanduser('~')

@kitao
Copy link
Owner

kitao commented Aug 2, 2018

@JeanAraujo @Godzil
Actually I don't have Linux environment now.
Though the code above looks correct, doest this code work on Linux actual? If so, I'll use this.

@JeanAraujo
Copy link
Contributor Author

JeanAraujo commented Aug 3, 2018

def _get_capture_filename():
        if os.name == 'nt':
            path = os.path.join(
                os.path.join(os.environ['USERPROFILE']), 'Desktop')
        else:
            try:
                path = subprocess.check_output(["xdg-user-dir DESKTOP"], shell=True).decode('utf-8').split('\n')[0]
            except subprocess.CalledProcessError:
                path = os.path.expanduser('~')

        return os.path.join(
            path,
            datetime.datetime.now().strftime('pyxel-%y%m%d-%H%M%S'))

Works for me, but I would first like @Godzil and @vesche thoughts on this 🤔

@Godzil
Copy link

Godzil commented Aug 3, 2018

@kitao As you must know there is not just one Linux distribution, and not all necessarily provide the same set of tools.

xdg-user-dir is not a tool installed by default on all distributions, on one of my computer, this tool does not exist, so relying only on it is not a good idea.

@JeanAraujo seems to be a good solution, try if the tool works, if not, fallback using the user folder, apart from missing Mac OS X, xdg-user is clearly not supported under Mac OS X.

@JeanAraujo
Copy link
Contributor Author

Oh, that's true, @Godzil
I'm sorry :p

   @staticmethod
    def _get_capture_filename():
        plat = platform.system()

        if plat == 'Windows':
            path = os.path.join(
                os.path.join(os.environ['USERPROFILE']), 'Desktop')
        elif plat == 'Darwin':
            path = os.path.join(
                os.path.join(os.path.expanduser('~')), 'Desktop')
        else:
            path = os.path.join(
                os.path.join(os.path.expanduser('~')), 'Desktop')
            if not os.path.exists(path):
                try:
                    path = subprocess.check_output(["xdg-user-dir DESKTOP"], shell=True).decode('utf-8').split('\n')[0]
                except subprocess.CalledProcessError:
                    path = os.path.expanduser('~')

        return os.path.join(
            path,
            datetime.datetime.now().strftime('pyxel-%y%m%d-%H%M%S'))

Well, this code looks a bit... extensive, but it supposedly works. I can't use a linux system at the moment so I'll check it later.
I still want @vesche thoughts on it.

@kitao
Copy link
Owner

kitao commented Aug 4, 2018

I imported the above code and released it as 0.7.5.
Please let me know if it doesn't work.

@kitao kitao closed this as completed Aug 4, 2018
@JeanAraujo
Copy link
Contributor Author

JeanAraujo commented Aug 4, 2018

Changed, tested and solved on #32.
Tested on Ubuntu 18.04.1 LTS

kitao pushed a commit that referenced this issue Aug 5, 2018
* Changes in _get_capture_filename

* Reformatting code
kitao added a commit that referenced this issue Jan 3, 2022
kitao added a commit that referenced this issue Jan 3, 2022
kitao added a commit that referenced this issue Jan 3, 2022
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

No branches or pull requests

4 participants