turanszkij / WickedEngine

3D engine with modern graphics
https://wickedengine.net
Other
5.86k stars 617 forks source link

Downgrade Jolt to official release 5.0.0 for now #877

Closed brakhane closed 5 months ago

brakhane commented 5 months ago

Since we're not keeping track of what git checkout we're using, the only way to keep track of version is through the JPH_VERSION macros in Jolt/Core/Core.h

Unfortunately, after a release this version gets increased to the next version, there's no "wip" marker, which means that we seemed to be using version 5.0.1, which isn't released yet and which interface might yet change (it already did compared to version 5.0.0, which makes me think that the current master branch will become 5.1.0 and if there are any bugfixes for 5.0.0 those will be 5.0.1 without the API changes, which would make Wicked's "5.0.1" even more confusing)

Therefore, I propose that we just go with 5.0.0 for now

turanszkij commented 5 months ago

I don't think downgrading is necessary unless you have some specific issue?

turanszkij commented 5 months ago

Also I made a fix in the current Jolt version that I need to have: https://github.com/jrouwe/JoltPhysics/pull/1158

brakhane commented 5 months ago

I don't think downgrading is necessary unless you have some specific issue?

I've just been burned in the past in projects that have vendored in some random version of a library, and it made it almost impossible to ever update it, since it was not clear what version was use and what changes were needed, it was also a not released version and it's interface did not match any released version. So in the end, we just left the random version in forever.

Also I made a fix in the current Jolt version that I need to have: https://github.com/jrouwe/JoltPhysics/pull/1158

This is exactly the kind of stuff I'm worried about, which will make updating Jolt in 1 year really difficult.

Can we at least add a patches.txt file or something into the directory that lists the git version it is based on and what patches were applied? This way, we will be able to regularly pull in the current master branch and also upgrade to 5.1.0 or whatever version comes next, and even apply the patch if it hasn't made it yet to upstream. Even a Jolt fork with the patches applied and mentioned in a file in that directory would help.

Sorry for being so annoying about this, but I've really been burned by that stuff in the past.

turanszkij commented 5 months ago

The patch is now merged to Jolt's master branch so upgrading won't be difficult. If there will be a patch that is not included in Jolt's repo then I will think about a way to document them.

brakhane commented 5 months ago

OK, I'll close this one then and will make "update Jolt" PRs from time to time then.