minerl icon indicating copy to clipboard operation
minerl copied to clipboard

Make headless=True run xvbf

Open neverix opened this issue 2 years ago • 5 comments

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

neverix avatar Jun 29 '22 03:06 neverix

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! :beers:

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

pep8speaks avatar Jun 29 '22 03:06 pep8speaks

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.

Miffyli avatar Jun 29 '22 07:06 Miffyli

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

neverix avatar Jun 29 '22 18:06 neverix

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.

Miffyli avatar Jun 29 '22 19:06 Miffyli

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

neverix avatar Jun 30 '22 11:06 neverix