fitzquake, r_nolerp_list parsing

[szo]

Noticed while reading Baker's post at http://celephais.net/board/view_thread. ... &start=538 the r_nolerp_list parsing of fitzquake is rather broken, see Mod_SetExtraFlags() in gl_model.c:

Code: Select all

   extern cvar_t r_nolerp_list;
   const char *s;
   int i;
[....]
   // nolerp flag
   for (s=r_nolerp_list.string; *s; s += i+1, i=0)
   {
      //search forwards to the next comma or end of string
      for (i=0; s[i] != ',' && s[i] != 0; i++) ;

      //compare it to the model name
      if (!strncmp(mod->name, s, i))
      {
         mod->flags |= MOD_NOLERP;
         break;
      }
   }


The loop would go beyond the null terminator of r_nolerp_list.string (besides, due to strncmp(), it technically would yield a false positive, but much less likely). In quakespasm, I'll replace it with the following, shortly, which is, IMO, more conservative and cautious:

Code: Select all

   extern cvar_t r_nolerp_list;
   const char *s;
   char tmp[MAX_QPATH];
   int i;
[....]
   // nolerp flag
   s = r_nolerp_list.string;
   while (*s)
   {
      //make a copy until the next comma or end of string
      i = 0;
      while (*s && *s != ',')
      {
         tmp[i] = *s;
         i++;   s++;
      }
      tmp[i] = '\0';
      //compare it to the model name
      if (!strcmp(mod->name, tmp))
      {
         mod->flags |= MOD_NOLERP;
         break;
      }
      //search forwards to the next comma or end of string
      while (*s && *s == ',')
         s++;
   }


[Baker]

I think I managed to turn the above function into 5-6 lines of code. I passed the cvar as a string, the "search for model" as string and the delimitor as string (so I could reuse the function for something else in the future). I made a copy of the string with the delimiter snprintf ("%s%s%s", delimiter, cvar_string, delimiter) and did the same for the search string. Then did a strstr or what not, returned 0 or 1 and let the "new map" function handle whether or not to assign the model flag.

Slightly inelegant for sure, but the shortness of function made sure I didn't have to spend any time debugging it.

I guess I did that because I didn't see why that function had to be so complex to begin with. It really is just looking to see if the model is in a comma delimited list.

I used metlslime's "special models" cvar idea for a few things. Like to allow special texture effects for surfaces (reflective-ish sphere-mapish, metallic-like, fence texture alpha mask style, additive blend) so all the cool stuff from Kurok would not have to be hard coded in engine code and could be more ... what's the word ... not hardcoded hacks but exposed to mod creator. (Without altering the model format to accommodate extra flags or doing anything to the .bsp)

[Yeah, yeah ... I still haven't released the engine work but I sure want to. But not till its done right and complete how I'd like to be. I went all in with some of the ideas I got from DirectQ's coding style, ideas from Quakespasm and some of the stuff Spike talked about being Q1 engine deficiencies solved in FTE. Sigh ... time ...]

[szo]

Applied the fix to quakespasm in a slightly modified way:
http://quakespasm.svn.sourceforge.net/v ... vision=517
... will be part of v0.85.5.

[mh]

I'd agitate for this string to be treated the same as GL_EXTENSIONS; i.e. a pretty standard space-delimited string that you can query with strstr. I've quite an intense dislike for lots of different textual formats each of which needs it's own custom parser.

Of course, that wouldn't be compatible with Fitz as it stands, but them's the breaks.

[szo]

I'd agitate for this string to be treated the same as GL_EXTENSIONS; i.e. a pretty standard space-delimited string that you can query with strstr.
[...] Of course, that wouldn't be compatible with Fitz as it stands,

Well, that's the reason I didn't touch the functionality but only fixed.

[metlslime]

I'd agitate for this string to be treated the same as GL_EXTENSIONS; i.e. a pretty standard space-delimited string that you can query with strstr. I've quite an intense dislike for lots of different textual formats each of which needs it's own custom parser.

Of course, that wouldn't be compatible with Fitz as it stands, but them's the breaks.

You could probably replace all commas with spaces at parse time, then parse it as a space-delimited string.

BTW thanks for finding the flaws in this code... whoops...