Windows: Skip cmd for release builds #5416

adrido · 2017-03-18T12:55:38Z

This skips the cmd window on startup for release builds. Startup time is much faster then.

The /INCREMENTAL:NO just solves a linker warning (Linking with /LTCG does not allow /INCREMENTAL)

SmallJoker · 2017-03-18T13:13:05Z

👎 The console window is helpful to develop mods and finding bugs. Compared to the debug.txt file all new lines can be viewed in the console instantly.

adrido · 2017-03-18T13:33:42Z

Yes I understand that it might usefull, but here I have some points against it:

Most players just want to play, and there is the console window just annoying.
Some young players might get irritated by seeing the console. (they does not know what it is)
No other program uses such a console window.
The console on windows before windows 10 is really useless, it is not even resizeable.
If someone want instant notice about what happens in the code, he can simply replace print(...) with minetest.chat_send_all(...)
A real application window makes a more native and finished look
If I remember correctly there is no console window on Linux popping up
On debug builds the console is still available

EDIT:
As it seems, that you are a coredev now, I want you to think about whats better for the whole Minetest project, and not only think about whats better for you 😉

Fixer-007 · 2017-03-18T14:06:03Z

Console is irritating and clutters my taskbar, If I really need it I will enable it in config. Players just want to play, console adds debug/notfinished feel to minetest and I need to damn select the right minetest window every time.
Also, there is debug.txt available, basically the same thing, with mtail it can be used as console anyway.
Can you make an option in config to enable/disable wincmd console somehow? With it disabled by default that will help a lot.

nerzhul · 2017-03-18T14:07:57Z

@SmallJoker cmd for debug build okay, but seems lgtm for release builds, it's not very proper and "profesionnal"

adrido · 2017-03-18T17:28:15Z

I added the cmake option ENABLE_CMD_WINDOW FALSE CACHE BOOL "Shows the cmd window in Release builds". If someone want it, its easy to enable.

sofar · 2017-03-18T22:12:36Z

👍 default disable, maybe this could actually be an option in minetest.conf, but, it seems @SmallJoker's concern for debug builds is entire covered this way.

nerzhul

Nice !

SmallJoker · 2017-03-19T09:31:55Z

@sofar This is a compiler parameter, thus it could be quite hacky to add a minetest.conf setting for the console window.

I want you to think about whats better for the whole Minetest project, and not only think about whats better for you 😉

@adrido Yes, that's true. My comment should only represent my own opinion - not meant as a downvote from a coredev. Blocking this PR if there are people who would like to have this window away would be nonsense. However, thanks for adding the CMake param in :)

nerzhul · 2017-03-19T09:43:59Z

@SmallJoker i think covering the whole usage are good, but for a end user point, console it crap, just permit to enable it for developers is good

adrido · 2017-03-19T10:14:13Z

@adrido Yes, that's true. My comment should only represent my own opinion - not meant as a downvote from a coredev. Blocking this PR if there are people who would like to have this window away would be nonsense. However, thanks for adding the CMake param in :)

Phew 😅 I thought this would end up in one of that "controversial" PR that does not have any future.

This PR changes the subsystem to Windows, this have to be done on link time. Its not possible to change the subsystem on run time. There exist (hacky) ways within the Windows API to allocate a new cmd window at runtime, but I does not know if this would have the wanted effect.

tobyplowy · 2017-03-19T11:12:43Z

@adrido i made an issue about this a while ago I'm happy that someone took the time to fix this

nerzhul · 2017-03-19T12:29:33Z

@sfan5 @Zeno- what is your opinion on this PR ?

sfan5 · 2017-03-19T13:43:50Z

agree that it's useful

however it's missing the code that does the same for MinGW

adrido · 2017-03-19T14:19:31Z

however it's missing the code that does the same for MinGW

If this document is a trusted source, the command line arguments might be
--subsystem "windows" --entry=mainCRTStartup, but I does neither know if this is the official documentation, nor I can verify that it works. Feel free to test it and create a PR to this branch, or a separate PR later.

Fixer-007 · 2017-03-19T14:25:38Z

From http:https://www.mingw.org/wiki/FAQ:

How do I remove DOS command windows?

In the link step add a "-mwindows" switch to the command line.

Also from https://gcc.gnu.org/onlinedocs/gcc/x86-Windows-Options.html:

-mwindows

This option is available for Cygwin and MinGW targets. It specifies that a GUI application is to be generated by instructing the linker to set the PE header subsystem type appropriately.

Fixer-007 · 2017-03-27T18:42:19Z

@adrido

adrido · 2017-03-27T18:55:50Z

Maybe the MinGW stuff should be added by someone who have more experience with MinGW than me. I does not like to blindly write code, that I cannot test.
I hope this can be considered to merge in current state, and someone else creates a separate PR for doing the same with MinGW later.

Fixer-007 · 2017-03-27T18:57:41Z

I can test it right now, I already pasted info above. It does not look like complicated stuff, also main builds are made with mingw.

sfan5 · 2017-03-28T10:56:15Z

I have used -mwindows before and can confirm that it has the intended effect (not tested it on MT tho).

Fixer-007 · 2017-03-28T12:13:19Z

I've tried it with -mwindows on MINGW64 cross-compilation on debian and had a success - no cmd window, starts up and runs fine, icon is visible in taskbar (but not in alt+tab taskbar switcher, but I'm ok with that). Debug.txt is working.
Here is what I did:
in src/CmakeLists.txt I found line https://github.com/adrido/minetest/blob/e85637b4e3d44112ba04fdf0cc456482805692ab/src/CMakeLists.txt#L771
and made it like this:

	if(MINGW)
		set(OTHER_FLAGS "${OTHER_FLAGS} -mthreads -fexceptions")
		if(NOT ENABLE_CMD_WINDOW)
			set(OTHER_FLAGS "${OTHER_FLAGS} -mthreads -fexceptions -mwindows")
		endif()
	endif()

By default minetest is compiled without cmd window, if needed -DENABLE_CMD_WINDOW=1 enables it, I checked.
Demo:

Cmake setup used:

cmake .. \
	-DCMAKE_TOOLCHAIN_FILE=$toolchain_file \
	-DCMAKE_INSTALL_PREFIX=/tmp \
	-DVERSION_EXTRA=$git_hash \
	-DBUILD_CLIENT=1 -DBUILD_SERVER=0 \
	\
	-DENABLE_SOUND=1 \
	-DENABLE_CURL=1 \
	-DENABLE_GETTEXT=1 \
	-DENABLE_FREETYPE=1 \
	-DENABLE_LEVELDB=1 \
	\
	-DIRRLICHT_INCLUDE_DIR=$libdir/irrlicht/include \
	-DIRRLICHT_LIBRARY=$libdir/irrlicht/lib/Win64-gcc/libIrrlicht.dll.a \
	-DIRRLICHT_DLL=$libdir/irrlicht/bin/Win64-gcc/Irrlicht.dll \
	\
	-DZLIB_INCLUDE_DIR=$libdir/zlib/include \
	-DZLIB_LIBRARIES=$libdir/zlib/lib/libz.dll.a \
	-DZLIB_DLL=$libdir/zlib/bin/zlib1.dll \
	\
	-DLUA_INCLUDE_DIR=$libdir/luajit/include \
	-DLUA_LIBRARY=$libdir/luajit/libluajit.a \
	\
	-DOGG_INCLUDE_DIR=$libdir/libogg/include \
	-DOGG_LIBRARY=$libdir/libogg/lib/libogg.dll.a \
	-DOGG_DLL=$libdir/libogg/bin/libogg-0.dll \
	\
	-DVORBIS_INCLUDE_DIR=$libdir/libvorbis/include \
	-DVORBIS_LIBRARY=$libdir/libvorbis/lib/libvorbis.dll.a \
	-DVORBIS_DLL=$libdir/libvorbis/bin/libvorbis-0.dll \
	-DVORBISFILE_LIBRARY=$libdir/libvorbis/lib/libvorbisfile.dll.a \
	-DVORBISFILE_DLL=$libdir/libvorbis/bin/libvorbisfile-3.dll \
	\
	-DOPENAL_INCLUDE_DIR=$libdir/openal_stripped/include/AL \
	-DOPENAL_LIBRARY=$libdir/openal_stripped/lib/libOpenAL32.dll.a \
	-DOPENAL_DLL=$libdir/openal_stripped/bin/OpenAL32.dll \
	\
	-DCURL_DLL=$libdir/libcurl/bin/libcurl-4.dll \
	-DCURL_INCLUDE_DIR=$libdir/libcurl/include \
	-DCURL_LIBRARY=$libdir/libcurl/lib/libcurl.dll.a \
	\
	-DGETTEXT_MSGFMT=`which msgfmt` \
	-DGETTEXT_DLL=$libdir/gettext/bin/libintl-8.dll \
	-DGETTEXT_ICONV_DLL=$libdir/gettext/bin/libiconv-2.dll \
	-DGETTEXT_INCLUDE_DIR=$libdir/gettext/include \
	-DGETTEXT_LIBRARY=$libdir/gettext/lib/libintl.dll.a \
	\
	-DFREETYPE_INCLUDE_DIR_freetype2=$libdir/freetype/include/freetype2 \
	-DFREETYPE_INCLUDE_DIR_ft2build=$libdir/freetype/include/freetype2 \
	-DFREETYPE_LIBRARY=$libdir/freetype/lib/libfreetype.dll.a \
	-DFREETYPE_DLL=$libdir/freetype/bin/libfreetype-6.dll \
	\
	-DSQLITE3_INCLUDE_DIR=$libdir/sqlite3/include \
	-DSQLITE3_LIBRARY=$libdir/sqlite3/lib/libsqlite3.dll.a \
	-DSQLITE3_DLL=$libdir/sqlite3/bin/libsqlite3-0.dll \
	\
	-DLEVELDB_INCLUDE_DIR=$libdir/leveldb/include \
	-DLEVELDB_LIBRARY=$libdir/leveldb/lib/libleveldb.dll.a \
	-DLEVELDB_DLL=$libdir/leveldb/bin/libleveldb.dll

adrido · 2017-03-29T09:53:59Z

Thanks for testing, but if it does not show up inside the ALT-TAB task switcher, something seems wrong to me. Or could that be because of your Windows version? It looks a bit older.

Fixer-007 · 2017-03-29T12:56:16Z

Icon in alt-tab mode is visible in console window, if you disable console, the behaviour for main window is the same as before. That can be improved later. I'm interested in getting this PR in for both msvc and mingw.

adrido · 2017-03-29T13:22:55Z

Ahh Sorry, did not understand you correctly. I understand that the whole Minetest application is not displayed in Task switcher 😆 . The Icon that is displayed in the Taskbar is the "Application Icon" that icon that's displayed in the "Classic" Task switcher is the "Window Icon". Since Windows 7 (Vista maybe too) the Window is displayed as a thumbnail. As long as Minetest does not have a proper Window Icon set there is nothing to display. If I remember correctly, the Window icon is hard coded by the Irrlicht engine to "irrlicht.ico"
Dont know if there exist other ways to set it, but just copy the "minetest-icon.ico" from misc\ to the "bin\" folder and rename it to "irrlicht.ico". Then it should display the proper Window Icon 😉.

Could you please check, if it also work if you do:

	if(MINGW)
		set(OTHER_FLAGS "${OTHER_FLAGS} -mthreads -fexceptions")
		if(NOT ENABLE_CMD_WINDOW)
			set(OTHER_FLAGS "${OTHER_FLAGS} -mwindows")
		endif()
	endif()

Fixer-007 · 2017-03-29T16:55:37Z

Dont know if there exist other ways to set it, but just copy the "minetest-icon.ico" from misc\ to the "bin" folder and rename it to "irrlicht.ico". Then it should display the proper Window Icon 😉.

Indeed! I did this and now I see proper icon for minetest everywhere (Windows 7 sp1)

Could you please check, if it also work if you do

Yes it does work.

rubenwardy · 2017-04-01T11:22:47Z

Is it possible to create a batch file to start with the console?

adrido · 2017-04-01T11:31:57Z

No, thats not possible. This is changing the subsystem where the application is running on. This have to be made at compile/ link time, and cannot be done on run time.

Fixer-007 · 2017-04-01T15:39:34Z

Tested. Thanks 👍

tenplus1 · 2017-04-01T16:15:58Z

Removing console would be a pain for mod makers when it comes to errors, having to refer to debug.txt all the time instead of simply looking at the cmd window... Also, if cmd was removed and minetest was made a windows program only a console would have to be added for just such checking so we would need someone to add that beforehand, which in itself comes with separate problems like when Mt crashes and closes the console window with it.

Fixer-007 · 2017-04-01T16:27:22Z

@tenplus1 http:https://ophilipp.free.fr/op_tail.htm

Fixer-007 · 2017-04-01T16:45:14Z

Downside of this is you don't see command line commands or any output when you start it from CMD.
But there are workarounds I think

Relevant:
https://stackoverflow.com/questions/13100785/mingw-build-gui-application-with-console
http:https://mingw-users.narkive.com/FoOlISaF/mwindows-with-printf-output-to-console-that-i-start-the-program
https://sourceforge.net/p/mingw/mailman/mingw-users/thread/oprpmtjhzencqdf8@localhost/

adrido · 2017-04-01T16:55:57Z

I found a simple way to dynamically allocate a console window within pidgin source code. [sarcasm] And it also leads all that useless crap to that console too (like it did before). hope that spam makes people happy again 😛 [/sarcasm]

@Fixer-007 you answered while i was compiling and taking that screenshot. Its similar to the last answer of your link. 😆

That solution does require some more programming to do it right, so I may need some more days.
Just a question to coredevs, how should the commandline argument be called to make the console visible?

celeron55 · 2017-04-01T17:01:14Z

What happens if Minetest is started from a console? Does it create a new console or use the existing one?

celeron55 · 2017-04-01T17:08:56Z

For that option, I'd suggest simply "--console".

Ezhh · 2017-04-01T17:10:00Z

I use console a lot from Windows version of MT. If it can be an option to enable, and then I run the game as usual and the console is there, it's great, otherwise this will be a huge annoyance. (And if it's not there by default, having to enable an option each time I test a client (and, I assume, then restart the game?) would also be a (slightly) lesser annoyance by itself.

adrido · 2017-04-01T17:10:41Z

My idea is to check if AttachConsole(ATTACH_PARENT_PROCESS) is successfull, then its called from a parrent console. If not, allocate a new console. So this is better then the console window before, because it does not open a new console if there is already one.I Just did some tests, and it works as expected. :-)
I does currently not know what happens if the parent process is not a cmd window. I assume it does not open a cmd window

celeron55 · 2017-04-01T17:14:32Z

It looks like we need to also add the option to pop up the console window like it currently does, and also make that behavior configurable in minetest.conf. We can't just decide people shouldn't like the current behavior.

rubenwardy · 2017-04-01T18:29:18Z

--console is enough if you also add a batch file
Whichever is cleaner

adrido · 2017-04-01T19:38:10Z

should work now except for reading the setting. I does not find the point when the settings-file got parsed, and g_settings is available.

Fixer-007 · 2017-04-06T21:13:40Z

Status?

nerzhul · 2017-04-06T21:51:27Z

i don't like windows but it's inacceptable for end users to have an opened cmd, minetest is not a Dos game on Windows 95

sofar · 2017-04-07T03:40:40Z

Looking at this patch it adds all the options needed for the console to be easily reachable:

--console option
minetest.conf option
console is started when relevant (e.g. server is started)

So, 👍 (But I can't test).

Fixer-007 · 2017-04-07T12:14:13Z

Got very nice results for now on mingw64 cross-compile from lin to win (updating):

starts without console by default
command line options work
minetest --console via link or cmd opens the console
minetest with "enable_console = true" in minetest.conf also opens console
debug.txt and console output seems good, nothing suspicious
enable_console is missing from settingtypes and minetest.conf (will be added in trivial PR)
console is enabled and opened by default on "Debug" builds

Finally that CMD window is gone, Console can be enabled easily via many ways, everyone is satisfied.
Big thanks to @adrido 👍

adrido force-pushed the no-cmd-on-release branch from a16b666 to 46e143b Compare March 18, 2017 17:24

adrido force-pushed the no-cmd-on-release branch from 46e143b to e85637b Compare March 18, 2017 17:31

nerzhul approved these changes Mar 19, 2017

View reviewed changes

nerzhul added @ Build CMake, build scripts, official builds, compiler and linker errors One approval ✅ ◻️ labels Mar 19, 2017

This was referenced Mar 19, 2017

Hiding command prompt? #4210

Closed

Don't show console window by default on windows #2128

Closed

adrido force-pushed the no-cmd-on-release branch 2 times, most recently from 4000365 to 36f22f3 Compare April 1, 2017 19:36

adrido mentioned this pull request Apr 2, 2017

Set Irrlichts loglevel to Minetests loglevel #5501

Closed

Windows: Skip cmd for release builds

0f12caa

adrido force-pushed the no-cmd-on-release branch from 36f22f3 to 0f12caa Compare April 2, 2017 09:12

Fixer-007 mentioned this pull request Apr 6, 2017

Minetest 0.4.16 roadmap #5530

Closed

nerzhul added this to the 0.4.16 milestone Apr 6, 2017

sofar added >= Two approvals ✅ ✅ and removed One approval ✅ ◻️ labels Apr 7, 2017

nerzhul merged commit 676951d into minetest:master Apr 7, 2017

nerzhul added this to Done in Minetest 0.4.16 Apr 29, 2017

adrido deleted the no-cmd-on-release branch August 19, 2017 03:05

Windows: Skip cmd for release builds #5416

Windows: Skip cmd for release builds #5416

Conversation

adrido commented Mar 18, 2017

SmallJoker commented Mar 18, 2017

adrido commented Mar 18, 2017 • edited Loading

Fixer-007 commented Mar 18, 2017 • edited Loading

nerzhul commented Mar 18, 2017

adrido commented Mar 18, 2017

sofar commented Mar 18, 2017

nerzhul left a comment

Choose a reason for hiding this comment

SmallJoker commented Mar 19, 2017 • edited Loading

nerzhul commented Mar 19, 2017

adrido commented Mar 19, 2017

tobyplowy commented Mar 19, 2017

nerzhul commented Mar 19, 2017

sfan5 commented Mar 19, 2017 • edited Loading

adrido commented Mar 19, 2017

Fixer-007 commented Mar 19, 2017 • edited Loading

Fixer-007 commented Mar 27, 2017

adrido commented Mar 27, 2017

Fixer-007 commented Mar 27, 2017 • edited Loading

sfan5 commented Mar 28, 2017

Fixer-007 commented Mar 28, 2017 • edited Loading

adrido commented Mar 29, 2017

Fixer-007 commented Mar 29, 2017 • edited Loading

adrido commented Mar 29, 2017

Fixer-007 commented Mar 29, 2017

rubenwardy commented Apr 1, 2017

adrido commented Apr 1, 2017

Fixer-007 commented Apr 1, 2017 • edited Loading

tenplus1 commented Apr 1, 2017

Fixer-007 commented Apr 1, 2017

Fixer-007 commented Apr 1, 2017 • edited Loading

adrido commented Apr 1, 2017

celeron55 commented Apr 1, 2017

celeron55 commented Apr 1, 2017

Ezhh commented Apr 1, 2017

adrido commented Apr 1, 2017

celeron55 commented Apr 1, 2017

rubenwardy commented Apr 1, 2017

adrido commented Apr 1, 2017

Fixer-007 commented Apr 6, 2017

nerzhul commented Apr 6, 2017

sofar commented Apr 7, 2017

Fixer-007 commented Apr 7, 2017 • edited Loading

adrido commented Mar 18, 2017 •

edited

Loading

Fixer-007 commented Mar 18, 2017 •

edited

Loading

SmallJoker commented Mar 19, 2017 •

edited

Loading

sfan5 commented Mar 19, 2017 •

edited

Loading

Fixer-007 commented Mar 19, 2017 •

edited

Loading

Fixer-007 commented Mar 27, 2017 •

edited

Loading

Fixer-007 commented Mar 28, 2017 •

edited

Loading

Fixer-007 commented Mar 29, 2017 •

edited

Loading

Fixer-007 commented Apr 1, 2017 •

edited

Loading

Fixer-007 commented Apr 1, 2017 •

edited

Loading

Fixer-007 commented Apr 7, 2017 •

edited

Loading