Moved #defines to build script #24

Closed
wants to merge 1 commit into from

2 participants

@devnought

Pulled out of example sources and added to premake script. Not sure if
GLFW_INCLUDE_COREARB is required for Linux, but it is not for Windows.

@devnought devnought Moved #defines to build script
Pulled out of example sources and added to premake script. Not sure if
GLFW_INCLUDE_COREARB is required for Linux, but it is not for Windows.
d896070
@memononen
Owner

Can you elaborate what's the thinking behind moving the defines to make files instead of adding platform specific defines to the examples? I think both are valid options, I'd just like to understand the rationale better.

I'd like keep the implementation defines in the files, since they are important to understand and are a bit hidden if put in the makefile, though that is a valid option too.

@devnought

From a Windows perspective, I wanted to pull out at a minimum GLFW_INCLUDE_GLCOREARB as that header doesn't exist on the platform. I thought it might be nice to pull out some of the other defines on top of that.

As a compromise, a simple #ifdef would solve the GLFW_INCLUDE_GLCOREARB issue if you wanted to keep more in the source files.

@devnought

Honestly I suppose there was no specific reason to do via preprocessor definitions. Call it habit more than anything else :)

@memononen
Owner

Ok :) I think it is better to move the define to the source files then. Is the arb header related to GLEW? maybe it could go to an #else in the glew define?

@devnought

I can't honestly say if it's related to GLEW.

It seems that the commit by @dougbinks addresses the arb header concern. GLEW is still required on windows and that is defined in the premake script for Linux. Can this be added for windows as well?

@memononen
Owner

Yes, GLEW should be added to windows premake script. The define should be pretty simple to do, do you have preferences how the library should be added? Could this work:
configuration { "windows" }
links { "glu32","opengl32", "gdi32", "winmm", "user32", "glfw3", "GLEW" }
defines { "NANOVG_GLEW" }

@devnought

That'll work just fine :) I can whip that up if you'd like.

@memononen
Owner

I updated the build settings as per above, can you test it and if it works, feel free to close the issue.

@devnought

Looks good. Thanks!

@devnought devnought closed this Feb 20, 2014
@devnought devnought deleted the unknown repository branch Feb 20, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment