Bug in console gui

#1 Welcome to the world of StringTable using TorqueScript exposed console functions.

#2 This bug does not exist with eval, nor does it exist when using enableWinConsole(1). This is specific to the gui object as far as I can tell. And no, I could not find where in the gui object that StringTable was doing this (though I suspected that first).

#3 Wow, this is messed up. I found the place in the code in the gui widget that actually pulls the value from the history of the commands entered. I was able to place a Con::warnf to see what was coming out and it does maintain the case for the command. However, by the time the Console::eval callback is called it has been converted to the stored value that matches the first call. So somewhere in between it is doing some sort of lookup. However I have yet to see anything resembling a StringTable insert. I did find some hash value being calculated, but it was related to the history (I think) and the history is pulling up the correct values.

So I did a check to make sure eval is working correctly:

eval("echo(\"hello\");");
eval("echo(\"Hello\");");

I put this code in a script file called testEval.cs and exec'd it with a button on the main screen. It spits out what it is supposed to spit out and does not seem to be putting anything in any kind of lookup.

#4 it exists in here:

$Con::warnVoidAssignment = false;

Investigated, and if you comment that, the echo "echos" case sensitive. However, you get that warning. Off to investigating the c++ now with that console variable.

Edit:

my question now becomes, how the heck is that making it messed up when its just doing an if check and then spewing out errors to the console.

case Namespace::Entry::VoidCallbackType:
  nsEntry->cb.mVoidCallbackFunc(gEvalState.thisObject, callArgc, callArgv);
  if( code[ ip ] != OP_STR_TO_NONE && Con::getBoolVariable( "$Con::warnVoidAssignment", true ) )
     Con::warnf(ConsoleLogEntry::General, "%s: Call to %s in %s uses result of void function call.", getFileLine(ip-4), fnName, functionName);
                        
  STR.popFrame();
  STR.setStringValue("");
  break;

That's the only place where its referenced. What the heck!?

#5 @jeff,
I am only seeing this through the gui console interface. No where else.

#6 Found the evil source of this bug.
guiTextEditCtrl.cpp,line 1503:

const char *GuiTextEditCtrl::getScriptValue()
{
   return StringTable->insert(mTextBuffer.getPtr8());
}

Basically this is adding a string to the StringInsert table every time you retrieve text from a control. However, it is not respecting case. Honestly I have no idea why you would want to insert everything into the StringTable. This seems like it could be a memory leak if there is a lot of text editing or field manipulation going on.

So my fix is thus:

const char *GuiTextEditCtrl::getScriptValue()
{
   return mTextBuffer.getPtr8();
}

If you really want it to be inserted into the string table:

const char *GuiTextEditCtrl::getScriptValue()
{
   return StringTable->insert(mTextBuffer.getPtr8(), true);
}

The true parameter preserves case information.

#7 Dude, TorqueScript is case-insensitive, what's the problem?



....

#8 @dB,
That should not effect the data. There is no reason to memoize the value returned from the GuiTextEditCtrl::getScriptValue. This is not about the console being case insensitive. It would still work. The issue is the data. Remember, eval does NOT do this. Only the GuiTextEditCtrl. This is not related to the console. It is only coincidence that that it is related to the widget that acts as the interface to the console. The data fed to the console is still treated case insensitively by the console. This is before the eval is even used on the command itself. Which occurs in the script callback command: ConsoleEntry::eval().

Here is where I found the problem:

// I have a callback that calls a Python function
MissionManager::loadMission(self, name)

// in the console gui control I type
MissionManager.loadMission("testmission");  // this fails because the m in the data needs to be capitalized.  It should be "testMission". 

// then I realize my mistake and type this
MissionManager.loadMission("testMission"); // this does not work still because the case is preserved based upon what I first typed in because case is ignored in the string table insert.

// but that stupid StringTable->insert sees it the first way like this
MissionManager.loadMission("testmission");

// the actual history that stores the command history when you type up and down stores the case correctly

Long story short this ONLY effects the actual GuiTextEditCtrl::getScriptValue(). This is used by the classes that derive from this functionality GuiConsoleTextCtrl. So this getScriptValue is memoizing everything that is ever extracted from a GuiTextEditCtrl object. Thus if your game uses a lot of fields and retrieve those values using that function either explicitly or implicitly you are building up a lot of memory usage for little to no gain IMHO.

At the very least if the memoize is desired it should be case sensitive so it does not f---...mess up your data at 3 am when you are testing database code. ;) I am keeping a log of bugs I find while doing my prototype and since this is THE interface for debugging the console this is really important. It should not be messing with the data entered at all.

Try doing an echo only changing case of the data in the string. Then do it from a script file. Different behavior.

#9 Sorry, I was being glib - great detective work going on here! I think me playing around with the console UI has contributed to just how I understand TS and the string table to work, so thanks for ending that illusion. This definitely does look like a bug if the behavior is different in script files versus through the console UI.

#10 There was some fiddling with this in T2D during the development of 3 Step Studio - we found that we didn't want the string table to be case-sensitive for that particular project. When a user named their tower (or whatever) "TowerOne" and later referred to it as "Towerone" we didn't want the user to be confused, or to have their project fail to work because of a typo. There was discusson on how to handle it because some of us also wanted the case-sensitivity - and then it wandered into "should we then try to correct the user if 'TowerOne' exists and 'towerOne' does not? What if he wants 'TowerOne' and 'towerOne' and 'Towerone' and 'ToWeRoNe' all to be distinct objects? (We sure hope not....)"

I don't recall what the final decision was, but I do remember these watercooler talks reverberating for something like two or three weeks....

#11 @dB,
You just showed me I had not explained the ramifications of this bug very well. So it is good you challenged my assertion that this is a bug. If this were something in the console itself I would have rethought trying to fix it.

@Richard,
I wonder if anyone has developed poor programming techniques because TS is blind to case? I could see someone deciding to change the case of an object or parameter depending upon how it is being used. It could actually make some code more readable, but would not carry over to very many programming languages.

When I get a chance I will submit this via a pull request. I have a bunch of other "tweaks" that I am finding in the engine. The project I am working on is unique in that I am splicing ScriptT3D into the engine and it is not always pretty. There are some things I just have no idea about. Like why do some objects give me the proper ID when I "eval" them and why do some not?
Example Python interface:

# SimObject
Con.eval("new SimObject();") # works fine
# ArrayObject
Con.eval("new ArrayObject();") # fails to return id
# work around
Con.eval("$obj = new ArrayObject(); return $obj;");  # works for all objects it seems.

All I can guess is something is slightly different in the return call for the ID of some objects versus others. I just chalked it up to being lucky to get the ID for some objects in the first place. This observation is not a bug, just something that makes you say, "hmmmmmm". Eventually I will create a native object creation function in C++, I am just focusing on getting my proto done for now. I am taking lots of notes on the engine though. Might fill up some doc pages somewhere.

#12

Quote:It could actually make some code more readable, but would not carry over to very many programming languages.

An interesting use case is in server/client commands. You can declare them in camel case (serverCmdMyThing(...)) and call them in camel case minus the prefix (commandToServer(myThing ...)). However, I think that's a poor reason to get rid of case sensitivity. We should have new function syntax for server commands (servercmd myThing(...)) instead.

#13 @Daniel

or maybe ServerCommand::MyThing() {}I thought TS had namespacing, why not use it?

#14 I'm betting the namespace thing didn't get used due to a perceived issue with networking that properly. I don't know if there would be an issue because I haven't done the homework on it but there might be a different can of worms to untangle on that route.

Personally I think it would be fairly intuitive if you could use Client:: and Server:: namespaces to define and call these.