Skip to content
This repository has been archived by the owner on Jan 13, 2022. It is now read-only.

Fixed static RakNet build for vs2017 using cmake #124

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

meruiden
Copy link

…akelists.txt leading to skip samples even if enabled

…akelists.txt leading to skip samples even if enabled
@@ -53,6 +53,6 @@ add_subdirectory(Lib)

set(RAKNET_COMMON_LIBS RakNetLibStatic)

if( RAKNET_GENERATE_SAMPLES )
if( RAKNET_ENABLE_SAMPLES )

Choose a reason for hiding this comment

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

This is a duplicate of #51 .

@@ -24,7 +24,7 @@ ENDIF(WIN32 AND NOT UNIX)


# Options
option( RAKNET_ENABLE_SAMPLES "Generate RakNet sample projects if true." TRUE )
option( RAKNET_ENABLE_SAMPLES "Generate RakNet sample projects if true." FALSE )

Choose a reason for hiding this comment

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

I don't think that changing the default and disable the generation of the sample projects would be good. Hence not going to incorporate this in SLikeNet. Feel free to let me know your rational behind the change so we can reconsider our decision.

@@ -20,7 +20,7 @@ IF(WIN32 AND NOT UNIX)
IF( MSVC10 OR MSVC11 OR MSVC12 )
set_target_properties(RakNetLibStatic PROPERTIES STATIC_LIBRARY_FLAGS "/NODEFAULTLIB:\"LIBCD.lib LIBCMTD.lib MSVCRT.lib\"" )
ELSE()
set_target_properties(RakNetLibStatic PROPERTIES STATIC_LIBRARY_FLAGS "/NODEFAULTLIB:"LIBCD.lib LIBCMTD.lib MSVCRT.lib"" )
set_target_properties(RakNetLibStatic PROPERTIES STATIC_LIBRARY_FLAGS "/NODEFAULTLIB:\"LIBCD.lib LIBCMTD.lib MSVCRT.lib\"" )

Choose a reason for hiding this comment

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

This looks kind of flawed. Please note that the section you modified was used to generate VS2005/2008 project files for which, as far as my understanding goes, the "-usage was required.

The proper "fix" to ensure that VS2015/VS2017 generated project files use the correct settings would be to either add MSVC14 to the list in ln 20 (see #64 for example) and/or drop the entire if/else clause here and solely go with the "-style (i.e. drop support for VS 2005/2008 which we did in SLikeNet now - the change will be available shortly in the SLikeNet Git and SVN repositories - https://github.com/SLikeSoft/SLikeNet ).

@Luke1410
Copy link

Luke1410 commented Apr 24, 2018

Thanks for the contribution @meruiden . I just reviewed your changes and decided not to incorporate them in our fork of RakNet (SLikeNet: https://github.com/SLikeSoft/SLikeNet). See the code comments on the code explaining the rational. If you think the decision should be reconsidered, just leave me a note.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants