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

Add mingw basic support #2196

Merged

Conversation

ericoporto
Copy link
Member

@ericoporto ericoporto commented Oct 22, 2023

This is to be able to build on Windows by using MinGW instead of MSVC. Of course, we will forever use MSVC because of the Editor (and C++/CLR), but still, wanted to investigate possibilities - just to safeguard.

Here is a 64-bit build through MinGW: ags.zip
Known issue: SDL2 is statically linked and SDL2 statically linked has some issue with fullscreen on Windows I couldn't yet figure it out (this happens in MSVC too, so it's not mingw specific)

There is no rush here, so this can be reviewed with calm.

@ericoporto ericoporto marked this pull request as draft October 22, 2023 01:16
#define max(x,y) (((x) > (y)) ? (x) : (y))
#endif
#define math_min(x,y) (((x) < (y)) ? (x) : (y))
#define math_max(x,y) (((x) > (y)) ? (x) : (y))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to go opposite way and disable macros coming from windows.h. Not only having to rename macros annoying, but also windows.h macros will prevent using std::min/max if one wishes to switch to these.

First I'd double check why windows.h is included here, but likely it's because of allegro.h
Then there are two ways the window header may be dealt with:

  1. Undef-ing conflicting macros after the header.
  2. Defining #define NOMINMAX before the header - in which case windows.h will skip these macro definition.

For example see
https://github.com/adventuregamestudio/ags/blob/master/Common/platform/windows/windows.h

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed a few things were actually supposed to be built only in non-builtin plugins, so I adjusted that, but also used the NOMINMAX define there and used std::min and std::max instead.

@@ -103,7 +104,11 @@ class WindowsLibrary final : public BaseLibrary
{
if (!_library)
return nullptr;
#if AGS_PLATFORM_WINDOWS_MINGW
return reinterpret_cast<void *>(GetProcAddress(_library, fn_name.GetCStr()));
#else
Copy link
Contributor

@ivan-mogilko ivan-mogilko Oct 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this does not need a platform switch, as this may be a valid type cast regardless.
MSVC seem to be tolerant to missing headers and type casts sometimes, so it does not error on these.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the check and used the reinterpret_cast as the way to go there.

@ericoporto ericoporto force-pushed the add-mingw-basic-support branch from e6f8997 to e644cee Compare October 25, 2023 21:38
`__except` is not supported on mingw and will require a different code for exception handling. I found working code but it used an undocumented nt api, so this is better to research with calm.
this is to avoid manually copying mingw dlls, and ensures a portable binary the same as traditional acwin.exe
@ericoporto ericoporto marked this pull request as ready for review October 25, 2023 21:41
@ivan-mogilko ivan-mogilko merged commit 3517286 into adventuregamestudio:master Oct 27, 2023
20 checks passed
@ericoporto ericoporto deleted the add-mingw-basic-support branch October 27, 2023 00:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants