Project

General

Profile

Actions

Feature #446

closed

packets: complex field support

Added by Alina Lenk about 1 month ago. Updated about 1 month ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
General
Target version:
Start date:
04/12/2024
Due date:
% Done:

0%

Estimated time:

Description

Introduce necessary code to support packets with fields of complex types (like vectors) that cannot just be bit-copied (safely). These require special handling for initialization, copying, and destruction.

Required for HRM #657288 (strvec fields).


Files

Actions #1

Updated by Alina Lenk about 1 month ago

Nomenclature issue: I'm trying to keep the wording for "de-initialize in place" and "de-initialize and release heap allocation" consistent with other parts of the code, where in-place is called "free" (e.g. astring, specvec) and release is called "destroy" (e.g. genhash, secfile), even though destroy is the one that calls free() on its argument. This isn't really an issue for those types, because they only ever have one of those two functions, but for packets we need both (unless we want to unconditionally allocate all packets on the heap, which... would be a last resort, I guess, but not a good one).

I think we'll just have to live with that; these individual functions will not be visible outside packets_gen.c anyway, so the damage is at least contained. Though I am considering calling the "free" functions (that don't call free() on the argument itself) something else, like "deinit". If anyone has input on the matter I'd appreciate it, 'cause neither option sounds that much better than the other.

Actions #2

Updated by Marko Lindqvist about 1 month ago

You may want to see that https://github.com/freeciv/freeciv-web/blob/develop/scripts/generate_js_hand/generate_packets.py remains functional, though it should not keep us down from making desktop freeciv better.

Actions #3

Updated by Alina Lenk about 1 month ago

Marko Lindqvist wrote in #note-2:

You may want to see that https://github.com/freeciv/freeciv-web/blob/develop/scripts/generate_js_hand/generate_packets.py remains functional, though it should not keep us down from making desktop freeciv better.

The changes in this ticket (will) affect neither the packets.def format nor the network protocol (just the implementation thereof), so that should be unaffected. Although if any of fcweb's code patches interact with packets allocated on the heap (they probably don't), those might have to replace free(packet) with the new packet_destroy(packet, type) in some places.

The changes this is preparing for (e.g. vector support) will make additions to the packets.def format, so that might break the old parser once those are actually used; this is already the case for arrays with more than two dimensions, which we support, but the old parser does not.
As far as I can tell, fcweb's generate_packets.py isn't being used for more than parsing the def and establishing which ID maps to which name? If that's the case, it shouldn't be too hard to import the new version and get the same information while having an up-to-date parser — the new script is safely importable without side effects, and the parsing and generation steps are cleanly separated. (Or alternatively, to cut out all the parts looking at the inside of the packet, so addition of more advanced field types will no longer break it.)

Actions #5

Updated by Alina Lenk about 1 month ago

  • Status changed from In Review to Closed
Actions

Also available in: Atom PDF