Task mpl 2

[royfalk]

This is a reopening of #816.

This code decouple the MPL from rest of main body of code and make it and cargo part of resource lib. It makes it easier to test (some basic tests included) and also removes dependencies on the gfx folder among others.

This PR is part of a list of small PRs intended to come before #810. They are intended to clear the way for it and reduce the actual code in it to a manageable level. Note that #810 is a draft PR for review. Once these PR's are merged, I may break it up into smaller chunks for better processing.

Code Changes:

[x] Have the PR Validation Tests been run? See https://github.com/vegastrike/Vega-Strike-Engine-Source/wiki/Pull-Request-Validation

Issues:

• Removed hard coded pilots, slaves and hitchhikers from the code. All items should be in the MPL json and we should trust it to be so. Pilots and slaves are there. Hitchhikers are not.
• Removed dynamically generated items. There was code in unit_csv.cpp:1423 (UpdateMasterPartList) that added stuff missing from MPL upon docking. We should expect the MPL to keep a copy of all items in the game.
• The code in script_call_unit_gen.cpp generated random cargo and had two fall backs. 1st looked for cargo with mission in the name for some reason. The 2nd just created a dummy cargo using new and let it leak with a comment that it won't. I replaced that with something I think is better and more consistent.
• For some reason my PR's still carry more commits than the last one. It's as if I'm not pulling latest before but I did. Not sure why.

[royfalk]

Summarizing discussions from #816:
1. Is this code compatible with PWCU and with Black Paralysis? Not an issue for black paralysis.

Probably not an issue for WC but I will set aside time after lib component to stabilize WC and test it.
2. Do we still look for cargo with mission in the name? If so, I think I am good to approve this PR. Subject to the file conflicts being resolved, of course.

We don't.

This code is supposed to generate random cargo for a category. Its fallback is to generate random category with the mission in the name.

It has been replaced with code that generates cargo for a category with fallback to just random cargo.

I don't see how this change could impact gameplay.

[royfalk]

I have no idea why this is failing.

/usr/include/c++/13/bits/alloc_traits.h:70:31: error: static assertion failed: allocator_traits::rebind_alloc<A::value_type> must be A
70 | _Tp>::value,
| ^~~~~
/usr/include/c++/13/bits/alloc_traits.h:70:31: note: 'std::integral_constant<bool, false>::value' evaluates to false

[stephengtuggy]

Summarizing discussions from #816: 1. Is this code compatible with PWCU and with Black Paralysis? Not an issue for black paralysis.

Probably not an issue for WC but I will set aside time after lib component to stabilize WC and test it. 2. Do we still look for cargo with mission in the name? If so, I think I am good to approve this PR. Subject to the file conflicts being resolved, of course.

We don't. This code is supposed to generate random cargo for a category. Its fallback is to generate random category with the mission in the name. It has been replaced with code that generates cargo for a category with fallback to just random cargo. I don't see how this change could impact gameplay.

Are you sure that generating random cargo with the mission in the name is just the fallback? Rather than one of the primary purposes of this code? If you're running a cargo mission, you need cargo to deliver, right? 🙂

Even if it is just a fallback, that would mean that we're changing code behavior with respect to an edge case. As a general rule, I think we're better off not changing effective code behavior during a refactor. Even in relatively minor ways. Separating out behavior changes from refactors can make things significantly easier to sort out later, it seems to me.

Of course, it's possible to be too pedantic about this. But what are your thoughts on the above?

[stephengtuggy]

You are quite correct about that memory leak being there in the old code, comment notwithstanding. We should definitely avoid that. Perhaps log a fatal error and exit, if that code block ever gets hit?

[stephengtuggy]

Cool!

[stephengtuggy]

I especially like how you used the inline lambda function there. That adds a nice touch