News:

Simutrans Sites
Know our official sites. Find tools and resources for Simutrans.

[Haiku] use find_directory instead of getenv to determine home directory

Started by taos, August 16, 2015, 06:22:25 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

taos

After introduction of launch_daemon in Haiku last month (2015-07-17_introducing_launch_daemon), a few ported applications such as Qupzilla and Simutrans started to show strange behaviour. They could no longer find their profile or launch saved games because they looked for paths containing quotation marks around /boot/home, e.g. "/boot/home"/simutrans. This was a result of how getenv("HOME") was implemented in Haiku.
After changing the implementation, Simutrans can now again load savegames or its configuration file on Haiku. However, one of the Haiku developers working on the getenv bug suggested a more future-proof (e.g. in regard of multi-user) approach by replacing getenv with find_directory, the "Haiku way" of searching for a specific directory.

I've attached a patch that would introduce a new condition for Haiku (in addition to already established clauses for Windows and Apple) to use find_directory instead of getenv to determine the home directory.

Edit: I forgot to mention that I checked if I could compile and run simutrans with this patch. Such a binary seems to work fine, even on older Haiku revisions that still include the getenv bug.

Ters

Eeek! sprintf with parameters coming from the outside! This part of Simutrans really needs some more safety. (Although I don't think it's a security vulnerability, as the user himself, or someone with even higher privileges must, set the hostile environment variables. Unfortunately, this is where doing the right thing might not be supported on a platform. Do you know if C11 is supported on Haiku? (Not that it helps much, so long as mingw doesn't.)

(As for Haiku delivering paths with quotes, and not understanding them when it gets them back, that's an error on their part I think.)

Using the proper API for finding the home directory seems like a good idea to me. The Windows code should also do this, rather than dig into some (likely) undocumented registry values.

DrSuperGood

QuoteThe Windows code should also do this, rather than dig into some (likely) undocumented registry values.
Patch attached. Not sure about compatibility with nightly build server though.

Offers 3 solutions. For builds targeting Vista and above it uses the current folder path API to locate the document folder. For builds targeting Windows 2000 and above it uses the deprecated folder path API to track down the document folder. For builds older than that or if the compiler fails to declare its target windows version correctly then it resorts to the previous registry method of finding the folder.

I have tested builds with both the modern API and deprecated API and they both seem to correctly track down the folder.

There is also a minor fix for typo when targeting Windows 8 and newer.

Ters

That was quick! I don't think we need multiple compile time switched implementations, though. Just go with SHGetFolderPath. Microsoft will support it for a long long time yet, and our official builds will likely support Windows XP for several years to come. (We just dropped Windows Me and older this year.) Nor does mingw know of SHGetKnownFolderPath.

NTDDI_VERSION is also not supported on Mingw, so be careful with its use. In this case, undefined macro equals undefined macro, so the Win8 code will always be selected on Mingw, despite Mingw not knowing of such recent APIs. That was however a problem with the old code as well, since a defined macro is apparently always greater than or equal to an undefined macro.

taos

Quote from: Ters on August 16, 2015, 08:53:09 PM
Do you know if C11 is supported on Haiku? (Not that it helps much, so long as mingw doesn't.)

Since Simutrans must be compiled with gcc4 on Haiku, C11 (or at least most of C11) should be supported on Haiku, as far as I know. From yesterday's IRC log:


[18:01:20] <eanyx> Do you have ported C++11 or even C++14?
[18:01:45] <PulkoMandy> yes, we have a version of gcc4 available and you can use it to compile apps
[18:02:13] <PulkoMandy> on the kernel side, the x86 version must stay compatible with BeOS, and for this reason uses gcc2 which doesn't support C++11/14
[18:02:31] <PulkoMandy> but for the other ports (including x86_64), we use gcc4 only and we use C++11 also in the kernel


So, I suppose, C11 shouldn't be a problem for Haiku.

Quote
(As for Haiku delivering paths with quotes, and not understanding them when it gets them back, that's an error on their part I think.)

That's a bug that has been already fixed for the latest nightlies.

DrSuperGood

QuoteI don't think we need multiple compile time switched implementations, though. Just go with SHGetFolderPath. Microsoft will support it for a long long time yet, and our official builds will likely support Windows XP for several years to come.
The official MS documentation states...
QuoteNew applications should use the known folder system rather than the older CSIDL system, which is supported only for backward compatibility.
As such I provided an implementation using the new system. The old implementation is supported but intended to be eventually removed.

If what you say is true then possibly the old registry implementation can be discarded. (since anything after 2000 supports the SHGetFolderPath implementation).

Ultimately the entire path system needs to be redefined in simutrans so that the core calls methods of some sort of "path" class rather than directly manipulating path strings. This would decouple the platform dependency of path separators as well as the underlying type of a path string to two files (header and implementation). As far as I can tell MS wants one to use WSTRING (16 bit characters) to deal with paths since vista and that they return a ready constructed String reference for the path (no dealing with path length) rather than trying (and possibly failing) to fill a character buffer. The path class approach was adopted in Java for the nio interface to eliminate the problem with implementation specific path strings.

QuoteNTDDI_VERSION is also not supported on Mingw, so be careful with its use. In this case, undefined macro equals undefined macro, so the Win8 code will always be selected on Mingw, despite Mingw not knowing of such recent APIs. That was however a problem with the old code as well, since a defined macro is apparently always greater than or equal to an undefined macro.
It is hard to know what Windows.h it has...

From what I have read NTDDI_VERSION is the newest version macro and it does everything including specifying specific service packs (important as they add support for some features). Although still used _WIN32_WINNT is deprecated now and is checked for consistency with NTDDI_VERSION (they both should match). WINVER is obsolete, it is (or should be) automatically set based on _WIN32_WINNT for legacy support.

In any case they should be command line defined macros which specify the target build (so you can automate multiple target builds at once). However I am guessing all the needed defines are missing from the version header (sigh).

Could people better explain the relationship of Simutrans with MinGW? I mean I thought the nightly servers were Linux but were linking for Windows. If they are Windows servers why can they not use MSVC compilers for the build? Or is it that they need that support to build the Linux builds from Windows?

Ters

Quote from: DrSuperGood on August 17, 2015, 05:54:16 PM
The official MS documentation states...As such I provided an implementation using the new system. The old implementation is supported but intended to be eventually removed.

But since we are going to support OSes not having the new API, there is no point in implementing the new for a handful of developers compiling Simutrans themselves, while the world at large uses something else. In fact, it may run against itself, because they will not notice if something is wrong with the old API that everyone else is actually using. Had it been a runtime switch, it could have made some sense, but Microsoft has so far never removed an API. Windows 10 happily detected that one of the games I had used DirectPlay, and automatically installed it without complaining. 32-bit Windows still supports the 16-bit Windows API from the 1980s, and while just taking the source code for such a program and compiling it with Visual Studio 10, it probably won't be API differences that cause compilation to fail (if the right typedef-ed types were used, at least).

Quote from: DrSuperGood on August 17, 2015, 05:54:16 PM
If what you say is true then possibly the old registry implementation can be discarded. (since anything after 2000 supports the SHGetFolderPath implementation).

At least the GDI build no longer supports non-NT, since it uses the Unicode API and we no longer link against unicows, and I think NT 4.0 would be a very rare find on the home market. And again, since you wrote compile time switches, only the oldest code would be used for our releases anyway. The newer code would just be wasted. I don't think there is much point is confusing the players even more with releases for different Windows versions. Having both GDI and SDL seems to cause enough trouble, as they have no idea what they downloaded when seeking technical support.

Quote from: DrSuperGood on August 17, 2015, 05:54:16 PM
From what I have read NTDDI_VERSION is the newest version macro and it does everything including specifying specific service packs (important as they add support for some features). Although still used _WIN32_WINNT is deprecated now and is checked for consistency with NTDDI_VERSION (they both should match). WINVER is obsolete, it is (or should be) automatically set based on _WIN32_WINNT for legacy support.

I don't understand what Microsoft is up to with all these version macros. The distinction between WINVER and _WIN32_WINNT I get, because there were two pretty different implementations of WIN32 in the 1990s, but what's this NTDDI_VERSION. The name NTDDI doesn't make any sense to me. When I first saw it, I thought you had been confused and used one of the macros for the DDK (driver development kit). Maybe they felt they couldn't use a name line _WIN32_WINNT anymore because the concept of Windows NT is slipping into the mists of time, and because they plan to deprecate the entire Win32 API (and native code in general).

Quote from: DrSuperGood on August 17, 2015, 05:54:16 PM
Could people better explain the relationship of Simutrans with MinGW? I mean I thought the nightly servers were Linux but were linking for Windows. If they are Windows servers why can they not use MSVC compilers for the build? Or is it that they need that support to build the Linux builds from Windows?

Prissi has explained to Simutrans is properly built with GCC on all platforms, because the handwritten assembly deemed necessary for performance uses GCC syntax. A Linux build server can cross compile for all supported platforms, or at least the three major ones, using GCC, though that may be a nice consequence rather than by design for all I know. I'm not sure what OS the nightly build server is, but if there is only one (most likely), it probably isn't Windows, as setting up cross compilers there isn't as easy. (You have to use cygwin, I guess. I don't know how it's package manager is or how up-to-date it's packages are.)

Some of the developers also don't like IDEs, and Microsofts compiler toolset is not as powerful on their own as the GCC toolset is (especially make is stuck in the 70s or 80s), but I don't know if they use Windows at all, so that point might be moot. And historically, Visual Studio didn't have a free version that could make proper executables. (I remember the first I ever knew was only Visual Basic, and could only make ActiveX components.)

DrSuperGood

Quotewhat's this NTDDI_VERSION
It provides support for different service packs. The other version macros did not particularly differentiate between service packs. Here they explain to some extent.

For example...
_WIN32_WINNT and WINVER -> _WIN32_WINNT_WINXP (0x0501)
NTDDI_VERSION -> NTDDI_WINXP (0x05010000), NTDDI_WINXPSP1 (0x05010100), NTDDI_WINXPSP2 (0x05010200) and NTDDI_WINXPSP3 (0x05010300)

This is important since many of the windows libraries are only available at a certain service pack levels. This is why a lot of software says "Requires Windows XP Service Pack 3" because the software must link to APIs needing XP SP3 rather than release, SP1 or SP2.

From what I can tell they are all integral for modern versions of Windows.h which handles all Windows API declarations. By default the API should declare these to the highest possible Windows version supported (Windows 8.1 for MSVC 2013 and likely Windows 10 for MSVC 2015).

I do understand the problem of no macro declarations for older build systems. Finding a solution is tricky however.

So what is the preferable way to differentiate Windows versions? Should I go back to the legacy "WINVER"? Would it have the Vista constant? Is it really not possible to use a more modern version of the Windows.h file or is it a third party version of Windows.h?

Maybe if I reverse the declaration order? So that it evaluates oldest by testing if version is less than the constant first running to newest? Hopefully in the case of uninitialized constants it would then use the "old" cases rather than taking the undeclared cases.

Ters

Quote from: DrSuperGood on August 17, 2015, 10:17:17 PM
It provides support for different service packs. The other version macros did not particularly differentiate between service packs.

Ah, yes, I had forgotten how IE version macros were used for that in the past. It doesn't explain the rather cryptic name, though.

Quote from: DrSuperGood on August 17, 2015, 10:17:17 PM
So what is the preferable way to differentiate Windows versions? Should I go back to the legacy "WINVER"? Would it have the Vista constant?

I seem to remember that somewhere else in the code, a version constant is specified as a literal, possibly because the corresponding macro did not exist in Mingw (at the time, which was around when someone did a botched job at updating the Mingw headers, which has since been reverted).

Quote from: DrSuperGood on August 17, 2015, 10:17:17 PM
Is it really not possible to use a more modern version of the Windows.h file or is it a third party version of Windows.h?

The official Windows SDK uses non-standard Microsoft extensions to C++, so the official headers can't be used with GCC (don't know about Intel's compiler), even though GCC does support some Microsoft extensions. Simply taking the official headers and adapting them to GCC would be in violation of copyrights, so Mingw has the write their Windows API header files from scratch based on the freely available information on MSDN. This is tedious, so virtually nothing happens anymore. The founder of Mingw64 took a more direct approach, but I don't know/remember exactly what that is. The original Mingw team considers this approach legally dubious, so to protect themselves and their users from the risk of multi-million dollar lawsuits from Microsoft, they refused his work. And so the Mingw64 fork was born. Last I checked, it was a source code only project.

DrSuperGood

Working on a revised version of the patch, hopefully it will not conflict.

According to MSDN there is no need to differentiate path separators ('\' and '/') since Windows NT. As such it should be possible to convert convoluted macros like...

#ifdef _WIN32
strcat(buffer, "\\");
#else
strcat(buffer, "/");
#endif

Into simpler...

strcat(buffer, "/");

Tests show that the path is correctly evaluated on Windows.

I am also looking into fixing the potential buffer overflow problem while I am at it.

Ters

Quote from: DrSuperGood on August 19, 2015, 05:28:55 AM
According to MSDN there is no need to differentiate path separators ('\' and '/') since Windows NT. As such it should be possible to convert convoluted macros like...

I've been trying to spread this information for years. It think Windows 9x also accepted normal slashes, although I could at the earliest have discovered this in 1998, so I can't be sure. That I first discovered it in XP seems a bit late, though. Maybe the higher APIs supported both, but not the 9x kernel. (DosBox supports forward slashes, but I don't know if that is staying true to MS-DOS or not. I have plans of installing proper MS-DOS on a virtual machine, but I can't find the floppies and they would likely be rotten now anyway.)

It's a one way thing, though, so the trick of not having to toggle path separators only works when constructing paths, not when parsing them. Not that Simutrans does a lot of path parsing. (Interestingly, it seems Microsoft have actually put a platform specific path separator constant into .NET, despite never having shown any interest in porting .NET to other OSes.)

Combuijs

Quote(Interestingly, it seems Microsoft have actually put a platform specific path separator constant into .NET, despite never having shown any interest in porting .NET to other OSes.)

Yup: Path.DirectorySeparatorChar
Bob Marley: No woman, no cry

Programmer: No user, no bugs



DrSuperGood

As I mentioned the correct approach to file paths is the Java one where a class is used to represent a path. You can avoid dealing with the platform specific characters when you need to traverse file systems as things like folder separator etc are all done by the implementation of that class (which is platform dependent, in a single place so easy to maintain and port).

For example such a class would use '\' on Windows, '/' on Linux/Mac/Whatever and wchar on Vista+ (MSCV build) and normal char on all others (I believe Linux and things encode characters with multi-byte but that is still known as *char). Speed does not really matter since processing the paths is trivial compared with the I/O required.

You would call methods and such to traverse folders and get files, instead of having to specify the folder separator at each stage. Things like the "boost" API seem to come with such a class already so it is not uncommon in software.

Ters

Quote from: DrSuperGood on August 19, 2015, 04:54:31 PM
(I believe Linux and things encode characters with multi-byte but that is still known as *char).

Windows is also multi-byte in its legacy char-based API in some parts of the world, and hence should be treated as such always. But it is also important to know that this multiness persists even with wchar, since Windows adopted Unicode when 16 bits was thought to be enough. This is also the case in Java. Only on Linux and the like, which has 32-bit wchar, is there a character datatype which can fit every Unicode characters, but there wchar is rarely used. (It would waste over 50 % memory on average.) Even then, there are some combining symbols, so a single (w)char can never be trusted to represent what readers might consider a character.

So simple a concept as written text, and yet so technically complex. And yet this is nothing compared to how difficult times and dates are. Fortunately, Simutrans doesn't have to deal with that in full.

DrSuperGood

QuoteWindows is also multi-byte in its legacy char-based API in some parts of the world, and hence should be treated as such always. But it is also important to know that this multiness persists even with wchar, since Windows adopted Unicode when 16 bits was thought to be enough. This is also the case in Java. Only on Linux and the like, which has 32-bit wchar, is there a character datatype which can fit every Unicode characters, but there wchar is rarely used. (It would waste over 50 % memory on average.) Even then, there are some combining symbols, so a single (w)char can never be trusted to represent what readers might consider a character.
Java abstracts dealing with Strings to the String class so does not really have the problems one runs into here. You can concatenate strings of any length and not run into problems as all the memory allocation is done for you and done correctly (if there are no bugs).

The problem with multi-byte characters in this case is how to allocate the space for them and what is the meaning of a character limit. A string of 100 actual characters could use 200 characters (or more if they are multi-char) of storage. Equally well if you count the storage used 100 characters (some kind of limit) might only be 50 actual characters long (less than the limit). Is the MS path limit in actual characters or in character storage space?

Ters

Quote from: DrSuperGood on August 20, 2015, 04:24:59 AM
Java abstracts dealing with Strings to the String class so does not really have the problems one runs into here.

No, not for concatenating, but that wouldn't really be the problem here either as strlen() and all length parameters operates on char elements, not typographical symbols. However, the indexes used on Java's String class are actually character elements, so you might end up splitting a character in two.

Quote from: DrSuperGood on August 20, 2015, 04:24:59 AM
Is the MS path limit in actual characters or in character storage space?

It must be character storage space, or WIN32_FIND_DATA would be broken. (Another note: The limit is in the Win32 API, carried over from Win16 I guess, for compatibility reasons. Windows NT itself has a much higher limit, which ultimately is how it's possible to create files whos path is longer than MAX_PATH. Good luck deleting those just using Explorer.)

DrSuperGood

QuoteIt must be character storage space, or WIN32_FIND_DATA would be broken. (Another note: The limit is in the Win32 API, carried over from Win16 I guess, for compatibility reasons. Windows NT itself has a much higher limit, which ultimately is how it's possible to create files whos path is longer than MAX_PATH. Good luck deleting those just using Explorer.)
The limit has apparently been shrinking over time.

Would there be any problem with using the C++ std::string (and possibly std::wstring for MSVC Vista+) for dealing with paths instead of C style char pointers? I understand that these have memory allocation overhead however I would not imagine that being a problem for file paths (as they are used so seldom).

Ters

Quote from: DrSuperGood on August 20, 2015, 03:45:17 PM
Would there be any problem with using the C++ std::string (and possibly std::wstring for MSVC Vista+) for dealing with paths instead of C style char pointers? I understand that these have memory allocation overhead however I would not imagine that being a problem for file paths (as they are used so seldom).

Not so much for Windows, since filling the std::string will still require allocating a char array (or std::vector<char>) first anyway. The other platforms will be much more straight forward. I don't think it will be easy to use std::wstring on Windows (not sure why only Vista+, there has been wchar since NT 3.51) and std::string otherwise, since the paths are passed on to other parts of Simutrans. If we were to standardize on Unicode in every part of Simutrans, we should stick to UTF-8 and char.

DrSuperGood

QuoteI don't think it will be easy to use std::wstring on Windows (not sure why only Vista+, there has been wchar since NT 3.51)
One could roll out to all parts of simutrans the path type which is then platform dependent. The reason Vista+ use wstring for paths is because their I/O operations target wstring only. In fact many of them lack normal string variants (hence the mess in my patch to translate from wchar to char).

Since both string and wstring share the same methods it becomes quite straight forward to change the type since the underlying operations should not need changing. Only the path constants need changing (from char to wchar) and those are currently badly declared anyway and in worst case could be (should be?) adapted appropriately.

Anyway you might find this quote interesting and stupid...
QuoteMulti-byte strings are not supported in Windows Store apps.

Ters

Quote from: DrSuperGood on August 20, 2015, 06:20:14 PM
One could roll out to all parts of simutrans the path type which is then platform dependent. The reason Vista+ use wstring for paths is because their I/O operations target wstring only. In fact many of them lack normal string variants (hence the mess in my patch to translate from wchar to char).
But since we target less than Vista, we can't use SHGetKnownFolderPath, hence nothing is wchar_t only. And I'm not sure Simutrans does enough with paths to really need any specific class for it. It's just concatenation at the moment, isn't it?

Quote from: DrSuperGood on August 20, 2015, 06:20:14 PM
Since both string and wstring share the same methods it becomes quite straight forward to change the type since the underlying operations should not need changing. Only the path constants need changing (from char to wchar) and those are currently badly declared anyway and in worst case could be (should be?) adapted appropriately.

But if you have:

std::string str;
gzFile gzf = gzopen(str.c_str(), "rb");

you can't just change the type of str into std::wstring. The trouble with wchar_t is that, besides Windows' wide API, "no" API supports it. You can't even open an std::wfstream using an std::wstring!

DrSuperGood

QuoteBut since we target less than Vista, we can't use SHGetKnownFolderPath, hence nothing is wchar_t only. And I'm not sure Simutrans does enough with paths to really need any specific class for it. It's just concatenation at the moment, isn't it?
Currently XP is targeted, but in the future that might be changed. By declaring a path type in say "simpath.h" and various path related methods and constants one could generalize the mechanics of path creation such that making a move would be as trivial as changing some declarations and definitions with macros.

Currently the reason it has to be looked at is some of the concatenation, specifically for Linux and Mac, is unsafe. It could potentially result in a buffer overflow. Something like std::string would make fixing this trivial otherwise some less readable C style code is needed which assures sufficient space.

There is also a small problem with platform style folder names. Currently Simutrans uses a lot of lower case "Unix" convention folder names (eg "save"). Mac and Windows should be using capital first letter folder name (eg "Save") to conform with general folder structures. This is already happening with the root folder ("simutrans" on Linux, "Simutrans" on Windows and Mac) but not its sub folders. This is where something like simpath.h could define constant adapters to automatically case the folders appropriately. The change will be backwards compatible on Windows as folders are not case sensitive (existing folders still accessed) however I am unsure about Mac as that was originally Unix based.

Ters

Quote from: DrSuperGood on August 20, 2015, 08:10:43 PM
Currently XP is targeted, but in the future that might be changed. By declaring a path type in say "simpath.h" and various path related methods and constants one could generalize the mechanics of path creation such that making a move would be as trivial as changing some declarations and definitions with macros.

But there is no point in spending lots of efforts on preparing for a possible change still many years into the future. Considering when we dropped support for Windows 9x, and the continued popularity of Windows XP, it will likely be ten years until Windows XP support is dropped. And there is currently nothing to gain from targeting any newer Windows versions explicitly. I just proposed we do a trivial fix to a small problem, not redesign a lot of things. Not that those things are not a good idea, but do them separately. That way, any problems with the bigger rewrite won't hold up the small fix.

DrSuperGood

Here is a revised patch that incorporates the Haiku patch as well as the approaches for windows. The Vista and above approach will (should) only be used when being built with MSVC.

Ters

Quote from: DrSuperGood on August 23, 2015, 05:05:04 AM
Here is a revised patch that incorporates the Haiku patch as well as the approaches for windows. The Vista and above approach will (should) only be used when being built with MSVC.

Nice work. My only small concern is that selecting different code should, in general, be an explicit configuration in the build, so that one has to explicitly decide to use it. That way, one knows that one has done something different from everyone else, which can avoid lots of wasted time wondering why something works on your computer and not on everybody else's, or vice versa, should there be a breaking difference in the two code paths.

DrSuperGood

It is an explicit difference. You need to order it to be built with Windows Vista or higher for that path to be taken (which it defaults to in modern MSVC). If you set MSVC to build targeting XP it will use the same code path as currently used by MinGW. This should only make a difference for people making builds with MSVC, in which case they should know which Windows platform they want to target.

I tested both code paths this time (unlike a patch for the POSIX server I submitted a while ago). They both appear to work, with a build finding the appropriate folder.

Ters

Quote from: DrSuperGood on August 23, 2015, 03:31:21 PM
You need to order it to be built with Windows Vista or higher for that path to be taken (which it defaults to in modern MSVC).

Exactly. That's my worry. It just happens for no directly obvious reason. Just because you happen to use MSVC2015, and not MSVC2010, mingw or whatever. Unless Simutrans explicitly states its target Windows version. I suddenly remembered that the Makefile used by Mingw defines _WINVER=0x501 and _WIN32_IE=0x0500 (had to look up exact definitions, though). Is the same done in the MSVC project? It should be to reduce the risk of nasty surprises when code written in MSVC is compiled with Mingw for the first time. If it is, my concern lessens, because people are not at the mercy of this hidden default. (Especially people coming from Mingw are used to having to opt in to newer APIs than what plain Windows 95 had, if they ever cared for new stuff at all. Not that there is much to opt in for. Maybe older Windows SDKs were like this as well.)

DrSuperGood

QuoteIs the same done in the MSVC project?
Yes. You can declare them as compiler definitions under project settings or you can let it automatically declare them (in which case the latest is chosen, which for MSVC 2015 is Windows 10). They are used by Windows.h to choose the available declarations for the compiler to use (so it will throw a syntax error at compile time if the code is incompatible).

Trying to run a build targeting a higher system on an older system will produce a run time error.

MinGW must be targeting the following...

#define _WIN32_WINNT_WINXP                  0x0501 // Windows XP


For developers using MSVC to produce their code I would recommend running two tests.
_WIN32_WINNT = _WIN32_WINNT_WINXP
_WIN32_WINNT = {their OS version if higher than above}
This would make sure code they submit works correctly when built with MinGW and latter OS. If you write code targeting an OS beyond what you have (eg you have Vista and write changes needing Windows 8 or 10) you should ask for someone to test the patch (or at least make it clear the changes are untested).

For example currently I build targeting Windows 10. However I also test the patches against XP for compatibility reasons.

Usability is not that much of a concern as anyone interested in MSVC development should already be aware of this. End users will never run into this problem as they will take the ready-made builds.

Ters

Quote from: DrSuperGood on August 23, 2015, 08:26:15 PM
Yes. You can declare them as compiler definitions under project settings or you can let it automatically declare them (in which case the latest is chosen, which for MSVC 2015 is Windows 10). They are used by Windows.h to choose the available declarations for the compiler to use (so it will throw a syntax error at compile time if the code is incompatible).

Seems like you are answering the question "can it be done", not the question "is it done".

Quote from: DrSuperGood on August 23, 2015, 08:26:15 PM
For developers using MSVC to produce their code I would recommend running two tests.
_WIN32_WINNT = _WIN32_WINNT_WINXP
_WIN32_WINNT = {their OS version if higher than above}

I would recommend only doing the first one. The latter is of no use to Simutrans, as the number of people actually running self-compiled Simutrans on Windows that bothers to change the target OS from the official Windows XP can probably be counted on one hand.

The version macros control the minimum supported Windows version, by allowing you to use APIs available from that version onwards, not the maximum version. Since Simutrans is meant to run on Windows XP, no higher API than that can be used, hence turning the values higher are pointless (except for something I'll get back to). It's not like code with _WIN32_WINNT = 0x501 won't work in the foreseeable future. The biggest problem might eventually be using the Win32 API at all (and the real C++). OpenFile has been deprecated since 1995, yet is ironically listen on MSDN as supported from Windows XP and up. The binary compatibility of the Windows API is so crucial to Microsoft's success, that they won't dare do anything to it. Deprecation means "there is a better way now", not "this will stop working soon".

Compatibility with the Windows SDK is another matter. It is written on MSDN that MSVC 2015 no longer supports targeting Windows versions below XP. It doesn't go into details on what the consequences are, though. Maybe they just don't bother keeping all the ifdefs, so that setting WINVER to 0x400 doesn't actually disable APIs introduced in Windows 98. They do make it clear that errors is a possible outcome, but the wording indicates that they haven't deliberately done something for it to break. As it is, Simutrans' minimum supported Windows version is the same as MSVC's, so there is no problem for the time being. It is, however, worth noting that mingw's maximum supported version is now less than or equal to MSVC's minimum supported version.

DrSuperGood

Honestly I do not see what the fuss is about. The code is there for people who want to build targeting it higher OS versions. It should not affect the minimum requirements at all.

Ters

Quote from: DrSuperGood on August 24, 2015, 02:10:49 PM
Honestly I do not see what the fuss is about. The code is there for people who want to build targeting it higher OS versions. It should not affect the minimum requirements at all.

I understand that, and at first pointed out that so far, these people are very very few, perhaps just you. Then there was my concern that people might end up targeting a higher OS version that intended, if the MSVC project did not explicitly set the target version. And my recommendation to just stick to that if they don't have any personal wishes for something else, because it makes no difference (for the time being).

If people want to run a non-standard Simutrans, that's fine. I do it myself, although that's due to a bug in the latest mingw, and I introduced a hack into simsys_w.cc rather than downgrade again. (Downgrading is the recommended alternative. Unless they've fixed it again, I haven't checked lately.) But those who do so, should know that they do so, and check if any bugs they observe is because of that before reporting them.

Edit:
Just to try and be a bit more clear: There are two, maybe three, separate issues here. The first is just a curiosity in why it was considered worthwhile to write code specifically for Vista+ that is functionally identical to the code for Windows 2000/XP+. I don't understand, and I like to understand. The default target Windows version in the build doesn't really have anything to do with the patch, it's just something I started think about when I saw a version switch. Maybe I should have started another discussion for that.

Ters

I've tested that the V2 patch compiles and works on the (almost) latest mingw (mingwrt 3.21). Running on Windows 10.

prissi

One could still use SHGetFolderPath, which is still recommended on "Managing the file System" on the MS documentation. It will use the newer one  automatically, and will use the old function when needed. That woudl avoid all those defines. (Actualy it is supported since Windows95 ... )

Ters

Quote from: prissi on September 02, 2015, 12:04:58 PM
One could still use SHGetFolderPath, which is still recommended on "Managing the file System" on the MS documentation. It will use the newer one  automatically, and will use the old function when needed. That woudl avoid all those defines. (Actualy it is supported since Windows95 ... )

Isn't that what the patch uses? Except it explicitly calls the ANSI version, but that is something to fix later. (And I can't find it in my Win32 reference from 1996, but anything before Windows XP is irrelevant.) A and W are not old and new.

DrSuperGood

QuoteOne could still use SHGetFolderPath, which is still recommended on "Managing the file System" on the MS documentation. It will use the newer one  automatically, and will use the old function when needed. That woudl avoid all those defines. (Actualy it is supported since Windows95 ... )
If you go to the documentation for the function it then says that it is deprecated and new applications should rather use the version I gave for Vista. Yes SHGetFolderPath still works and probably will keep working for a while to come but since I was making the change I thought I would add the more up to date approach for it now.

Quote from: MSDNNote  As of Windows Vista, this function is merely a wrapper for SHGetKnownFolderPath. The CSIDL value is translated to its associated KNOWNFOLDERID and then SHGetKnownFolderPath is called. New applications should use the known folder system rather than the older CSIDL system, which is supported only for backward compatibility.
I do admit the macros are a pain though. I wanted to avoid hard-coded constants for readability since the macro constants are much more human friendly.

Ters

I would have gone for the new function myself, if we could get away with only using that. The question is whether the project wants to maintain and support the code using SHGetKnownFolderPath, when there is little indication that more than five persons will ever use that code. Or maybe the problem is the other way around, the SHGetKnownFolderPath code is maintained because the Windows developers use it (since MSVS2015 enables it by default), however the thousands of players out there use the code that isn't maintained.

And since the nightlies are made with MSVS apparently, they won't work on anything less than Windows 10 if MSVS2015 is used! It probably uses a somewhat older MSVS, so the actual Windows version required will be less, for now.