Multiple flags are correctly read into VS projects, added support for preprocessor definitions in addons #214
Conversation
|
thanks! looks like a helpful change |
|
the way overwriting works is by using += if you don't want to overwrite and just = if you want to overwrite, the syntax is based on makefiles syntax and allows to for example overwrite the defaults that are parsed from the file system which is quite useful for certain cases. or i'm understanding this wrong? Also aren't preprocessir definitions the same as the already present ADDON_DEFINES? |
|
Yes, this is how it should work. But the project generator for vs overwrites the setting even, when using +=. So I did the fix, that it works how it should. Yes, you are right. I did not get it that the ADDON_DEFINES should be used for the preprocessor. The had a small bug as well. Fixed it and removed the separate preprocessor definitions |
| if(variable == "ADDON_CFLAGS"){ | ||
| addReplaceStringVector(cflags,value,"",addToValue); | ||
| } | ||
|
|
||
| if (variable == "ADDON_PREPROCESSOR_DEFINITIONS") { |
There was a problem hiding this comment.
this reference to ADDON_PREPROCESSOR_DEFINITIONS should go as well
There was a problem hiding this comment.
so, then adding ADDON_PREPROCESSOR_DEFINITIONS is useful?
Since I changed
additionalOptions = items[i].node().child("ClCompile").child("AdditionalOptions");
for
additionalOptions = items[i].node().child("ClCompile").child("PreprocessorDefinitions");
Should we leave it like this or should we use the seperate preprocessor flag?
There was a problem hiding this comment.
no sorry, what i meant is it should go away
There was a problem hiding this comment.
already is in the latest commit
| @@ -310,7 +315,7 @@ void ofAddon::parseVariableValue(string variable, string value, bool addToValue, | |||
| } | |||
|
|
|||
| if(variable == "ADDON_DATA"){ | |||
| addReplaceStringVector(data,value,"",addToValue); | |||
| addReplaceStringVector(data,value,addonRelPath,addToValue); | |||
There was a problem hiding this comment.
what is this? doesn't seem related to any of the problems explained in the PR no?
There was a problem hiding this comment.
No, sorry I don't really know why that happened.
|
not sure what's going on but all the changes you've marked as resolved aren't really there. perhaps you forgot to push? |
Did I oversee something (do I need to make a new pull request, or does this work automatically)? The latest commit should resolve the addReplaceStringVector() thing: The one before the preprocessor issue You have any idea? |
|
those commits are already in, you don't need to create a new PR i think you are just missing some bits that are still referring to addon preprocessor define. look carefully at my annotations and the line numbers |
|
sorry arturo, I was a little confused about the codeline. should be fixed now. |
| @@ -45,6 +45,7 @@ class ofAddon { | |||
| std::vector < std::string > frameworks; // osx only | |||
| std::vector < std::string > data; | |||
| std::vector < std::string > defines; | |||
| std::vector < std::string > preprocessorDefinitions; // vs only | |||
There was a problem hiding this comment.
this one is still missing, it should go away to. there should be no changes in ofAddon.h only on visualStudioProject
Before the project generator only saved the last flag of each type and overwrote the previous ones. This could be fixed by overwriting the first child of the note but not the not itself
from
additionalOptions.first_child().set_valueto
additionalOptions.set_value((std::string(additionalOptions.value()) + " " + cflag).c_str());Addionally I added the support for preprocessor definitions in addons in VS projects.