Fixed inconsistent use of memory management functions

[pzychotic]

Pull Request Type

• [ ] GitHub Workflow changes
• [ ] Documentation or Wiki changes
• [x] Build and Dependency changes
• [ ] Runtime changes
  • [ ] Render changes
  • [ ] Audio changes
  • [ ] Input changes
  • [ ] Network changes
  • [ ] Other changes

Description

Before #436 MEM_USE_RTL was a define in lib/mem.h that could be uncommented to change the memory management functions. So every library including mem.h was agreeing on the same memory functions.

By changing this to a CMake option and only enabling it on the Descent3 target, all other libraries would never activate this preprocessor definition when the option is enabled and therefore use different memory functions.

Allocating memory from any code of a library and freeing this memory from code compiled as part of Descent3, or vice versa, will result in memory corruption.

Related Issues

Fixes #444

Screenshots (if applicable)

Checklist

• [x] I have tested my changes locally and verified that they work as intended.
• [x] I have documented any new or modified functionality.
• [x] I have reviewed the changes to ensure they do not introduce any unnecessary complexity or duplicate code.
• [x] I understand that by submitting this pull request, I am agreeing to license my contributions under the project's license.

Additional Comments

[winterheart]

Probably is better to move target_compile_definitions into mem module, where it should belong to.

[Lgt2x]

Ugh I missed that at review. I agree that it should be a target_compile_definitions though

[pzychotic]

I understand why you would want to do that, but...

In order for this to work, every library that includes mem.h would also need a dependency to the mem target through target_link_libraries in order for that define to propagate correctly. Not a single library does that currently. This means changing close to 20 of our roughly 50 libraries.

And if than someone refactors parts of the project or includes mem.h in another lib that doesn't have the dependency yet, we might not even realize that we end up with the same runtime problems we got now.

Unless we can somehow detect this at build time, having this globally seems to be the save option.

[Lgt2x]

ok for this temporary fix, we can refactor later. I'll create an issue so we don't forget.

[Lgt2x]

See #447