refactor(network): Remove memory pool for NetPacket

[Caball009]

• Rebase and merge after PR perf(network): Reduce packet allocations and improve loop logic #2659

This PR makes two changes:

1. Refactors NetPacket::ConstructBigCommandList so that NetPacket doesn't have to be memory pooled.
2. Removes memory pool for NetPacket.

TODO:

• Verify everything works ok after perf(network): Reduce packet allocations and improve loop logic #2659 is merged.

[greptile-apps]

Greptile Summary

This PR removes NetPacket from the memory pool system by stripping its MemoryPoolObject base class and replacing all newInstance/deleteInstance calls with plain new/delete. It also refactors ConstructBigCommandPacketList into ConstructBigCommandList, which returns a NetCommandList* directly instead of building a list of intermediate NetPacket wrappers that were immediately consumed and discarded.

• NetPacket no longer inherits MemoryPoolObject; its pool entries are removed from both GameMemoryInitPools_Generals.inl and GameMemoryInitPools_GeneralsMD.inl.
• ConstructBigCommandList eliminates the round-trip through packet serialization/deserialization for oversized commands, directly populating a NetCommandList; it also fixes a pre-existing relay self-assignment bug (ref->setRelay(ref->getRelay())) and adds a missing deleteInstance(origref) that was leaking in the old send path.

Confidence Score: 4/5

The refactor is logically sound and also fixes two pre-existing defects (a relay no-op self-assignment and a missing origref delete). Only minor style concerns remain.

The core logic change — returning a flat NetCommandList instead of a list of temporary NetPacket wrappers — is correct, resource cleanup is properly handled, and relay propagation now works as intended. The only open items are the non-idiomatic new(Type) allocation syntax used in several call sites (readable as placement new by casual reviewers) and the two now-unused typedefs left in the header.

NetPacket.h still declares NetPacketList and NetPacketListIter which appear orphaned; Connection.cpp uses the confusing new(NetPacket) syntax at two call sites.

Important Files Changed

Filename	Overview
Core/GameEngine/Include/GameNetwork/NetPacket.h	Removes MemoryPoolObject base and the MEMORY_POOL_GLUE macro; replaces ConstructBigCommandPacketList with ConstructBigCommandList; adds explicit destructor; NetPacketList/NetPacketListIter typedefs now appear unused.
Core/GameEngine/Source/GameNetwork/NetPacket.cpp	Refactors ConstructBigCommandList to return a NetCommandList* directly instead of constructing intermediate NetPacket objects; also fixes a pre-existing relay self-assignment bug (ref->setRelay(ref->getRelay())); logic and cleanup look correct.
Core/GameEngine/Source/GameNetwork/Connection.cpp	Replaces newInstance/deleteInstance with new/delete for NetPacket; simplifies the big-command path to iterate the returned NetCommandList directly; also adds missing deleteInstance(origref) that was leaked in the old code. Non-idiomatic new(NetPacket) syntax used throughout.
Core/GameEngine/Source/GameNetwork/ConnectionManager.cpp	Replaces newInstance/deleteInstance calls with new/delete for NetPacket allocations; logic is unchanged and correct.
Core/GameEngine/Source/Common/System/GameMemoryInitPools_Generals.inl	Removes the NetPacket pool entry; straightforward cleanup matching the class no longer inheriting MemoryPoolObject.
Core/GameEngine/Source/Common/System/GameMemoryInitPools_GeneralsMD.inl	Same NetPacket pool entry removal as the Generals variant; change is mechanical and correct.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant C as Connection
    participant NP as NetPacket (static)
    participant BCL as ConstructBigCommandList
    participant NCL as NetCommandList

    C->>NP: reset() — test if msg fits
    NP-->>C: addCommand() → FALSE (too large)

    C->>BCL: ConstructBigCommandList(origref)
    BCL->>BCL: copyBytesForNetPacket → bigPacketData[]
    BCL->>NCL: newInstance(NetCommandList)
    loop each chunk
        BCL->>BCL: create NetWrapperCommandMsg chunk
        BCL->>NCL: addMessage(chunkRef)
    end
    BCL->>BCL: delete[] bigPacketData
    BCL-->>C: "return commandList*"

    loop each chunkRef in commandList
        C->>C: "m_netCommandList->addMessage(ref1->getCommand())"
    end
    C->>NCL: deleteInstance(list)

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant C as Connection
    participant NP as NetPacket (static)
    participant BCL as ConstructBigCommandList
    participant NCL as NetCommandList

    C->>NP: reset() — test if msg fits
    NP-->>C: addCommand() → FALSE (too large)

    C->>BCL: ConstructBigCommandList(origref)
    BCL->>BCL: copyBytesForNetPacket → bigPacketData[]
    BCL->>NCL: newInstance(NetCommandList)
    loop each chunk
        BCL->>BCL: create NetWrapperCommandMsg chunk
        BCL->>NCL: addMessage(chunkRef)
    end
    BCL->>BCL: delete[] bigPacketData
    BCL-->>C: "return commandList*"

    loop each chunkRef in commandList
        C->>C: "m_netCommandList->addMessage(ref1->getCommand())"
    end
    C->>NCL: deleteInstance(list)

Loading

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
Core/GameEngine/Source/GameNetwork/Connection.cpp:138
**Non-idiomatic placement-new-like syntax**

`new(NetPacket)` is syntactically valid C++ (it's `new` with a parenthesized type-id), but it is visually indistinguishable from a placement-new call where `NetPacket` would be the memory buffer—which would be a serious error. Readers unfamiliar with this syntactic edge case will spend time checking whether a custom `operator new(void*)` overload is in play. The same pattern appears at line 261 in this file and in `ConnectionManager.cpp` (lines 469, 495, 522). Standard `new NetPacket` / `new NetPacket(&...)` conveys the intent unambiguously.

### Issue 2 of 2
Core/GameEngine/Include/GameNetwork/NetPacket.h:45-46
**Orphaned typedefs after API removal**

`NetPacketList` and `NetPacketListIter` were the return type of the now-removed `ConstructBigCommandPacketList`. With the function replaced by `ConstructBigCommandList` (which returns `NetCommandList*`), these two typedefs have no remaining users in the changed files. If they are also unused elsewhere in the codebase, they should be removed to avoid dead code accumulating in the header.

_{Reviews (1): Last reviewed commit: "Removed memory pool for 'NetPacket'." | Re-trigger Greptile}