Use ordered id for netvars instead of shitty hash
This superscedes !980 (closed) and addresses concerns over demo compatibility.
-
Master
Hold on, this doesn't address my concern regarding demo compatibility. It maintains compatibility with demos that have already been created, but writes new demos that will have the same issue outlined in my comment on the previous merge request.Quoted for comparison:I do not believe it is a solid approach. Specifically, it prevents replay compatibility between minor versions that add or remove cvars, whether that's versions of Lua mods or the base game in the case of Kart's netreplays, in a way that silently fails càstrophically.
You have addressed compatibility with old versions, but you have not addressed compatibility between subsequent patches/mods that introduce additional netvars. -
Master
New demo files should use plain cvar names, if it's not the case already.
-
Master
...somehow I misread the diff initially, and was just about to place a new comment when you explained it yourself. Thanks for pointing it out; however, I'm confident I've read the diff properly this time and although I see where the name is read from demos, I don't see any place where it is written to them. As of 2.2.0 it used
CV_SaveNetVars()
, which writes the netid regardless of context, and I don't see that replaced anywhere in this diff. How is that handled? -
Master
Oh, I apologise! Your comment had the vibe of having read the diff more thoroughly than I, not as a statement of what an ideal solution would be doing.
-
Master
I am way too lazy to read diffs. :P
-
Owner
I really FORGOT
-
Master
And this is why you always need to read diffs thoroughly, even if it's boring.
-
Owner
Thank you for being the responsible one...
-
150 150 151 151 #undef DEALIGNED 152 152 153 #define WRITESTRINGN(p,s,n) { size_t tmp_i = 0; for (; tmp_i < n && s[tmp_i] != '\0'; tmp_i++) WRITECHAR(p, s[tmp_i]); if (tmp_i < n) WRITECHAR(p, '\0');} 154 #define WRITESTRING(p,s) { size_t tmp_i = 0; for (; s[tmp_i] != '\0'; tmp_i++) WRITECHAR(p, s[tmp_i]); WRITECHAR(p, '\0');} 155 #define WRITEMEM(p,s,n) { memcpy(p, s, n); p += n; } 153 #define WRITESTRINGN(p,s,n) do { size_t tmp_i = 0; for (; tmp_i < n && s[tmp_i] != '\0'; tmp_i++) WRITECHAR(p, s[tmp_i]); if (tmp_i < n) WRITECHAR(p, '\0');} while (0) -
Master
Before I plusone this branch, what do the changes in this commit do? Does it attempt to run the block once and then exit? Why does this require the
do while (0)
structure rather than the previously existing curly brace context? -
Owner
Only one statement can follow
if
,while
etc. You know how you don't need a semicolon after a block? That whole block can basically be considered a single statement.So here's an example:
if (in_demo) WRITESTRING(*p, cvar->name); else
So with what the macros were before, that would turn into this:
if (in_demo) { ... } ; else
That's basically a block followed by an empty statement. And now the
else
doesn't follow anif
statement. It's just hanging there and that generates a compiler error too.if (in_demo) do { ... } while (0); else
In C,
do while
does require a semicolon to end it, so it's a single statement and everything is fine.Bonus: You can turn those blocks into single statements by surrounding them with parenthesis too. The
READ
macros do this. -
Master
bruh
But it works, so plusoned.
-
-
merged
Toggle commit list