fix compression packet handling for q2repro#6
Conversation
res2k
commented
Jan 25, 2026
res2k
commented
- ensure we write svc_q2repro_zdownload, rather than svc_r1q2_zdownload
- handle deflate end so the stream resumes
- ensure we use context->zpacket_cmd rather than hard-coding svc_r1q2_zpacket, which was incorrect
- ensure we write svc_q2repro_zdownload, rather than svc_r1q2_zdownload - handle deflate end so the stream resumes - ensure we use context->zpacket_cmd rather than hard-coding svc_r1q2_zpacket, which was incorrect
|
@jackharrhy, I just stumbled upon those changes in your fork. Those look like legit bugs, so I took the liberty to open this PR. |
| // FIX(jackharrhy) q2protoio_deflate_get_data finishes the stream, | ||
| // so we need to end it and mark as invalid for the next packet | ||
| // (which will create a new stream) | ||
| q2protoio_deflate_end(state->deflate_io); |
There was a problem hiding this comment.
Hmm, one issue I have with this bit is that this basically means that each chunk of compressed download data is it's own, complete deflate stream; but I think the intention was to allow streams that span multiple packets.
(IIRC original q2pro does it like that for zdownloads).
It's also possible that q2repro's q2protoio_deflate_get_data() needs adjustment, or that some more communication between q2protoio_deflate_get_data() and it's callers are required - perhaps a return code that implied "end deflating".
(Although, this could also be tracked in the q2protoio_ implementation, which would mean addressing this in q2repro.)
There was a problem hiding this comment.
this was the biggest reason i didn't really bother considering upstreaming this, as while this made things work again, it didn't feel like a great solution
There was a problem hiding this comment.
Fair enough.
But the command fixes are already valuable, so I pulled those out and pushed them.
For now, I'd leave this PR open for discussion or perhaps future updates.
There was a problem hiding this comment.
@jackharrhy So I arrived at a solution...
At it's core, the issue was that q2protoio_deflate_get_data() was really used for different purposes in different cases - mostly "one-shot" compression (finish immediatelty), but in the problematic code above, it was "streaming" compression (finish eventually).
To address this I added, well, an additional argument to indicate whether compression is to be "finished" (b2b4407).
Of course this also required changes in q2repro - see https://github.com/res2k/q2pro/commits/rerelease-game/; so you'll probably need to merge that should you update q2proto.
2 participants
