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

Make headless=True run xvbf #641

Open
wants to merge 3 commits into
base: v1.0.0
Choose a base branch
from
Open

Conversation

neverix
Copy link

@neverix neverix commented Jun 29, 2022

It doesn't do anything anyway and there's no way to turn it on so why not

It doesn't do anything anyway and there's no way to turn it on so why not
@pep8speaks
Copy link

pep8speaks commented Jun 29, 2022

Hello @neverix! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2022-06-29 18:13:39 UTC

@Miffyli
Copy link
Contributor

Miffyli commented Jun 29, 2022

Hey! Thank you for this: this actually was a thing on my todo list but deep in the "good to haves" list 😅 . I will definitely test this out. Just a small request: change from full/absolute path to just xvfb-run command, as sometimes people might the commands available somewhere else, and then they can always add things to PATH if xvfb-run is not in PATH by default.

@neverix
Copy link
Author

neverix commented Jun 29, 2022

test this out

It is completely untested, I'm still struggling to install MineRL locally

change from full/absolute path

True, I wrote that thinking psutil couldn't read from PATH

@Miffyli
Copy link
Contributor

Miffyli commented Jun 29, 2022

Cheers, I will merge this once it is fully fleshed out (e.g., you can pass the "headless" argument from gym.make all down here or so), so that people can use it. In current form it not set-able, but that should not be a big work :)

Btw please first raise issues or discuss changes on Discord before making PRs, and only make PRs when you are confident it could be merged. This is to reduce spam for everyone included.

@neverix
Copy link
Author

neverix commented Jun 30, 2022

Yeah, this was kind of a joke when I first made the PR. I did not expect this to actually be useful. Will note for future contributions

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