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

CREATE_PROJECT: Fix missing MSVC x64 defines #1725

Merged
merged 3 commits into from Jul 1, 2019

Conversation

Projects
None yet
3 participants
@SupSuper
Copy link
Contributor

commented Jul 1, 2019

This addresses the issues brought up in PR #1722 in a more robust way;

To disable nasm from x64, the whole define list was rewritten from scratch using getFeatureDefines and getEngineDefines. This ended up removing important defines added later in the setup phase, which presumably someone noticed, as they were poorly re-added.
Instead, I opted to just find and remove the nasm define for x64. Better one little hack than a whole pile of them. :)

This also adds the new libcurl Win32 dependencies and cleans up the rest so they're only generated for the appropriate targets. More detailed explanations in the commit messages.

SupSuper added some commits Jul 1, 2019

CREATE_PROJECT: Fix defines for x64 MSVC project
Rewriting the define list from scratch to disable nasm lost
a lot of important defines set up in the setup phase.
Instead, let's just remove the nasm define and preserve the rest.
CREATE_PROJECT: Use stricter checks for Win32 dependencies
Instead of checking the project type, check the define.
winmm and winsparkle are only required on Windows.

@SupSuper SupSuper force-pushed the SupSuper:x64-defines branch from c06cb4a to 9036a1b Jul 1, 2019

@lephilousophe

This comment has been minimized.

Copy link
Member

commented Jul 1, 2019

I failed to compile in Release mode but I suspect more my VM than the code.
I got a titanic.lib which is over 850MB and got permission denied when linking it in the last stage.
Anyway, I think it will fix the bugs I got. It had in Debug.

@bluegr

This comment has been minimized.

Copy link
Member

commented Jul 1, 2019

Well done with the pull request @SupSuper, and thanks for testing it @lephilousophe. This is good to be merged now

@bluegr bluegr merged commit 69cb2ef into scummvm:master Jul 1, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.