Separate Cargo from Equipment #5389

Web-eWorks · 2022-06-27T19:47:21Z

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:

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)

data/libs/SpaceStation.lua

CargoManager replaces EquipSet for managing ship commodity cargoes. It will eventually handle timed / mission-specific cargo as well as be the backing store behind station player cargo storage. CommodityType replaces EquipType to represent commodity items - serialization of commodity items will be handled by commodity IDs instead of using table identities.

Refactor users of Commodities module to refer to old Equipment.cargo table instead. Refueling and Jettisoning cargo uses CargoManager now.

Add commodity-related functions to SpaceStation, which currently redirect to existing EquipmentStock functions. Handle a ship's type changing at runtime (persist cargo across ship type change). Refactor Ship Market screen to work with new cargo API. Refactor station lobby screen to use new cargo API. Refactor CommodityMarket to use new cargo API instead of EquipSet. GetTotalSpace() returns available cargo space after equipment mass.

Convert mission modules to use new cargo API. Refactor parts of DebugRPG to be cleaner. Clean up the starting situation definitions in mainmenu and use new cargo API.

All that remains is removing C++ calls using the old Cargo API.

When loading a CommodityType object that's already registered, return the registered commodity instead. If the commodity doesn't exist, register a new commodity and return it. This ensures table identities are preserved across savefile boundaries and commodity types are mostly safe against refactoring.

CommodityType.life_support denotes the level of life support required for the commodity to avoid perishing. Currently statically assigned, should be refactored in C++ to load from commodity JSON files.

Move life-support checks into a timed callback running for all ships - it's not fast, but it's not a significant performance problem. Move cargo scoop and fuel scoop handling partially out of C++ and move to new cargo API.

Commodity stocks now have a separate implementation backed by the Commodities table. Remove EquipType.cargo handling, Equipment.cargo table, and don't create EquipTypes for commodity items anymore.

Web-eWorks · 2022-07-08T21:48:03Z

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

impaktor · 2022-07-11T09:11:29Z

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

first time I could get there on the hydrogen I had in my cargo.
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?
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: d6ecf49

impaktor · 2022-07-11T18:26:40Z

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

robothauler · 2022-07-16T11:38:53Z

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 · 2022-07-16T12:03:51Z

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

data/libs/SpaceStation.lua

Bodasey · 2022-07-19T08:55:32Z

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 · 2022-07-19T22:31:42Z

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

Web-eWorks · 2022-07-19T22:53:49Z

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 · 2022-07-20T16:54:06Z

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 · 2022-07-20T19:56:15Z

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

Web-eWorks · 2022-07-21T19:25:41Z

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 · 2022-07-21T19:42:13Z

easy way to get one?
command line option?

Web-eWorks · 2022-07-21T19:45:29Z

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 · 2022-07-21T19:59:24Z

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

Bodasey · 2022-07-21T20:23:03Z

Thank you!

Bodasey · 2022-07-21T23:31:43Z

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

Web-eWorks · 2022-07-22T01:08:03Z

@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 · 2022-07-28T08:44:37Z

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.

Log a warning when attempting to set commodity or equipment stocks before visiting the station. Fixes crash with event handler ordering issues in NewsEventCommodity.

Web-eWorks · 2022-09-26T00:35:21Z

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 ☹️

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 · 2022-10-01T21:37:20Z

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 ;-)

Web-eWorks · 2022-10-18T02:17:37Z

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 · 2022-10-25T10:38:37Z

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?

Web-eWorks · 2022-10-28T07:10:20Z

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. 😄 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.

Web-eWorks · 2022-11-06T01:44:54Z

@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.

Web-eWorks added Design New feature Savegame bump Lua API labels Jun 27, 2022

Web-eWorks requested a review from Gliese852 June 27, 2022 19:47

Gliese852 reviewed Jul 5, 2022

View reviewed changes

data/libs/SpaceStation.lua Outdated Show resolved Hide resolved

Gliese852 reviewed Jul 5, 2022

View reviewed changes

data/libs/SpaceStation.lua Outdated Show resolved Hide resolved

Gliese852 reviewed Jul 5, 2022

View reviewed changes

data/libs/SpaceStation.lua Show resolved Hide resolved

impaktor force-pushed the master branch 3 times, most recently from 876b6fc to b5bc47c Compare July 6, 2022 13:11

Web-eWorks added 13 commits July 8, 2022 17:36

Convert econ_trade screen to use CargoManager

18eef1f

Refactor users of Commodities module to refer to old Equipment.cargo table instead. Refueling and Jettisoning cargo uses CargoManager now.

Convert main menu, mission modules part 1

59167a9

Convert mission modules to use new cargo API. Refactor parts of DebugRPG to be cleaner. Clean up the starting situation definitions in mainmenu and use new cargo API.

Add serialization to CargoManager/CommodityType

ba5d3c9

Finish porting modules to new cargo API

ac10987

All that remains is removing C++ calls using the old Cargo API.

Add Engine.RequestProfileFrame, profile LuaTimer

5d772b5

Add commodity life support field

e4c9d9d

CommodityType.life_support denotes the level of life support required for the commodity to avoid perishing. Currently statically assigned, should be refactored in C++ to load from commodity JSON files.

Refactor life support, cargo and fuel scooping

8da5603

Move life-support checks into a timed callback running for all ships - it's not fast, but it's not a significant performance problem. Move cargo scoop and fuel scoop handling partially out of C++ and move to new cargo API.

Use Commodity functions in commodity-market

e847327

Separate commodity stocks, remove Equipment.cargo

d014dd3

Commodity stocks now have a separate implementation backed by the Commodities table. Remove EquipType.cargo handling, Equipment.cargo table, and don't create EquipTypes for commodity items anymore.

Update copyright headers

d6ecf49

Web-eWorks force-pushed the cargo-component branch from 04cb390 to d6ecf49 Compare July 8, 2022 21:47

Gliese852 reviewed Jul 16, 2022

View reviewed changes

data/libs/SpaceStation.lua Show resolved Hide resolved

Gliese852 reviewed Jul 18, 2022

View reviewed changes

data/libs/SpaceStation.lua Show resolved Hide resolved

Web-eWorks added 2 commits September 25, 2022 19:49

Fix error in cargo life support check

174b274

Fix crash when mutating station market before visiting

af5d76e

Log a warning when attempting to set commodity or equipment stocks before visiting the station. Fixes crash with event handler ordering issues in NewsEventCommodity.

Web-eWorks mentioned this pull request Oct 16, 2022

Scoop missions do not work properly because onCargoDestroyed event is not emitted when cargo is scooped. #5406

Closed

Web-eWorks and others added 2 commits November 5, 2022 21:49

Address review comments

57ffb8c

Merge branch 'master' into cargo-component

8dfe910

Web-eWorks merged commit f58f8ed into pioneerspacesim:master Nov 6, 2022

Web-eWorks deleted the cargo-component branch November 27, 2022 03:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Separate Cargo from Equipment #5389

Separate Cargo from Equipment #5389

Web-eWorks commented Jun 27, 2022 •

edited

Loading

Web-eWorks commented Jul 8, 2022

impaktor commented Jul 11, 2022 •

edited

Loading

impaktor commented Jul 11, 2022

robothauler commented Jul 16, 2022

robothauler commented Jul 16, 2022

Bodasey commented Jul 19, 2022

Bodasey commented Jul 19, 2022

Web-eWorks commented Jul 19, 2022

Bodasey commented Jul 20, 2022

Bodasey commented Jul 20, 2022 •

edited

Loading

Web-eWorks commented Jul 21, 2022

Bodasey commented Jul 21, 2022

Web-eWorks commented Jul 21, 2022

impaktor commented Jul 21, 2022

Bodasey commented Jul 21, 2022

Bodasey commented Jul 21, 2022

Web-eWorks commented Jul 22, 2022

Bodasey commented Jul 28, 2022

Web-eWorks commented Sep 26, 2022

robothauler commented Oct 1, 2022

Web-eWorks commented Oct 18, 2022

Bodasey commented Oct 25, 2022

Web-eWorks commented Oct 28, 2022

Web-eWorks commented Nov 6, 2022

Separate Cargo from Equipment #5389

Separate Cargo from Equipment #5389

Conversation

Web-eWorks commented Jun 27, 2022 • edited Loading

Testing Notes

Tested and Working Modules / Features:

Web-eWorks commented Jul 8, 2022

impaktor commented Jul 11, 2022 • edited Loading

impaktor commented Jul 11, 2022

robothauler commented Jul 16, 2022

robothauler commented Jul 16, 2022

Bodasey commented Jul 19, 2022

Bodasey commented Jul 19, 2022

Web-eWorks commented Jul 19, 2022

Bodasey commented Jul 20, 2022

Bodasey commented Jul 20, 2022 • edited Loading

Web-eWorks commented Jul 21, 2022

Bodasey commented Jul 21, 2022

Web-eWorks commented Jul 21, 2022

impaktor commented Jul 21, 2022

Bodasey commented Jul 21, 2022

Bodasey commented Jul 21, 2022

Web-eWorks commented Jul 22, 2022

Bodasey commented Jul 28, 2022

Web-eWorks commented Sep 26, 2022

robothauler commented Oct 1, 2022

Web-eWorks commented Oct 18, 2022

Bodasey commented Oct 25, 2022

Web-eWorks commented Oct 28, 2022

Web-eWorks commented Nov 6, 2022

Web-eWorks commented Jun 27, 2022 •

edited

Loading

impaktor commented Jul 11, 2022 •

edited

Loading

Bodasey commented Jul 20, 2022 •

edited

Loading