Skip to content

resolve #14#47

Merged
gynt merged 3 commits intomainfrom
fix/resolve-type-conflicts-14
Mar 11, 2026
Merged

resolve #14#47
gynt merged 3 commits intomainfrom
fix/resolve-type-conflicts-14

Conversation

@gynt
Copy link
Contributor

@gynt gynt commented Mar 6, 2026

@gynt gynt requested a review from TheRedDaemon March 6, 2026 23:26
#include "framework.h"
#include "winnt.h"
#include "wtypes.h"
#include <windef.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

Any advantage over using the whole "Windows.h" header in the precomp (and here) and abandoning the "framework.h"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about the other way around and moving these three lines inside "framework.h"?

#include "winnt.h"
#include "wtypes.h"
#include <windef.h>

Windows is kind of the framework the game operates in anyways, so it makes sense to put those there. Then "common" is only for common types outside of the windows ones?

Copy link
Contributor

@TheRedDaemon TheRedDaemon Mar 10, 2026

Choose a reason for hiding this comment

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

Also a good idea, although the reason I push for the Windows.h header is due to the fact that at least the Mss32.h includes the whole header anyway, so I was wondering if we could just add it to the precomp and be done with it.

I have another idea, although a bit more complicated:

  • Check if we have other cases that import the Windows.h directly.
  • See if you could change all positions to not use it.
  • If the current setup becomes build able, I think it would work. The test would require to include the headers that currently use the full windows.h in the build. I would assume the compiler would complain if definitions are missing.
  • Since it is a dependency, at least Mss32 should work without include files created by us.

It could also be that the mmsystem requires the whole windows.h, I do not know at the moment. Important is that the header Mss32.h works.

Sorry for the sudden other tangent. I think your idea is fine, but for me the whole thing is a follow up of the question whether we need Windows.h completely or if reduced is enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I have put windows.h in pch.h!

@gynt gynt requested a review from TheRedDaemon March 10, 2026 12:22
@gynt gynt merged commit 4b6b0f2 into main Mar 11, 2026
1 check passed
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