Check out #854. It should be even faster by requesting only one usercmd_t per frame, and doesn't require ABI changes.

That's an interesting trick. Especially since it can be used right now by mods.

Anyway, something my patch does (it's better seen on game side), is to entirely remove the call to trap_GetCurrentCmdNumber. Would you have a way to cache trap_GetCurrentCmdNumber as well?

Notice that even if your patch has the advantage to don't break compatibility, this keeps the crufty design and we may want to clean-up the engine at some point. One good example is that function returns a bool. Actually no one code in game code makes uses of that bool. And if I may imagine there may have been people wanting that bool return, the need for that bool return is only motivated by the fact the code has to check on engine side that your request is properly done on game side. By requesting the whole array once for all, the code is not only simpler, but it totally removes the need for any check, and then totally removes the bool return.

OK I made a cache for the number too. Unvanquished/Unvanquished#2684

Oh I only read your code just now; I see it uses the same approach for caching the command number.

Notice that even if your patch has the advantage to don't break compatibility, this keeps the crufty design and we may want to clean-up the engine at some point. One good example is that function returns a bool. Actually no one code in game code makes uses of that bool.

Well if the cgame doesn't do any error checking, so much the worse for it. But then the engine doesn't try too hard to report errors, either. Right now we have negative command numbers requested at the beginning of the match, which is an error that goes undetected. But it was convenient to implement the cache the same way as in the engine, so I didn't touch it for the moment.

The current design may be inefficient (when used naively), but I don't see anything crufty about it.

Yeah, caching the number within a frame is easy to do (and to redo 😁️), but this still means two trapcalls are done per frame: trap_GetUserCmd and trap_GetCurrentCmdNumber. To only do one trap call I assume we have to add a trap_GetUserCmdWithNumber function (and break compat).

this at least would merit an overflow check, since I read "we assume it never decrease" well, yet it can.

this at least would merit an overflow check, since I read "we assume it never decrease" well, yet it can.

The case of the number resetting is already handled correctly UNLESS it resets less than 64 frames from the previous reset. Anyway the worst thing that can happen is wrong prediction.

ok then.

the worst thing that can happen is wrong prediction.

And basically a wrong prediction looks like a small lag from my experience at trying to optimizing this (and doing mistakes 😁️), so not a big deal.