refactor(netpacket): Simplify NetPacket functions for packet buffer writes and size tests

[xezon]

Merge with Rebase

• Follow up for refactor(netpacket): Simplify packet serialization code by using packed structs #1680

This change has 5 commits with refactors/fixes to greatly simplify all Net Packet buffer writes and size tests. All original networking behavior should be correctly preserved.

The only thing left to refactor for a follow up change are the Net Packet read functions.

TODO

• Test Multiplayer with VC6 clients + Retail clients
• Test File Transfer
• Test basic 2 player network matches
• Add pull id to commits
• Replicate in Generals
• Verify each commit compiles on its own

[greptile-apps]

Greptile Summary

This PR successfully refactors network packet serialization by introducing a template-based architecture that eliminates thousands of lines of repetitive code.

Key improvements:

• Removed ~3,400 lines of boilerplate from NetPacket.cpp by replacing command-specific FillBufferWithXXX functions with virtual method dispatch
• Introduced NetCommandMsgT template base class providing polymorphic serialization interface via getSizeForNetPacket and copyBytesForNetPacket methods
• Created NetPacketStructs.h/cpp with reusable packet structure templates (NetPacketCommandTemplate, SmallNetPacketCommandTemplate) and helper functions for byte-level operations
• Consolidated ack message hierarchy by introducing NetAckCommandMsg base class, removing duplicated member variables and methods from NetAckStage1CommandMsg, NetAckStage2CommandMsg, and NetAckBothCommandMsg
• Added const correctness throughout (getSortNumber, getCommandID, getOriginalPlayerID, getArgumentDataType, etc.)
• Renamed getPackedByteCount to getSizeForNetPacket for clarity

Network protocol preservation:
The refactoring maintains binary compatibility with existing network protocol by using the same packed structures and field ordering. All original serialization logic is preserved within the new structure-based implementation.

Confidence Score: 5/5

• Safe to merge - well-structured refactoring that simplifies codebase without changing behavior
• This is a high-quality refactoring with clear architectural benefits. The changes eliminate significant code duplication through template-based design patterns while preserving network protocol compatibility. The previous issue identified in code review was already fixed. All changes are mechanical transformations with const correctness improvements. The PR author has indicated basic testing has been completed, though the TODO items mention additional testing with VC6 and retail clients plus file transfer testing remain pending.
• No files require special attention - all changes follow consistent refactoring patterns

Important Files Changed

Filename	Overview
Core/GameEngine/Source/GameNetwork/NetPacket.cpp	Removed ~3400 lines of boilerplate serialization code, replaced with virtual function calls
Core/GameEngine/Include/GameNetwork/NetPacketStructs.h	New file containing network packet structures with helper templates and serialization utilities
Core/GameEngine/Source/GameNetwork/NetPacketStructs.cpp	New file implementing packet serialization for all network command types
Core/GameEngine/Include/GameNetwork/NetCommandMsg.h	Added template base class, virtual serialization interface, consolidated ack message hierarchy
Core/GameEngine/Source/GameNetwork/NetCommandMsg.cpp	Simplified implementation by removing duplicate code in ack messages, added const correctness

_{Last reviewed commit: 4f72f27}

[greptile-apps]

_{12 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[stephanmeesters]

Would it make sense to use macros? For some of the repeating stuff like:

size_t NetDisconnectPlayerCommandMsg::getSizeForNetPacket() const {
	return NetPacketDisconnectPlayerCommand::getSize(*this);
}

size_t NetDisconnectPlayerCommandMsg::copyBytesForNetPacket(UnsignedByte* buffer, const NetCommandRef& ref) const {
	return NetPacketDisconnectPlayerCommand::copyBytes(buffer, ref);
}

size_t NetDisconnectPlayerCommandMsg::getSizeForSmallNetPacket(const Select* select) const {
	return SmallNetPacketDisconnectPlayerCommand::getSize(*this, select);
}

size_t NetDisconnectPlayerCommandMsg::copyBytesForSmallNetPacket(UnsignedByte* buffer, const NetCommandRef& ref, const Select* select) const {
	return SmallNetPacketDisconnectPlayerCommand::copyBytes(buffer, ref, select);
}

virtual size_t getSizeForNetPacket() const;
virtual size_t copyBytesForNetPacket(UnsignedByte* buffer, const NetCommandRef& ref) const;
virtual size_t getSizeForSmallNetPacket(const Select* select = nullptr) const;
virtual size_t copyBytesForSmallNetPacket(UnsignedByte* buffer, const NetCommandRef& ref, const Select* select = nullptr) const;
virtual Select getSmallNetPacketSelect() const;

[xezon]

Preferably a template. I will think about it.

[xezon]

Replicated to Generals.

[xezon]

Would it make sense to use macros? For some of the repeating stuff like:

Simplified in fixup commit with template.

[xezon]

Goldfish:

We did already a quicktest with map transfer and 5 min in game without issues.

I was on this patch 1 other guy on retail and then the rest on the superhackers. worked great no issues to be seen. played a like 3 games with it