branchrust_reboot/main/itemcontainer_poolingcancel
28 Commits over 0 Days - ∞cph!
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.
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: 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.
Update: PlayerInventory pools ItemContianers
Tests: equipped backpack, loaded up on items, killed self and checked that look transferred properly. Confirmed recycling in 2p session by killing the sleeper.
Update: BaseRidableAnimal pools ItemContainers
- Also "fixes" silent id leak by returning it
Test: On Craggy, equipped and shot a horse - saw 2 ItemContainers returned to pool in stats. Spawned another one - they got consumed, and gear was empty.
Clean: remove BaseRidableAnimal.OnInventoryFirstCreated
- Inlined only usecases's event code into init
- Cleaned up double-nested ifdef and a bunch of newlines
It has been used incorrectly anyway and was misleading.
Tests: none, trivial changes
Update: ShopFront pools ItemContainer
- also deleted empty virtual override
Test: 2 player session with a shop front, one was interacting with the front while I destroyed it - items dropped, next front clear
Clean: Remove redundant StorageContainer.ResetState
All the cleanup is already done in DoServerDestroy. Might be worth consolidating these callbacks in the future.
Tests: none, trivial change
Update: StorageContainer now pools ItemContainer
- had to introduce a member variable to avoid funky pool API usage because oif property
- also had to relax empty container assert, as there already are cases that don't clean up their inventory on kill
Tests: on Craggy killed all entities via console, saw the pool container stats go down to 0
Update: ItemContainer supports IPooled
- Minor fix - avoid double returning container UID
- replace a static-readonly with a const
Right now there's only 1 place that pools ItemContainers(reclaim manager), so impact of this is low.
Tests: in softcore killed self and looted reclaim backpack.
Tests:
Optim: avoid copying items on container clear
- Also consolidated it into one function.
Did an overview of existing code, didn't spot cases where we can try to modify the container as we're clearing it.
Tests: killed a bunch of animals, loaded up a wooden storage box then killed it
