Separate Cargo from Equipment

[sturnclaw]

This has been a long time coming as I'm sure @laarmen can attest, but with the advent of the Lua Component feature, I finally have all of the technical pieces in place to do this refactor.

I've added a completely new Lua CargoManager component for ships that is wholly separate from the EquipSet management, and converted all commodities in the game to use the new cargo manager system. Commodity items are now tracked by count and managed by type rather than having hundreds of instances of the same item in an array. This gets rid of a significant amount of overhead when working with commodities, and enforces a clear semantic separation between "items in your cargo hold" and "items mounted to your ship".

As part of this refactor, I've made (and backported) changes to Lua serialization to fix common pain-points when working with commodity types; when loading a saved game, commodity items serialized in the save by lua modules should no longer be duplicated / have a different table instance compared to the ones that were registered as part of normal game startup. I've still made the effort to deal with commodities using string IDs internally to be as robust as possible across serialization, but this change should be useful in the future for other modules as well.

The design of the CargoManager component is intentionally minimal at this time; for this PR I only focused on separating existing commodity-handling functionality out of EquipSet, but the API is designed to allow for future expansions to bring features like:

• Marking certain cargo items as illegally salvaged from a wreck;
• Tracking mission-target cargo items separately from player-purchased commodities;
• Volatile or time-sensitive cargoes which need special handling;
• Storing and transporting equipment items, such as ones removed from the player's ship;
• and potentially even changing ship cargo space to be controlled by adding or removing cargo rack equipment from the ship.

I've moved most C++ handling of fuel scooping and cargo scooping into Lua callbacks that can be further expanded or modded to support custom behaviors, and moved the "cargo life support timer" functionality into a Lua timer callback registered shortly after ship creation. This does add a little bit of overhead due to the unoptimized state of the LuaTimer implementation, but in my testing it averaged around an extra 200us / frame in a Debug build with 400+ ships alive and moving between planets, which is well below noticeable levels.

And lastly (but certainly not leastly) I've ported all of the mission modules and UI to use this new API instead of the old AddEquip/RemoveEquip/GetEquip/CountEquip/GetCargo methods on Ship. This by far took the longest, as most of our missions have at least something to do with cargo management (even if it is just to add a few tons of hydrogen).

And if it wasn't clear already, this PR will break backwards compatibility of game saves. With the sheer size of the PR and changes to major fundamental systems, I have no qualms about savebumping for this release and it's simpler to do it this way and focus on designing the new systems to maintain compatibility going forwards than to try and shoehorn in migration between incompatible systems.

Testing Notes

Due to the fact that it took me literally all of my free time this week late into the night just to do this refactor and make everything run with the new API, I've not had time to thoroughly test this branch to ensure that the mission modules work correctly and are error free.

As such, I'm going to request that the readers of this PR (yes, you, even if you're not a Pioneer developer) download and test the latest build from this PR and report back when you've tested a mission module below and ensured that it doesn't have any major regressions that would prevent the mission from working properly.

Because of the huge number of changes in this PR, I'd like to have the below checklist completely checked-off before merging this PR, so please test and report back your findings! It's acceptable to change mission parameters a little to make them spawn on the BBS or trigger rare events more often if needed to actually test the behaviors.

Tested and Working Modules / Features:

• [ ] Assassination (entering system with assassination target should not trigger errors)
• [ ] Breakdown Servicing (trigger a hyperdrive failure)
• [ ] Cargo Run Missions:
  • [ ] Deliver local cargo
  • [ ] Deliver interstellar cargo
  • [ ] Pickup and return cargo
  • [ ] Negotiate smaller delivery amount
• [ ] News Event Commodity (extreme demand event, extreme supply event)
• [ ] Scoop Missions:
  • [ ] Scoop legal cargo
  • [ ] Scoop illegal cargo
  • [ ] Scoop rescue capsules
  • [ ] Scoop illegal weapons
• [ ] Search and Rescue Missions:
  • [ ] Pickup/deliver crew (general regression test)
  • [ ] Deliver fuel

Regular game features that should be tested:

• [ ] Fuel scooping (from multiple gas giants preferably)
• [ ] Cargo scooping (jettisoned cargo)
• [ ] Regular cargo trading (ensure station stock depletion works properly across save/reload and when leaving and re-entering a system)

[sturnclaw]

Rebased onto latest master with a few minor rebase cleanups - I'll address the weird-parens towards the end of the review period.

[impaktor]

I've tested the News event module just now, I believe I've found two bugs, one of them probably related to this PR (or some earlier cargo/station refactoring?). I was in system A, had an event in system D, flew:

A -> B -> C (saved) -> D

1. first time I could get there on the hydrogen I had in my cargo.
2. if I load the game from system C, I have to pump down some hydrogen to make the last jump (regardless if soft / hard restart of pioneer). Possible this bug is on master as well?
3. if I do hard restart (quit pioneer, start & load game), it crashes when I jump into D and dock, when the last line in onShipDocked() is called

stack traceback:
	[C]: in function 'assert'
	[T] libs/SpaceStation.lua:462: in function 'AddCommodityStock'
	[T] modules/NewsEventCommodity/NewsEventCommodity.lua:367: in function 'cb'
	[T] libs/Event.lua:34: in function '?'
	[T] libs/Event.lua:185: in function <[T] libs/Event.lua:180>
Fatal: Frame instance deletion outside 'DeleteFrame' [0]

(line number is off by one, since I've removed one require line in my copy of NewsEventCommodity.lua)

Beside the above, things looked OK. The sold out commodity had 0t in stock, as it should. Price looked to be more than x2 the usual though. I'll look into that now.

I'll see if I can test the SoldOut module next

EDIT: I'm playing on your branch post your rebase, the other day, my head is on: d6ecf49336318e32799b01c34dd6c7f3130e94b0

[impaktor]

I'm attaching (not a zip!) a save when I'm in the process of docking, just before the crash. test_newsevent.zip

[robothauler]

I have tested the Scoop mission. It works, but the mission script does not know that all the mission containers/rescue capsules were scooped up. To complete the mission, the player has to save the game ...

Error: Lua serializer '.ShipClass..1.missions.1.debris.1.body' tried to serialize an invalid object The save file may be invalid. Error: Lua serializer '.ShipClass..1.missions.1.debris.2.body' tried to serialize an invalid object The save file may be invalid. Error: Lua serializer '.ShipClass..1.missions.1.debris.3.body' tried to serialize an invalid object The save file may be invalid. Error: Lua serializer '.ShipClass..1.missions.1.debris.4.body' tried to serialize an invalid object The save file may be invalid. Error: Lua serializer '.ShipClass..1.missions.1.destination' tried to serialize an invalid object The save file may be invalid.

... reload and land again.

Maybe this has nothing to do with this PR and slipped through from another PR.

[robothauler]

Current master seams to have the same issue. So not related to this PR.

[Bodasey]

Does this affect only code level and not the ships' definitions? I still see shared payload for equipment and freight when playing. Or have I done something wrong? I have installed this branch in a separate directory, of course.

[Bodasey]

Ok, just after starting a new game on Mars, having loaded everything towards Earth, started fly, some seconds later from nothing visible or done:

Stars picked from galaxy: 125000 Generating 0 random stars Final stars number: 125000 Fatal: [T] libs/CargoManager.lua:205: attempt to compare number with nil stack traceback: [T] libs/CargoManager.lua:205: in function 'DoLifeSupportChecks' [T] modules/LifeSupport.lua:22: in function 'LifeSupportCallback' [T] modules/LifeSupport.lua:51: in function <[T] modules/LifeSupport.lua:51> Fatal: Frame instance deletion outside 'DeleteFrame' [0]

Crash to Desktop

[sturnclaw]

Does this affect only code level and not the ships' definitions? I still see shared payload for equipment and freight when playing. Or have I done something wrong? I have installed this branch in a separate directory, of course.

The cargo mass metric is the same - your ship still has a fixed "mass capacity" which both equipment and cargo subtract from, and there is no volume metric. This PR simply refactors the way cargo works internally to allow moving to a volume metric.

[Bodasey]

Next crash to study, happened after hyperjump from the Moon to Barnard's Star, got a message of a pirate ship around and a police ship pursuing it, waiting some seconds -> memory access error

CTD-after-hyperjump-barnard.txt

[Bodasey]

Autopilot: It may also happen that your Sinonatrix ship starts rocking upside down 15km over Los Angeles (never seen that before), manual control cannot bring the ship out of that mode, so give order to fly back to Mexico City - same problem there.

Workaround: Fly to Gates Spaceport and sell some tons of Hydrogen, then land in LA (but the approach to LA is very slow).

Test assumption: buy all the hydrogen in LA again and fly to Mexico City - indeed. your ship never lands, drifting in 15km height.

=> Sinonatrix can not land when loaded almost fully. (jettison 3t of Hydrogen, and the ship lands)

SORRY PLEASE - not related to this PR

[sturnclaw]

Next crash to study, happened after hyperjump from the Moon to Barnard's Star

We cannot debug or resolve this crash without a backtrace from a debugger. The log has no actionable information.

[Bodasey]

easy way to get one? command line option?

[sturnclaw]

easy way to get one? command line option?

@Bodasey you'll need to run Pioneer in GDB (with gdb /path/to/pioneer) and provide the stack trace information from the GDB info stack command when the game crashes. Google is your friend when figuring out how to do all this, the process is pretty much the same regardless of which application you're trying to debug.

[impaktor]

@Bodasey https://pioneerwiki.com/wiki/FAQ#Debug_backtrace

[Bodasey]

Thank you!

[Bodasey]

It will take me some time to install all this debuginfo packages manually (automatic fails), but the crash after hyperjump seems to be reproducable. Hope this helps you nevertheless.

jumpBarnard2of2.txt

[sturnclaw]

@Bodasey that log is almost helpful - you'll need to type info stack at the (gdb) prompt and post the output from that command. Alternatively, a save file that can be used to consistently reproduce the crash would be extremely useful!

[Bodasey]

another freeze while arriving in Barnard's Star system:

Savegame, but no guarantee it is reproducable, other times these jumps went well: Cargo_vor_Flug_Barnard.txt

The first two reports are not from this game.

gdb log: freeze-after-hyperjump-Barnard.txt

last messages seen on screeen: Police: scan for illegal goods Ship1: another fat goose Police: we protect you Ship2: you will pay with your life for this cargo Police: no illegal cargo detected

Hope this report fulfills your needs now.

[sturnclaw]

Long belated reply, but I'm cleaning up some stuff on this PR again.

another freeze while arriving in Barnard's Star system

As far as I can tell, this is a bug in the AI system and not related to the cargo refactor.

if I do hard restart (quit pioneer, start & load game), it crashes when I jump into D and dock, when the last line in onShipDocked() is called

Blah, this looks like an ordering issue where the onShipDocked handler in NewsEventCommodity is being run before the one in SpaceStation.lua... not really a good way to fix this unfortunately. I'll throw in a compatibility patch to hopefully fix it, but that's just going to slow things down a bit.

I'm attaching (not a zip!) a save when I'm in the process of docking, just before the crash.

I cannot debug or test this savefile as it includes modded ships (in this case longnose is the first problem child) which I do not have installed. Sorry :frowning_face:

Ok, just after starting a new game on Mars, having loaded everything towards Earth, started fly, some seconds later from nothing visible or done:

Unsure what's responsible for this error, but fixed.

[robothauler]

Regarding the Scoop Mission. If I accept a mission to scoop illegal goods, I can see cargo from the Cargo Run Mission as well. So wedding dresses are now illegal ;-)

[sturnclaw]

Alright, seeing no further progress on testing this PR, I'm going to cleanup the remaining issues on it by start of next month and go ahead and merge - it can be tested in master and issues created as needed.

[Bodasey]

All cargo run missions: check fuel scoop on gas giants: check regular cargo trading: check

Alright, seeing no further progress on testing this PR, I'm going to cleanup the remaining issues on it by start of next month and go ahead and merge - it can be tested in master and issues created as needed.

As long as there are still so many basic bugs in the master behind, this way is - after the PR a certain time being open, say one month - the better one I think.

Do you have got some roadmap to the next (February) release yet? Are there still more bigger updates like this one or will there be a time for fixing bugs only?

[sturnclaw]

Do you have got some roadmap to the next (February) release yet? Are there still more bigger updates like this one or will there be a time for fixing bugs only?

The changelog is the best roadmap. :smile: Anything not currently on it will land only if the authors have enough time and energy to get their work done before the next release.

We'll most likely enter a soft feature freeze starting on Jan 1 2023, with a hard bugfix-only freeze 1-2 weeks out from Pioneer day. Anything that gets done and PR'd before then will be in the build, obviously, but I can't make any promises as to the specific contents of the next update.

[sturnclaw]

@robothauler

I have tested the Scoop mission. It works, but the mission script does not know that all the mission containers/rescue capsules were scooped up. To complete the mission, the player has to save the game ...

This has most likely been fixed by #5407, and the mission should complete correctly.