fix(memory): Fixed memory leaks at shutdown

[bassdr]

Was not cleaning up the context properly, leaving things initialized while other were getting deleted. I did not run valgrind or such, so there are probably other memory leak.

Build Artifacts

• soh-windows.zip
• soh-mac.zip
• soh-linux.zip

[Pepper0ni]

There's already a fix for the crash at least, but that is currently on a different branch due to LUS development pipelines being in a bad place. Kenix3/libultraship#1103

Said Pipelines mean there's a good chance the LUS PR will not be accepted for a while.

[bassdr]

I update this PR

• Reverted Move to port-maintainance LUS branch to fix crash on close #6629, my fix works with LUS' main, I don't think pinning port-maintainance branch is the way to go IMHO
• Removed the pin to the CTRL+C (aka. SIGINT) fix, it's not exactly related

So this PR does NOT depend on any version of LUS anymore, it can be merged as is.

[Malkierian]

Why don't you think pinning port-maintenance is the answer?

[bassdr]

While I am not a fan of using std::shared_ptr to manage lifetime of objects, as it's much clearer IMO having clear ownership and use eg. std::unique_ptr or no pointer at all in the owner and let consumers use references to access it.

But, LUS is not built that way at all, it has std::shared_ptr everywhere, and lifetime of objects are managed by keeping a std::shared_ptr on the object while using it. Destruction of the objects happens when nobody uses them, no matter when this happens. While not my favorite pattern, it's defendable the way it's used in LUS. I feel like diverging from that with the GetRawInstance is moving away from this pattern, creating confusion in the design. My PR just embrases the design and release the shared_ptr only when done using it, delaying destruction just enough so we don't have issues.

Also, I am more interrested in #6668 landing, I think it's a HUGE audio quality improvement. I am done, just have to prepare a video or something so that people understand (aka. hear) the difference, it's actually amazing. A big chunk of the work was done in LUS itself, and it started from main. It's already tedious to have to pin my branch on LUS, I know have to also bring the changes from port-maintenance, and it's very annoying.

If you tell me the plan is to start a branch in LUS and use that one in shipwright, then I'll target that branch with my PRs in LUS (probably will end up having two PRs, for other games too). That can be a fine answer. I just don't know the plan right now, and I personally think we should stick on main, especially that this PR fixes the same issue that was adressed in #6629 still pinning main.

[bassdr]

I ended up removing the revert from here... so this is just memory management cleanup now.

[Malkierian]

Sorry, missed the last comment until just now. The changes louis made for Context on port-maintenance is how we're planning on going with most of the rest of the shared_ptrs throughout the codebase, it was just a first step. Though, whether to use unique_ptr or raw pointers is something that could still be discussed, I think.

[bassdr]

OK, thanks, I still think this PR worth merging IMO, even if it's not a matter of crashing at runtime anymore, releasing the shared_ptrs we do not use anymore is a sane thing to do, while we migrate to the new ownership pattern.

[mergify]

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

• Queue this pull request