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.

Sign up for GitHub

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

Jump to bottom

[cmake] Add windows resource file for application icon and metadata #384

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

[cmake] Add windows resource file for application icon and metadata #384

wants to merge 9 commits into from

Conversation

IAmNotHanni
Copy link
Member

@IAmNotHanni IAmNotHanni commented May 3, 2021 •
edited
Loading

Closes #382
Closes #420

metadata

TODO

  • Switch language to English
  • Do we really need resource.hpp?
  • Add Git SHA to application's metadata
  • Ignore resource file in CMake on Linux to fix build
  • Ask @IceflowRE for help

@IAmNotHanni IAmNotHanni added the cat:enhancement enhancement/requested feature/update of existing features label May 3, 2021
@IAmNotHanni IAmNotHanni self-assigned this May 3, 2021
IceflowRE
IceflowRE reviewed May 4, 2021
View reviewed changes
example/CMakeLists.txt Outdated Show resolved Hide resolved
IceflowRE
IceflowRE reviewed May 4, 2021
View reviewed changes
Copy link
Member

@IceflowRE IceflowRE left a comment

Choose a reason for hiding this comment

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

In general theres no need to configure the .rc file, as you just can include our meta.hpp with all the data and use it there.

@IAmNotHanni IAmNotHanni added the org:on hold on hold, until ... label May 16, 2021
IAmNotHanni
IAmNotHanni commented Jul 19, 2021
View reviewed changes
example/CMakeLists.txt Outdated Show resolved Hide resolved
@IAmNotHanni IAmNotHanni marked this pull request as ready for review July 19, 2021 22:08
@IAmNotHanni IAmNotHanni removed the org:on hold on hold, until ... label Jul 19, 2021
@IAmNotHanni
Copy link
Member Author

Alright I'm ready for review 👍

@IAmNotHanni
Copy link
Member Author

I will include the following fix in meta.hpp.in

#ifndef NDEBUG
constexpr const char *BUILD_TYPE = "Debug";
#else
constexpr const char *BUILD_TYPE = "Release";
#endif

This will also fix #420

@IceflowRE
Copy link
Member

Will review the rest later.

@IAmNotHanni
Copy link
Member Author

I rebased onto master.

@IAmNotHanni
Copy link
Member Author

There's a problem with this code. The binary does not contain all the information I want to be in there:

grafik

IceflowRE
IceflowRE requested changes Aug 2, 2021
View reviewed changes
example/CMakeLists.txt
if(MSVC)
add_executable(inexor-vulkan-renderer-example
main.cpp
${CMAKE_BINARY_DIR}/include/inexor/vulkan-renderer/win_resource_file.rc
Copy link
Member

Choose a reason for hiding this comment

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

Source files shouldn't be in the include folder.

@IceflowRE
Copy link
Member

There's a problem with this code. The binary does not contain all the information I want to be in there:

grafik

Will take a look today.

@IAmNotHanni IAmNotHanni added the prio:low This has low priority. label Sep 27, 2021
@IAmNotHanni IAmNotHanni marked this pull request as draft November 28, 2021 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IceflowRE IceflowRE IceflowRE requested changes

@yeetari yeetari Awaiting requested review from yeetari

Requested changes must be addressed to merge this pull request.

Assignees

@IAmNotHanni IAmNotHanni

Labels
cat:enhancement enhancement/requested feature/update of existing features prio:low This has low priority.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Build type in engine metadata is empty on Windows Add Windows resource file for executable icon and version
2 participants
@IAmNotHanni @IceflowRE