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

[Windows, 3.x] Improve console handling and execute. #55987

Merged
merged 1 commit into from
Jan 10, 2022

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Dec 16, 2021

Alternative for the #55967 improved version of #41164, shouldn't be affected by #41328 (which was caused by adb running on background and usage of _wpopen()).

With these changes, console behavior on Windows should be the same as on Linux and macOS:

  • When editor is started from the console (or by another process with active console), output will be redirected to the parent console (including output from the running projects).
  • When editor is started from the GUI (shortcut, Windows Explorer, etc.), no console is opened.

Changes:

  • Always build with the GUI subsystem.
  • Redirect stdout and stderr output to the parent process console.
  • Use CreateProcessW for blocking execute calls with piped stdout and stderr (prevent console windows for popping up when used with the GUI subsystem build, and have more consistent behavior with non-blocking calls).
  • Add open_console argument to the execute to open a new console window (for both blocking and non-blocking calls).
  • Remove interface/editor/hide_console_window editor setting.
  • Remove Toggle System Console menu option.
  • Remove set_console_visible and is_console_visible functions.

@DmitriySalnikov
Copy link
Contributor

It turns out that it will no longer be possible to launch Godot from a shortcut on the desktop with output to the console?
And there is no way to add an argument to run godot with the console, for example godot.exe --open_console?

@bruvzg
Copy link
Member Author

bruvzg commented Dec 17, 2021

It turns out that it will no longer be possible to launch Godot from a shortcut on the desktop with output to the console?

Command line argument won't be passed to the child processes and console will be allocated for the process manager (the same issue as with passing --video-driver etc.).

But, you can use a batch file, like this:

@echo off
godot.windows.opt.tools.64.exe
pause > nul

Or add cmd /k to the target in the shortcut:
Screenshot 2021-12-17 111505

In both cases behavior will be similar to old. The only difference, terminal won't auto close after exiting/crashing the editor.

@bruvzg bruvzg marked this pull request as ready for review December 17, 2021 09:35
@bruvzg bruvzg requested review from a team as code owners December 17, 2021 09:35
@bruvzg
Copy link
Member Author

bruvzg commented Dec 17, 2021

Everything seems to be working, but there's one breaking change.

Previously, it was possible to directly execute built-in commands like dir (it was working only in the blocking mode with piped output), now it won't work anymore, and cmd /c should be prepended to the command.

It's consistent with the non-blocking execute and should not be an issue, but probably it's too big change for 3.4.1 and should be kept for 3.5. Also, documentation need to be checked and updated accordingly. (Documentation already suggests cmd /c for built-in and composite commands).

Tested it with MSVC, MinGW/GCC and MinGW/LLVM builds (original RedirectIOToConsole() code was working with GCC only).

Always build with the GUI subsystem.
Redirect stdout and stderr output to the parent process console.
Use CreateProcessW for blocking `execute` calls with piped stdout and stderr (prevent console windows for popping up when used with the GUI subsystem build, and have more consistent behavior with non-blocking calls).
Add `open_console` argument to the `execute` to open a new console window (for both blocking and non-blocking calls).
Remove `interface/editor/hide_console_window` editor setting.
Remove `Toggle System Console` menu option.
Remove `set_console_visible` and `is_console_visible` functions.
@akien-mga akien-mga merged commit 8a192cd into godotengine:3.x Jan 10, 2022
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants