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

Start browser using proc_play #2602

Merged
merged 1 commit into from
Sep 27, 2022
Merged

Conversation

tong
Copy link
Contributor

@tong tong commented Sep 27, 2022

Use state.proc_play instead of webbrowser.open to start the html5 player.
This allows to stop the browser from withing blender (Stop button) and prevents opening multiple browser windows, like on krom.
Not sure why this wasn't the default behavior, but i can't see any drawbacks.

@MoritzBrueckner MoritzBrueckner added the Release Notes: Changes A pull request that is a feature change, not a fix. Used to generate release notes. label Sep 27, 2022
@luboslenco luboslenco merged commit 76f2d02 into armory3d:main Sep 27, 2022
@MoritzBrueckner
Copy link
Collaborator

Sorry that I just now got the time to test this after it is already merged, unfortunately it doesn't seem to work with Firefox on Windows:

Traceback (most recent call last):
  File "[...]/armory\blender\arm\handlers.py", line 143, in poll_threads
    raise e
  File "[...]/armory\blender\arm\handlers.py", line 136, in poll_threads
    callback()
  File "[...]/armory\blender\arm\make.py", line 488, in build_done
    build_success()
  File "[...]/armory\blender\arm\make.py", line 571, in build_success
    state.proc_play = run_proc(cmd, play_done)
  File "[...]/armory\blender\arm\make.py", line 78, in run_proc
    p = subprocess.Popen(cmd)
  File "[...]\Blender\Blender 3.3\3.3\python\lib\subprocess.py", line 966, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "[...]\Blender\Blender 3.3\3.3\python\lib\subprocess.py", line 1435, in _execute_child
    hp, ht, pid, tid = _winapi.CreateProcess(executable, args,
OSError: [WinError 87] Falscher Parameter

The traceback looks a bit weird (handlers.py:143 before handlers.py:136), I suppose that the first line is raising this very exception that only starts with the second line in the traceback.

Falscher Parameter translates to wrong parameter, the cmd list is ['', 'http://localhost:8040/build_ui_canvas/debug/html5/']. So apparently webbrowser.get().name doesn't work like expected.

The type of webbrowser.get() is WindowsDefault in my case, which seems to be created whenever the browser is set up as Window's default browser. Unfortunately, it doesn't seem to get instanciated with a name because Python then delegates all the browser handling to the system and doesn't care about the specific browser.

Perhaps it would be a good compromise to check whether the returned name is empty, and if it is use webbrowser.open() like before?

@tong
Copy link
Contributor Author

tong commented Sep 27, 2022

@MoritzBrueckner
Copy link
Collaborator

MoritzBrueckner commented Sep 27, 2022

Yes, but it doesn't seem to have any advantage over the old approach since it is a shell command (you'd need to set shell=True) that will return immediately after invocation. There's also no difference regarding opening new windows, it never happend to me that Armory opened multiple browser windows with Firefox on Windows, instead the game was/is always opened in a new tab.

@tong
Copy link
Contributor Author

tong commented Sep 28, 2022

The goal is to be able to pass custom arguments to the browser launch command (#2476) which isn't possible when using the webbrowser module.

I can't see any difference in webbrowser.open and webbrowser.get so i am not sure why this fails. https://github.com/python/cpython/blob/dc0a87d9a0cf18fdfc51865a3c1bd2d5ebe9eae9/Lib/webbrowser.py#L605

.. opened in a new tab. Exactly.
Makes already 2 :)

@MoritzBrueckner
Copy link
Collaborator

Ah yes, then using start will be the better solution :)

It's not directly about webbrowser.get(), but about webbrowser.get().name. When using open(), Python is only interested in opening whatever browser the system is using, but if we want to start it from the command line we need its name, which Python simply doesn't know in this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release Notes: Changes A pull request that is a feature change, not a fix. Used to generate release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants