TommyKaneko / Sketchup-API-C-Wrapper

A complete set of C++ Wrapper classes for SketchUp C API objects
MIT License
28 stars 8 forks source link

No Visual Studio / XCode project/solutions? #30

Open thomthom opened 6 years ago

thomthom commented 6 years ago

Is there no projects set up for this wrapper?

I was going to look into setting up a PR for GoogleTest (https://github.com/TommyKaneko/Sketchup-API-C-Wrapper/commit/ffc704c243d8bba272c4064b46276227f5a27c0b#r27718155)

For that to happen there needs to be either a static or dynamic lib for which the GoogleTest project can link with. And I had also assumed that's how it would have been consumed.,

How are you currently using it? Adding all the files into the same projects you are using it with?

TommyKaneko commented 6 years ago

My project that I use it for is a private project.

I have an XCode project for the Cpp Wrapper, which outputs a dylib. For my private project, I have the CppWrapper as a subproject, so updates on CppWrapper can be rebuilt at the same time as my private project.

For the cpp files that I create, I include the header files from the CppWrapper like this: #include <SUAPI-CppWrapper/Geometry.hpp>

Shall I share the XCode project for this?

thomthom commented 6 years ago

To easily consume this wrapper library it would be nice to have projects for Visual Studio and Xcode that builds static libs. That way the user of the link can just add references to these static libs. (Similar to how you consume GoogleTest and other projects.)

Another snag I ran into when trying to set up a GoogleTest project, I needed to move the "SUAPI-CppWrapper" under a sub-directory to avoid conflating includes. My current dev branch I moved it to src/SUAPI-CppWrapper.

That's a rather common setup since a project typically needs tests as well as the source along side.

+ Project Root
  + src/
  + tests/
  + resources/

Ultimately it's up to you - but I'll try to set up an example of how it can be provided as a static lib with tests.

thomthom commented 6 years ago

I'm seeing some errors and warnings when building with VS2017:

image

thomthom commented 6 years ago

The #ifdef isn't working right:

image

thomthom commented 6 years ago

std::transform is missing the include header for it. (Another reason to keep the wrapper in a separate static lib so include paths doesn't get mixed up with other projects.)

image

thomthom commented 6 years ago

Might be worth enabling treating warnings as errors.

image

What is that piece of code doing? Is it capping rhs into the range of -2PI to +2PI? (If so, modulo might be an alternative.)

TommyKaneko commented 6 years ago

Thanks for your big review of the code today, ThomThom. I will get on all those issues over the next week!

thomthom commented 6 years ago

I think this should do the same - while avoiding the warnings.

image

My current dev branch is here: https://github.com/thomthom/Sketchup-API-C-Wrapper/tree/dev-vs-project

TommyKaneko commented 6 years ago

FYI - I have uploaded my XCode environment here: https://github.com/TommyKaneko/Sketchup-API-C-Wrapper/tree/xcode_project

thomthom commented 4 years ago

I've been looking into CMake recently to simplify cross platform development. Maybe a CMake project file could be useful for this project? VSCode and Visual Studio has nice integration with CMake.

TommyKaneko commented 4 years ago

I think you have already implemented this no: https://github.com/TommyKaneko/Sketchup-API-C-Wrapper/tree/master/cmake

thomthom commented 4 years ago

😱😵!!

I really have no recollection of doing that!

But I see there's a few TODO notes. And I've been reading up a book on CMake, diving into best practices and learning a lot more about it. So I'll probably come back and revisit this.

With GitHub having CI now we can set up test builds to test that the various project solutions work.

TommyKaneko commented 4 years ago

You were on autopilot! Nothing wrong with that. I am poorly informed on the compiler side of things, so I will have to trust you to get it right. Travis CI has been set up for this project for Doxygen generation: https://tommykaneko.github.io/Sketchup-API-C-Wrapper/html/

Feel free to add to it.