TLK Bug - overuse of dStrcmp().

#1 Ps,
because it gives me a kick, here is how i "instrumented" dStrcmp():

int dStrcmp(const char *str1, const char *str2)
{
   static bool crazyDump = false;
   if      (!strcmp(str1, "TURNCRAZYDUMP_ON" ))
   {
      crazyDump = true;
   }
   else if (!strcmp(str1, "TURNCRAZYDUMP_OFF"))
   {
      crazyDump = FALSE;
   }

   if (crazyDump)
   {
      Con::errorf("crazydump! \"%s\" ?= \"%s\"", str1, str2);
   }

   return strcmp(str1, str2);   
}

then dumping all strings can be activated from script by issuing:

strcmp("TURNCRAZYDUMP_ON", "foo");

and off by issuing:

strcmp("TURNCRAZYDUMP_OFF", "foo");

#2 Interesting, thanks. I'll have to look into this, too...

#3 Nice one Orion!

Just took a quick look through the uses of sgGetLightingModel(const char *name) and I think your StringTableEntry assumption is valid - that version is only ever used with the sgLightInfo's sgLightingModelName which is a StringTableEntry.

#4 for those who are interested,
i did track down most of the remaining strcmp()s,
they were coming from dynamic_casts used here and there.
(on windows, dynamic_cast is apparently implemented via strmp().
tracking this down was a bit tricky:
i stepped into the disassembly for strmp() and then later placed a breakpoint there
and then stepped back up the call stack, invariably into a dynamic_cast.)

GuiShapeNameHUD had one dynamic_cast per ghosted object per frame,
getServerConnection(), getConnectionToServer(), and getLocalClientConnection() all had one,
GuiControl::getParent() and GuiControl::getRoot() each had one,
sgSetupZoneLighting() had a few,
and a custom routine called getAppropriateCustomAmbientLighting() had some.

there's actually hundreds more in the codebase,
but these seemed to be the ones which were getting called on a regular basis,
and eliminating them moved strcmp() out of the #1 or #2 spot in vTune to about #7.

i eliminated them through a sort of janky home-grown workaround
where i put a virtual method in SimBase for each class i'm interested in,
eg "virtual bool isClassPlayer() { return false; }",
and then override it in the actual class.
ala www.garagegames.com/community/resources/view/13244

#5 a little more on this -
it turns out there is a spot where a non-StringTable string was being passed to sgGetLightingModel(), but it's easily fixed:

sgUniversalStaticLight.cc

around line 387, change this line:

mLight.sgLightInfoData.sgLightingModelName = mDataBlock->sgLightingModelName;

to this:

mLight.sgLightInfoData.sgLightingModelName = StringTable->insert(mDataBlock->sgLightingModelName);

#6 note, Andy reports that this line isn't present in his TGE 1.5.2 version of TLK, which is quite possible. my codebase is TLK circa TGE 1.4.

#7 okay, we had a couple more gotchas show up here,
but they probably only affect folks with old versions of TLK.

the issue is that sgLightingModel::sgLightingModelName was a char[64], not a StringTableEntry.

fwiw, it's simple enough to convert it to a StringTableEntry,
but be aware that constructors like sgLightingModelAdvanced() run before the StringTable is initialized (because they're static instances and StringTable is a static pointer), so you'll want to add _StringTable::create() in the various constructors.

if this is too vague, please let me know and i can give more detail.

#8 I'm keeping an eye on this thread as i want to roll any relevant changes back into Torque 3D.

#9 Orion, thanks for the tip. I believe I got everything working ok, but I thought I should mention to make sure you add a check in _StringTable::create() that one hasn't already been created.

#10 hey Thomas -
ah, probably a good idea!

#11 Also, I just want to say that about half my todo list today more or less consisted of browsing your resources and TGE forum topics for lists of things that could be fixed or improved. Super helpful :)

#12 :D