jrouwe / JoltPhysics

A multi core friendly rigid body physics and collision detection library. Written in C++. Suitable for games and VR applications. Used by Horizon Forbidden West.
MIT License
6.38k stars 409 forks source link

Request for official vcpkg support #1176

Closed miss-programgamer closed 1 week ago

miss-programgamer commented 1 month ago

There is currently a vcpkg port of Jolt available, though it is an unofficial one supported by a third party. Would you consider making an official vcpkg port and maintaining it in the future? It would be nice since vcpkg is by far the most convenient way to download C++ dependencies. Thank you for your consideration!

jrouwe commented 1 month ago

I'm not very familiar with setting up / using vcpkg so I'm quite happy for someone else to take care of this. Is there any disadvantage of the current package?

miss-programgamer commented 1 month ago

Only minor things, like the fact that its name contains "unofficial", which is a bit of a mouthful, but having it maintained in a first party fashion would also means that the library version on vcpkg would pretty much always be up to date with the latest distribution. Of course, I understand that it's a time/effort commitment, so the decision to not go ahead with this is entirely fair.

jrouwe commented 3 weeks ago

@RT2Code, I'm fine if you want to remove the 'unofficial' bit from the vcpkg port.

I'm planning to release a new version of Jolt soon and I see that you have some patches which could perhaps be merged into the main line.

I'm hoping that this change:

https://github.com/microsoft/vcpkg/blob/fe1cde61e971d53c9687cf9a46308f8f55da19fa/ports/joltphysics/fix-export.diff#L9-L12

is no longer needed since you can use OVERRIDE_CXX_FLAGS on MSVC too now.

This change:

https://github.com/microsoft/vcpkg/blob/fe1cde61e971d53c9687cf9a46308f8f55da19fa/ports/joltphysics/fix-export.diff#L33-L43

can perhaps be replaced by this change in Jolt:

https://github.com/jrouwe/JoltPhysics/blob/d2cd9e8fe5b9bd662d421d7d2ba60e027d438cd4/Build/CMakeLists.txt#L266-L273

but I'm not very familiar with 'installing' using cmake so I may be completely wrong.

RT2Code commented 1 week ago

The unofficial prefix here is misleading. The port is not unofficial at all, only the CMake export is, and the prefix is used to avoid conflict when upstreams add an official one..

Now that joltphysics has a built-in CMake export, this patch is essentially obsolete, and I think we can remove it entirely. I'm going to update the vcpkg port.

jrouwe commented 1 week ago

Let me know if you need to create another patch. If there is, let's see what I need to change so that you don't need to patch.

RT2Code commented 1 week ago

I made a PR to remove the patch and update the port to the 5.1.0 release: https://github.com/microsoft/vcpkg/pull/40526

I'm waiting for the vcpkg team feedback, but it should be good to go. :)

jrouwe commented 1 week ago

Thanks for maintaining this!

RT2Code commented 1 week ago

And thanks for your hard work on this amazing library. The PR was merged, this issue can be closed.