Game Development Community

GuiTextEditCtrl bugfix- set Variable before evaling Command

by Orion Elenzil · in Torque Game Engine · 02/06/2007 (5:03 pm) · 4 replies

Edit: this post is for TGE 1.3.5. see a few posts down for 1.4+.

This one has bit me and others on my team a few times,
not sure why it took me so long to realize that i could just Fix it instead of working around it.

what's wrong with this GuiControl setup code ?
new GuiTextEditCtrl() {
   // blah blah blah
   variable = "$MyAwesomeSetting";
   command = "ApplySetting($MyAwesomeSetting);";
   // blah blah blah
};


.. nothing's wrong with it, right ?

right !

the problem is that when you modify its contents,
GuiTextEditCtrl executes the "command" before updating the "variable",
so what ApplySetting() actually sees the *previous* value of $MyAwesomeSetting.

fixed by assigning the variable first.
also, whoever wrote this code in the first place did a kinda janky job in that they duplicated code all over the place. this is bad practice because it means if someone comes along later and wants to improve the functionality, they have to 1) figure out that this functionality is duplicated all over the place and 2) duplicate their improvements. I chose a third option: 3) refactor to remove the code duplication.

GuiTextEditCtrl.h
add this near the bottom:
void onContentsUpdated();

GuiTextEditCtrl.cc
add this wherever you please; i chose right above "void GuiTextEditCtrl::setFirstResponder()
void GuiTextEditCtrl::onContentsUpdated()
{
   // Update the console variable!
   if (mConsoleVariable[0])
      Con::setVariable(mConsoleVariable, mText);

   // Execute the console command!
   if ( mConsoleCommand[0] )
   {
      char buf[16];
      dSprintf( buf, sizeof( buf ), "%d", getId() );
      Con::setVariable( "$ThisControl", buf );
      Con::evaluate( mConsoleCommand, false );
   }
}

still in GuiTextEditCtrl.cc,
replace all instances of these lines:
(in my codebase there were five instances)
// Execute the console command!
if ( mConsoleCommand[0] )
{
   char buf[16];
   dSprintf( buf, sizeof( buf ), "%d", getId() );
   Con::setVariable( "$ThisControl", buf );
   Con::evaluate( mConsoleCommand, false );
}

// Update the console variable!
if (mConsoleVariable[0])
   Con::setVariable(mConsoleVariable, mText);

with this:
onContentsUpdated();

#1
02/07/2007 (12:29 am)
In 1.5 it is only implemented at one place, no copy paste implementation anymore.
The function is called: "execConsoleCallback()" and looks similar to the one you "fixed" before fix (with the addition of UTF support
This means still exec, then UTF conversion then variable set.

Perhaps thats "wanted" for some reason

EDITED The posting might have been missleading to believe that TGE 1.5 looks like the fixed instead of the unfixed.
#2
02/07/2007 (7:37 am)
Huh, thanks for the info Marc.
edited my post to reflect.
i'll take a look at the order-of-operations in 1.5 when i get into work.
#3
02/07/2007 (9:17 am)
Thanks to Marc,
here is the fix for TGE 1.4+ :

in GuiTextEditCtrl.cc
change this:
void GuiTextEditCtrl::execConsoleCallback()
{
   // Execute the console command!
   if ( mConsoleCommand[0] )
   {
      char buf[16];
      dSprintf( buf, sizeof( buf ), "%d", getId() );
      Con::setVariable( "$ThisControl", buf );
      Con::evaluate( mConsoleCommand, false );
   }

   // Update the console variable:
   UTF8 buffer[1024];
   mTextBuffer.get(buffer, 1024);

   if ( mConsoleVariable[0] )
      Con::setVariable( mConsoleVariable, (char*)buffer );
}

to this:
void GuiTextEditCtrl::execConsoleCallback()
{
   // Update the console variable:
   UTF8 buffer[1024];
   mTextBuffer.get(buffer, 1024);

   if ( mConsoleVariable[0] )
      Con::setVariable( mConsoleVariable, (char*)buffer );

   // Execute the console command!
   if ( mConsoleCommand[0] )
   {
      char buf[16];
      dSprintf( buf, sizeof( buf ), "%d", getId() );
      Con::setVariable( "$ThisControl", buf );
      Con::evaluate( mConsoleCommand, false );
   }
}

no need to touch the .h file or make new functions.
thanks again, Marc.


does anyone know a reason why we would *not* want the variable updated before executing the command ?
#4
02/16/2007 (3:11 pm)
Nice one.

btw, why we (processor) need to work with "buffer" if there is no mConsoleVariable? (i'm about 1.4 and 1.5 routine, seems to be okay in 1.3x)
I prefer to make it looking like this:
// Update the console variable:
   if ( mConsoleVariable[0] )
   {
      UTF8 buffer[1024];
      mTextBuffer.get(buffer, 1024);
      Con::setVariable( mConsoleVariable, (char*)buffer );
   }
instead of
// Update the console variable:
   UTF8 buffer[1024];
   mTextBuffer.get(buffer, 1024);

   if ( mConsoleVariable[0] )
      Con::setVariable( mConsoleVariable, (char*)buffer );