Game Development Community

[Need Help] - Optimizing an 'IF' statement in TorqueScript

by Nicolai Dutka · in General Discussion · 03/17/2010 (4:28 pm) · 12 replies

I need help optimizing a small chunk of code. I am sure there is a better way to write this, but I never learned one:

if(%name !$= "blockerObjectTemplate" &&
       %name !$= "constructoObjects" &&
       %name !$= "destructoObjects" &&
       %name !$= "doors" &&
       %name !$= "flammablesTemplate" &&
       %name !$= "freezablesTemplate" &&
       %name !$= "shockablesTemplate" &&
       %name !$= "telekineticObjects")

Self explanatory I guess, just looking for a way to remove the "%name !$=".

Thanks!!

#1
03/17/2010 (7:21 pm)
Could use switch$ instead :

switch$ (%name)
{
    case "blockerObjectTemplate" :
    case "constructoObjects"     :
    case "destructoObjects"      :
    case "doors"                 :
    case "flammablesTemplate"    :
    case "freezablesTemplate"    :
    case "shockablesTemplate"    :
    case "telekineticObjects"    : break;
    default                      : <....code ....>
}

would have the same effect.

However, I tend to avoid string comparisons if at all possible as they are slow. Have you thought about assigning a 'type' variable to your objects instead to use as the check?

e.g. a variable called objType set to 1 for blockerObjects, 2 for constructoObjects, etc

Just a thought
#2
03/17/2010 (7:24 pm)
The switch makes sense. Let me post some more of the code so you can see more of what I am looking for:


function updateDatablockMenu(%type)
{
  // So, we have to update the datablocks drop-down.
  // Purge the lists first...
  %list = %type@"_Data_List";
  %list.clear();

  // Get the Selection of the Types drop-down menu.
  %type = getFolder(%type);

  // Plug in the path
  %path = "scripts/server/" @ %type @ "/";
  
  // Initialize the count
  %count = 0;

  // Set our string pattern
  %stringPattern = %path @ "*.cs";
  
  if(%type $= "emitters"){
      %fileObject = new FileObject();
      if(!%fileObject.openForRead("scripts/server/EmitterList.cs")){
        %fileObject.close();
        %fileObject.delete();
      }
      %emitterCount = %fileObject.readLine();
      for(%i=0; %i<%emitterCount; %i++){
        %list.add(%fileObject.readLine(),%count,0);
        %count++;
      }
      %fileObject.close();
      %fileObject.delete();
      %list.sort();
      return;  // If we're using this part of the code, we don't need the rest
  }

  for (%file = findFirstFile(%stringPattern); %file !$= ""; %file = findNextFile(%stringPattern))
  {             
    %name = fileBase(%file);
    if(%name !$= "blockerObjectTemplate" &&
       %name !$= "constructoObjects" &&
       %name !$= "destructoObjects" &&
       %name !$= "doors" &&
       %name !$= "flammablesTemplate" &&
       %name !$= "freezablesTemplate" &&
       %name !$= "shockablesTemplate" &&
       %name !$= "telekineticObjects")
      {
        echo (%count @ ": " SPC %file);
        if(%type $= "statics" || %type $= "destructo"){
            %fileObject = new FileObject();
            if(!%fileObject.openForRead(%file)){
              %fileObject.close();
              %fileObject.delete();
            }
            %list.add(fileBase(%file),%count,0);
            %fileObject.close();
            %fileObject.delete();
        }
        else{
              %list.add(fileBase(%file),%count,0);
        }
        %count++;
      }
  }
}
#3
03/17/2010 (7:29 pm)
This should do it.

Replace lines 40 to 64 with this :

switch$ (%name)  
{  
    case "blockerObjectTemplate" :  
    case "constructoObjects"     :  
    case "destructoObjects"      :  
    case "doors"                 :  
    case "flammablesTemplate"    :  
    case "freezablesTemplate"    :  
    case "shockablesTemplate"    :  
    case "telekineticObjects"    : break;  
    default                      :   
            echo (%count @ ": " SPC %file);  
            if(%type $= "statics" || %type $= "destructo"){  
                %fileObject = new FileObject();  
                if(!%fileObject.openForRead(%file)){  
                  %fileObject.close();  
                  %fileObject.delete();  
                }  
                %list.add(fileBase(%file),%count,0);  
                %fileObject.close();  
                %fileObject.delete();  
            }  
            else{  
                  %list.add(fileBase(%file),%count,0);  
            }  
            %count++;
}

Not sure if that's any faster, better or more readable though ....
#4
03/17/2010 (7:37 pm)
It's the same 'amount' of code... What I am looking for is something that either condenses the code or makes the code run faster in the engine...

If what I have is about as condensed and 'fast' as I am gonna get, that'll have to do. Although I prolly will go with a 'switch' if nobody has any better ideas....
#5
03/17/2010 (8:54 pm)
You could always replace it with a string search.

First, initialize:
// define this once well beforehand, preferably only once
$DatablockTypes = "blockerObjectTemplate,constructoObjects,destructoObjects,doors,flammablesTemplate,freezablesTemplate,shockablesTemplate,telekineticObjects";

Then, this is your new if:
// then your if looks like this:
if (strpos($DatablockTypes,%name) < 0)
{
   // not found, do what you will
}
#6
03/17/2010 (11:47 pm)
your code cant be optimized much due to the fact that youre using a string as the identifier. you could do this more efficiently using an enumerated list or bit flags.
#7
03/18/2010 (1:48 am)
I think that the 'break' statment from the switch structure isn't needed and isn't supported in TorqueScript.

Jaimi's solution is the simplest and the more maintainable.

Nicolas Buquet
www.buquet-net.com/cv/
#8
03/18/2010 (2:41 am)
Yep, Jaimi's solution seems to be the best. :D
#9
03/18/2010 (1:15 pm)
You can definately simplify the code by using TorqueScript's own string functionality.

%BadWords = "blockerObjectTemplate constructoObjects destructoObjects doors flammablesTemplate freezablesTemplate shockablesTemplate telekineticObjects";

for (%file = findFirstFile(%stringPattern); %file !$= ""; %file = findNextFile(%stringPattern))   
  {               
    %name = fileBase(%file);  
   
    if (StringHasWord(%BadWords, %name)
    {
      %list.add(fileBase(%file),%count,0);  
    }
    //string had no bad words
    else
    {
      //do the right thing
    }

The above relies on the the following function:

function StringHasWord(%String, %Word)
{
  for (%WordIndex = 0; %WordIndex < GetWordCount(%String); %WordIndex++)
  {
    //Echo("comparing" SPC GetWord(%String, %WordIndex) SPC "to" SPC %Word);

    if (GetWord(%String, %WordIndex) $= %Word)
    {
      return true;
    }
  }

  return false;
}

The performance may suffer a bit but it's far simpler to read and understand.
#10
03/18/2010 (4:34 pm)
I like Jaimi's solution too, that is EXACTLY what I was thinking, I just never learned how to do it in TS...

@Nicolas, 'break' does work in TS, I've used it before. ;)
#11
03/19/2010 (1:20 am)
@Nicolai : you can put "break;" statement at the end of your 'case' block, but they are superflous.

From Ed Maurina docs :
Quote:Note: Unlike C/C++, the break statements in switches are superfluous. Torque Script will
only execute matching cases and NOT automatically execute all subsequent cases. This is
shown in the example below.

Just verified here on T3D 1.0.1 :

function test( %val )
{
	switch( %val )
	{
		case 0 : echo("is Zero."); break;
		case 1 : echo("more than 0"); break;
		case 2 : echo("more than 1"); break;
		case 3 : echo("more than 2"); break;
		case 4 : echo("more than 3"); 
		case 5 : echo("more than 4"); break;
		default : echo("more than all"); break;
	}
}

A call to 'test(4)' won't put "more than 3" line AND "more than 4" line. It will just put the the first line. The 'break;' statement is implicit in 'case' block in TorqueScript.

Nicolas Buquet
www.buquet-net.com/cv/
#12
03/19/2010 (8:35 am)
@Nicolas

Thanks for that tidbit! Saves me a little typing here and there. :P