generate_packets.py: Support for opposite of handle-via-packet
alienvalkyrie Reply To cazfi
If this feature already exist
It doesn't.
It would probably be sensible to do away with the whole "implicit handle-via-packet for ruleset packets and packets with more than five fields" anyway and instead always set it explicitly in the packets definition – would have to decide whether handle-via-packet or handle-via-args should be the default; I'll see what's more common atm.
alienvalkyrie Reply To alienvalkyrie
I'll see what's more common atm.
On current master, the tally is:
So if we got rid of the implicit handle-via-packet, we'd have to add it explicitly to 60 packet definitions. I'm torn on whether that's better or worse than the complication of having an implicit default, an explicit opt-in and an explicit opt-out.
alienvalkyrie alienvalkyrie Minimal "explicit is better than implicit" patch. If you'd rather we keep the implicit handling, tell me and I'll look into a more complex solution.
Do we ever have a good reason to use handle-via-fields, from the POW of the quality / performance of the code produced? (I think it's mostly to make writing the handler function more convenient)
If yes: I do see some value in that it "suggests" switching to handle-via-packet mode when you add more fields. Doing that switch is unlikely to occur to the coder at that point.
If not: We could make handle-via-packet always the default (i.e. redundant to define explicitly), and just provide a handle-via-fields option. But defining it explicitly either way is fine too.
alienvalkyrie Reply To cazfi
Do we ever have a good reason to use handle-via-fields, from the POW of the quality / performance of the code produced? (I think it's mostly to make writing the handler function more convenient)
Wrt performance, I'd figure only for packets with no fields at all, but we could easily special-case that anyway; otherwise, handle-via-packet is probably more performant. (Unless the handlers get inlined across files, in which case it shouldn't matter.)
Wrt maintainability, handle-via-fields is more accessible to new contributors. It has all the relevant info right there, even if you're just looking at the source on github and thus don't have the generated files; and it means that when you change the packet, you cannot forget to also change the handler (since the signatures would no longer match up), which is helpful if you haven't fully figured out how we do networking yet (sidenote: maybe we should rename README.delta as README.networking?).
Wrt quality of the generation script, completely phasing out handle-via-fields would get rid of certain complications – there are probably still some field types that don't currently work with handle-via-fields (I've been meaning to look over everything, but haven't yet found the spoons to do so). If it was entirely up to me (and I was in the mood to adjust 96 handler functions), this is probably the route I'd take (with a special case for empty packets).
Reply To cazfi
If yes: I do see some value in that it "suggests" switching to handle-via-packet mode when you add more fields. Doing that switch is unlikely to occur to the coder at that point.
I thought about that, but I'm not sure if there's a good way to do it. Such a suggestion will very likely be buried under all the other compilation output. Unless it's an error that interrupts compilation, which would kinda defeat the point.
Tho it would be possible to finish writing the output files and only then raise the error, so that the script isn't run again next time. But that seems kinda hacky.
Reply To cazfi
If not: We could make handle-via-packet always the default (i.e. redundant to define explicitly), and just provide a handle-via-fields option. But defining it explicitly either way is fine too.
We could do that, altho as it stands, that would mean the minority of packets would follow the default.
If we do add both handle-via-packet and handle-via-fields flags, we'll probably want to always require exactly one of them (or no-handle); kinda like the direction flags. (Which really makes them not exactly flags anymore, but introducing new syntax in packets.def is a story for another time.)
Come to think about it; given that it would be sensible to report an error if no-handle and handle-via-packet are ever used together, it might be sensible to introduce advanced handling code for that anyhow, so this wouldn't even necessarily be added complexity.
Reply To alienvalkyrie
Reply To cazfi
If yes: I do see some value in that it "suggests" switching to handle-via-packet mode when you add more fields. Doing that switch is unlikely to occur to the coder at that point.
I thought about that, but I'm not sure if there's a good way to do it.
I mean the current behavior of implicit switch to handle-via-packet mode. You get the compilation failure from your handler function that refers to 'param1' and 'param2' instead of 'packet->param1' and 'packet->param2'
alienvalkyrie I think completely phasing out handle-via-fields (apart from the empty packet special case, if you wanna call it that) is actually the way to go. Reason being; if it already was that way, I doubt anyone would seriously consider it worth adding. So unless someone considers handle-via-fields an important feature, I'm going to be doing that.
alienvalkyrie Turns out AI code directly calls certain handle functions to do things. Might have to reconsider this after all (or use a whole bunch of compound literals).
alienvalkyrie So, here's the scoop:
So I decided to go with the third option: Making handle-via-packet the default (except for empty packets), with an opt-in handle-via-fields flag, and leaving the work of removing that flag where it might be appropriate for the future, e.g. when those packets are modified anyway, or when there's an acute reason to do so.
Reply To alienvalkyrie
So I decided to go with the third option: Making handle-via-packet the default (except for empty packets), with an opt-in handle-via-fields flag, and leaving the work of removing that flag where it might be appropriate for the future, e.g. when those packets are modified anyway, or when there's an acute reason to do so.
Yep, sounds like the best option we have.
alienvalkyrie 
If this feature already exist, it's not obvious from documentation in packets.def.
Sometimes it's annoying in development when you add a parameter to a packet, temporarily, for debugging purposes, and suddenly the handle function prototype changes (as parameter count goes over 5). Then you have to adjust all the code for that, and your two-line debug test comes much bigger diff to handle. For these cases it would be handy if one could tell generate_packets.py to just add new parameter to the handle -function despite their count going over 5.