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

Fix VS2017 build #980

Merged
merged 4 commits into from Feb 23, 2018
Merged

Fix VS2017 build #980

merged 4 commits into from Feb 23, 2018

Conversation

msc94
Copy link

@msc94 msc94 commented Feb 22, 2018

Hello, this fixes the build on the newest version of VS2017.
I have also documented a few of the problems I have encountered for others.

Also, I enabled multi-processor compilation on MSVC, that makes a significant difference in build times.

This is my first pull request on github, so be gentle. :P

Greetings.

copying.md Outdated
@@ -108,6 +108,7 @@ _the openage authors_ are:
| Julian Guillotel | arialwhite | julian dawt gullotel à gmail dawt com |
| Arne Sellmann | PythonicChemist | arne dawt sellmann à gmx dawt de |
| Rafael X. Morales Georgi | chocoladisco | chocoladisco á gmail dawt com |
| Marcel Schneider | schnema123 | marcelschneider5 á outlook dawt de |
Copy link
Member

Choose a reason for hiding this comment

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

Replace á with à (similar to line 109 above).
You can optionally fix the last-but-one line also.

@tusharpm tusharpm added os: windows Windows-specific issue documentation Involves the project documentation bugfix Restores intended behavior labels Feb 22, 2018
Copy link
Member

@tusharpm tusharpm left a comment

Choose a reason for hiding this comment

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

Few suggestions.

@@ -30,6 +31,8 @@
If you want, you can still use [the prebuilt version](https://www.qt.io/download-open-source/) instead.
If you do so, include `-DCMAKE_PREFIX_PATH=<QT5 directory>` in the cmake configure command.

_Note:_ If you are planing to build the 64-bit Version of openage, you are going to need 64-bit libraries. Add command line option `--triplet x64-windows` to the above command or add the enviroment variable `VCPKG_DEFAULT_TRIPLET=x64-windows` to build x64 libraries. [See here](https://github.com/Microsoft/vcpkg/issues/1254)
Copy link
Member

Choose a reason for hiding this comment

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

nit:
s/planing/planning/
s/Version/version/
s/enviroment/environment/

Copy link
Author

Choose a reason for hiding this comment

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

Oops, my bad :)

## Running openage (in devmode)
While this is straightforward on other platforms, there is still stuff to do to run openage on Windows:
- Install the [DejaVu Book Font](https://dejavu-fonts.github.io/Download.html).
- Download and extract the latest `dejavu-fonts-ttf` tarball/zip file.
- Copy `ttf/DejaVuSerif*.ttf` font files to `%WINDIR%/Fonts`.
- Set the `FONTCONFIG_PATH` environment variable to `<vcpkg directory>\installed\<relevant config>\tools\fontconfig\fonts\conf.d`.
- Set the `FONTCONFIG_PATH` environment variable to `<vcpkg directory>\installed\<relevant config>\tools\fontconfig\fonts\`.
- Copy `fontconfig/57-dejavu-serif.conf` to `%FONTCONFIG_PATH%`.
Copy link
Member

Choose a reason for hiding this comment

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

Nice catch. My bad. :P
Please update this line also: [...] %FONTCONFIG_PATH%/conf.d

@@ -21,6 +21,7 @@
#include <unistd.h>
#endif

#include "../error/handlers.h"
Copy link
Member

Choose a reason for hiding this comment

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

The associated header file comes first.
Also, #include "handlers.h".

@msc94
Copy link
Author

msc94 commented Feb 22, 2018

Ok guys, I had a running build (I have already broken it again.. but I was seeing something that looked like AoE2 on the screen like an hour ago, I swear!). I think I have documented every step that was needed to get there... Feel free to make suggestions.

@tusharpm
Copy link
Member

Tested it locally. Builds and runs properly for both 32bit and 64bit. Nice work!
@schnema123, whenever you're ready, please group and squash the commits by components (try git rebase -i master). Also, while editing the commit messages, prefix the component they modify (doc, buildsystem, util). Some commits like the one modifying copying.md might not have prefixes, that's fine.
After this, you'll have to git push -f to update the branch on GitHub.
You can remove the [WIP] tag from the PR when you are ready for merging.

@msc94 msc94 changed the title [WIP] Fix VS2017 build Fix VS2017 build Feb 23, 2018
@msc94
Copy link
Author

msc94 commented Feb 23, 2018

I have squashed and grouped the commits together. Greetings.

Copy link
Member

@tusharpm tusharpm left a comment

Choose a reason for hiding this comment

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

Cool! :shipit:

@tusharpm tusharpm merged commit 8481cfa into SFTtech:master Feb 23, 2018
@tusharpm
Copy link
Member

Thank you, @schnema123! Great work! 👍

@simonsan simonsan added this to Done in documentation Jun 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Restores intended behavior documentation Involves the project documentation os: windows Windows-specific issue
Projects
documentation
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants