259 Commits over 62 Days - 0.17cph!
Merge: from main
Tests: built all modes in editor
Update: Disable ItemPooling on Client and make server on by default
- Controlling variable is no longer replicated to client.
Client-side item handling is very lax and would need substantial changes. I don't want to hold up serverside release to get the full package out, I'd rather relase it bit by bit (and less risk of volatility).
Tests: Did all tests with both enabled and disabled server-side Item pooling
- Built all modes in editor.
- Tried reproducing 2 original bugs - didn't work.
- Loaded up a save that used to lead to a bug - didn't happen.
- Tested in local 2p session on Craggy - could kill, loot, equip and see changes on characters.
- Logs enabled through entire process - client didn't try to explicitly cleanup items (since disabled, as intended).
- Validated that it's possible to launch with command line disabling item pooling.
- Validated that it's possible to override item pooling with executable args
Update: added ItemManager logging of Item's lifecycle
- level 1 reports creation, loading and removal
- level 2 reports scheduling for removal
Tests: ran locally in the editor, switched items, dropped gear, looted existing containers. Checked that we get logging related allocs, but they're small (and I can remove them a bit later)
Bugfix: Rcon no longer waits for first command to start broadcasting log messages
Tests: Connected to server via test RCon page - started seeing output immediately
New: RustLog utility - consistently formatted logs with conditional output
- Extracted from BaseMonoBehavior, with forwarding of it's logs to RustLog
- Added Item category
We have scenarios where we have categorized logs in cases where we don't have BaseMonoBehavior available(Items) - this fills that niche.
Tests: Built all modes in editor. Ran with Network logging enabled - saw expected output. RCon page was good.
Update: regenerated ItemManager.EnablePooling command
Forgot to do it in previous submit
Tests: none, trivial change
Update: ItemRemove now schedules self recycling on Client
- subject if ItemManager.EnablePooling is set or not
This can automatically handle cases where we have client-side WorldItem & ItemContainers storing client-side items(amortizes synchronization) - but needs code fixing for that.
Tests: Tested dropping items(with pooling disabled it's ok, with enabled it's borked), equipping armor(same) - will fix in next CLs
Merge: from main
Tests: Build Server mode in editor, all good
Clean: comment explaining Client-side exclusive ItemManager use
Tests: none, trivial change
Merge: from main
Tests: editor build for all modes(Server borked, but fix coming)
Bugfix: Don't leak pooled lists in RunInRadius<T>
Thanks for the report!
Tests: ran admin commands that would generate a leak - no more.
Update: ItemManager is available on the client
- Like server, ticks in the background and cleans up items
- when creating/loading items can optionally specify whether it's client-side or server-side item (checked in exclusive modes)
- Made EnablePooling a replicated var, it disables clientside effects
We have a bunch of client-side item allocations that we don't correctly clean up - this will help deal with that.
Tests: built all modes in editor
Bugfix: don't leak an item if creation request was invalid
Tests: none, trivial change
Bugfix: don't create and access a dangling Item when Pooling is enabled
- at the same time it also optimizes inventory processing by batching the update as a result of multiple equipment events.
Tests: Tested by loadinga a save that borked the inventory by spawning NPCs - passed. Also validated that in multiplayer, wearables update when the player sleeps if they got changed by a different player.
Update: adding a server "itemmanager.enablepooling" convar to control item pooling
- starts off by default
Item pooling is too unstable to not have a dynamic switch - already got bit 2 times by it. Who knows what other horrors are hiding out there.
Tests: enabled after loading craggy, gave an ak - saw an uptick in pool.print_memory telemetry. disabled, despawned ak - no changes in telemetry.
Merge: from main/item_pooling
This fixers the NRE on server tick (and blocker for joining).
Tests: loaded a save that consistently produced NREs - not anymore
Bugfix: disabling Item pooling
This causes issues on the server which prevents playing.
Tests: loaded a save that consistently produced exceptions - not anymore.
Merge: from main
Tests: none
Merge: from item_pooling
This reintroduces pooling for Items while fixing the original bug that caused their backout.
Tests: build all modes in editor, tried to repro original bug - didn't happen.
Merge: from main
Tests: none (no conflicts)
Bugfix: Don't throw when cancelling partially finished crafting queue work item
Tests: Got 2 stacks of 300wood, scheduled 2 single doors as a single crafting operation, cancelled after first door crafted - no exceptions
Bugfix: avoid sending consumed-0 telemetry events when crafting
Tests: Had a queue of 2 items with perfect resource allocation - hit breakpoint once on first item craft instead of twice as before
Merge: from main
Tests: none
Merge: from itemcontainer_pooling
Fixes the "Double init of inventory!" assert on killing NPCs.
Tests: spawned scientist NPC, killed them - no assert logged, corpse still clothed and loot present.
Merge: from main
Tests: none, no conflicts
Bugfix: Remove assert that forbade double-loading of ItemContainer-owning types
- Removed a separate dead assert
Entities can be synchronized over the network multiple time via doing a whole load, which invalidated my previous logic.
Tests: Spawned a bunch of NPCs and killed them - they didn't generate asserts.
Update: Bringing back Item pooling
- Added a couple extra fields that were missed
Still need to reproduce the NRE we encountered earlier.
Tests: built all modes in editor. Spawned an AK, put an ext-mag on it, threw away to despawn, spawned a new one - didn't have attachments
Merge: from main
Tests: default editor build
Clean: removing obsolete methods from Pool
Tests: built all modes in editor
Merge: from main
Tests: editor build
Merge: from itemcontainer_pooling
Tests: built all modes in editor, booted craggy - no asserts, loaded a save on procgen - no asserts.
Merge: from main
Tests: editor compiled
Update: Assertions to catch leaking ItemContainer on init
- Removed lazy cleanup on init - due to Load-Init flow it's dead code
- Removed a redundant ItemContainer.Clear call
Tests: Booted Craggy - no asserts. Booted a save game from a proc map - no asserts.
Clean: removing redundant ItemContainer.Kill calls
- Removed a comment since it became obsolete
Returning to Pool will "Kill" them.
Tests: None, trivial changes
Clean: removing overriden-but-not-really method from LootableCorpse
Tests: none, trivial change
Clean: simplifying ItemContainer.MarkDirty
Tests: none, trivial changes
Update: ModularVehicleInventory pools ItemContainer
- Had to cause it to discard MVI on server destroy to properly clean up pool containers. Can add MVI pooling later.
Tests: on craggy approached one of spawned vehicles, inserted pistons in the engine then blew up with c4. Pistons dropped, next spawned vehicle didn't have them. Telemetry had expected values.
Update: ContainerIOEntity pools ItemContainer
- reimplemented inventory accessor to be backed by an explicit private var
Tests: spawned small rain collector, destroyed it - saw expected changes in telemetry
Update: ItemBasedFlowRestrictor pools ItemContainer
Tests: in editor on Craggy made fusebox mortal and shot it - saw ItemContainer returned to pool
Update: DroppedItemContainer pools ItemContainer
Tests: spawned a wooden box, added rock and torch, destoyed it with AK. Looted the dropped container, it disappeared. Saw the uptick and downtick in telemetry when expected.
Bugfix: Avoid dangling ItemContainer reference on Clear.
- ItemContainer.Clear now removes items immediately, rather than deferring them to ItemManager's removal queue
- Callbacks are also invoked earlier, before the ItemManager's removal queue pumping
- Instead of sending out per-item ItemContainer-MarkDirty events, we only do 1 for all
- ItemContainer.Kill also clears onPreItemRemove and parent
This prevents a pooling bug where we recycle ItemContainers in same frame before ItemManager removal queue being pumped. It would lead to "invisible" items occupying the inventory.
Tests: Spawned a wooden box, filled it up with stuffs, destroyed it - dropped container spawned with inventory(instead of dropping on the ground). Looting it caused it to despawn(instead of stay in the world).
Bugfix: don't invalidate ItemModContainer's availableSlots
- Also restricted access to availableItems to prevent future accidents and make it easier to reason about list's lifetime.
This is my bug - Pooling resets available slots on ItemContainer, so if they're shared by ref - original source gets nuked.
Tests: got ak47 with ext mag, dropped it, it despawned, spawned new one, tried to put rock into gear slots - it got rejected
Update: output results in micros for Recursion test
Tests: ran the benchmark, confirmed output
Update: PlayerLoot pools Client-side ItemContainers
Tests: tested both server-client looting and just client looting. checked telemetry on both sides - saw expected ups and downs.
Update: forgot to allocate the ItemContainer from pool for IndustrialCrafter
Tests: Setup a T3 workbench with industrial crafter and ak bp(saw an uptick), broke the foundation(saw a downtick), setup again - it was empty as expected
Update: IndustrialCrafter pools ItemContainer
- reimplemented properties as explicit interface props where applicable
- cached LootPanelTitle translated phrase (as it was allocating on every get)
- removed dead code in CanPickup
- got rid of if branch on ServerInit (since we now destroy inventory on entity destroy)
Tests: Setup a T3 workbench with industrial crafter and ak bp(saw an uptick), broke the foundation(saw a downtick), setup again - it was empty as expected
Update: Item & ItemModContainer pools ItemContainer
Tests: spawned an AK, added extended mag, dropped it - saw uptick. after it despawned - got downtick. Spawning another AK gave me a fresh and empty one.
Bugfix: missed original allocation in PlayerCorpse
Tests: none, trivial changes
Update: PlayerCorpse pools ItemContainer
Tests: dressed up, suicide, saw uptick of containers, body disappeared, saw downtick of containers. Repeated, got expected results and didn't see a leak
Update: LootableCorpse pools ItemContainer
Tests: Equipped a horse with backpack and items, killed it - saw the uptick in ItemContainer telemetry, then once corpse despawned, it ticked down.
