Weird "Opcode " error when comparing strings

[OneManClan]

Hey people,

I'm trying to compare a string variable to a string constant:

Code: Select all

//============ VERSION 1:

local string str;

if (str == "John"          
   
      ||   "Paul"         
      ||    "George"         
      ||    "Ringo"         
      ||    "Robert"      
      ||    "Jimmy"      
      ||    "JohnPaul"      
      ||    "Bonham"      
      ||    "Gene"      
      ||    "Ace"      
      ||    "Angus"      
      
      )

The above syntax is wrong because apart from the initial John comparison, after the first || the code asks if the strings exist (ie it's not comparing them with str). The solution should be to put a bracket around all the arguments, as follows:

Code: Select all

//============= VERSION 2:

local string str;

if (str == ("John"          
   
      ||   "Paul"         
      ||    "George"         
      ||    "Ringo"         
      ||    "Robert"      
      ||    "Jimmy"      
      ||    "JohnPaul"      
      ||    "Bonham"      
      ||    "Gene"      
      ||    "Ace"      
      ||    "Angus"      

      )
      )



This SHOULD work, but now fteqccgui gives the error:

Code: Select all

error: Opcode "<CIF>|C_ITOF" not valid for target

What does this mean?

thanks,

OneManClan

[Spike]

There is no sane || operator that can accept two strings.

("fred" || "barney")
makes no sense.
If you were programming in C, it would always evaluate to true. Which makes it pointless.
The fact that you have an equality operator outside of the brackets makes no difference.
Computer logic is more rigid than hunam logic, which is actually fairly vauge and ambigous.

There is no operator that says: if( a is (b, c, d, e, or f)).
You must explicitly compare each string.
if (str == "b" || str == "c" || str == "d" || str == "e")
Note however, that QC will not early-out. It will check each and every single comparison, even if an earlier one was already true.
You can explicitly request that it early out, but doing so can result in changed behaviour, including infinite loops in player animations. Its some compiler option, I don't remember its name but it should be listed in the fteqccgui options. Even so, each of the earlier strings will be checked in turn.


if you're okay with fteqcc specifics, you can try a switch statement.
switch(str)
{
case "Paul":
case "George":
case "Ringo":
case "etc":
mycode();
break;
default:
whattodoifnonematch();
break;
}

Tbh, I'm not sure if I ever tested that form... but hey.
Even with this, it'll check each case from the first to the last. It will early out, at least, so try to put the most common case at the top of the switch block.

[OneManClan]

There is no operator that says: if( a is (b, c, d, e, or f)).
You must explicitly compare each string.
if (str == "b" || str == "c" || str == "d" || str == "e")

Thanks Spike. I guess I was trying to be 'efficient', possibly needlessly, as my grasp of exactly how a CPU processes instructions is tenuous. ie code that looks (to me) clunky/laborious in the QuakeC might actually run faster than something that looks simpler/cleaner.

Q: Is the 'explicit comparison' (as above) method any faster/slower than a switch (as below)? All I'm doing is setting a flag, so a simplified version could be:

switch(str)
{
case "Paul":
case "George":
case "Ringo":
case "John":
self.member_of_beatles = 1;


}

This actually compiled fine, will test later today.


thanks!

OneManClan

[gnounc]

who's robert?
How'd ace get in but paul missed the boat?
And wheres bonn?

[Spike]

a switch statement as implemented in fteqcc is a series of comparisons. as soon as one comparison is true, it'll jump to the code. ie:
if (str == "foo") goto blob;
if (str == "foo2") goto blob;
if (str == "foo3") goto blob;
goto defaultorendofblob;
(each line is an OP_EQ_S and an OP_IF, last line is an OP_GOTO)
(note that if 'str' is actually ent.field then it is evaluated in advance, and is a 1-time extra OP_LOAD_S instruction)

a series of comparisons separated by an || operator will evaluate the entire set of comparisons, even if an earlier comparison is sure to result in the branch being taken or not taken.
ie:
temp1 = (str == "foo");
temp2 = temp1 || (str == "foo2");
temp3 = temp2 || (str == "foo3");
if (!temp3) goto endofblob;
(first line is an OP_EQ_S, second line is an OP_EQ_S and an OP_OR, last line is an OP_IFNOT)
(note that if 'str' is actually ent.field then that's an additional instruction per instance of 'str')

In a normal CPU, reducing the number of branches is considered good. However, string comparisons are a series of branches in themselves, and QC instructions are an order of magnitude slower than real cpu instructions. As far as optimising in QC goes, the priorities are to reduce expensive builtins called (traceline is an example), then to reduce total QC instructions executed (cache usage is less important to QC than to native instructions, so quantity/locality of instructions is also less important), and then to keep string comparisons low, this is a fairly low priority as they generally yield results in the first two chars or so, but they are potentially the slowest single instruction, especially if the strings do actually match.


If you are especially curious as to optimisations and opcodes generated, the -Fwasm flag to fteqcc (should be an option in the gui) can be fairly informative - its quite useful to debug qc compiler bugs. It prints out the opcodes present in each function, and their various arguments and values. If you want to try some extreeme qc coding, fteqcc is meant to accept the asm syntax as found in this file, but that doesn't mean it can correctly compile only this file.

[metlslime]

In a normal CPU, reducing the number of branches is considered good. However, string comparisons are a series of branches in themselves, and QC instructions are an order of magnitude slower than real cpu instructions... ...and then to keep string comparisons low, this is a fairly low priority as they generally yield results in the first two chars or so, but they are potentially the slowest single instruction, especially if the strings do actually match.

Since strings in qc are all immutable, and the string variable itself is just an index into the string table, aren't string comparisons just index comparisons? Or does quake really use the standard C string compares for this?

[andrewj]

Or does quake really use the standard C string compares for this?

from pr_exec.c

Code: Select all

        case OP_EQ_S:
                c->_float = !strcmp(pr_strings+a->string,pr_strings+b->string);
                break;


I guess string references cannot be directly compared due to the buffer used by builtins like ftos().

[Spike]

if the address matches, you know its the same string, if the address doesn't match, it may still match.
remember classnames from the bsp and stuff.

[OneManClan]

Thanks Spike for your detailed info - though it will take some time (+homework) before I understand it fully.

I *can* confirm that the following works (fteqcc):

Code: Select all

switch(str)
{
case "Paul":
case "George":
case "Ringo":
case "John":
 self.member_of_beatles = 1;

}


You've since told me that: "writing QC code carefully to avoid extraneous instructions takes lots of time.... personally I would say that if its not going to be called 500 times a second, readability beats performance, every time." So I'll assume/conclude that the above code runs (for all practical purposes) exactly as fast as:

if(str == "Paul" || str == "George" || str == "Ringo" || str == "John" ||)
self.member_of_beatles = 1;


thanks,

OneManClan
ps gnounc:
1. robert = Robert Plant
2. We already had a Paul, and I didn't want to call the Starman Paul2
3. Bonn is in Germany.

[frag.machine]

Since strings in qc are all immutable, and the string variable itself is just an index into the string table, aren't string comparisons just index comparisons? Or does quake really use the standard C string compares for this?

In the following case, you are indeed correct:

Code: Select all

if ("george" != "ringo")
(...)


However, comparison against local or global variables are different:

Code: Select all

local string beatle = "john";
if (beatle != "ringo")
(...)


[mh]

if the address matches, you know its the same string, if the address doesn't match, it may still match.
remember classnames from the bsp and stuff.

Well not really. You could for example have something like:

Code: Select all

char blah[256] = {0};
ent->v.somestring = pr_strings - blah;

strcpy (blah, "some text");
PR_ExecuteProgram (ent->v.touch);

strcpy (blah, "some different text");
PR_ExecuteProgram (ent->v.touch);

The addresses will match inside both touch functions but the strings are clearly different (and blah could even be a local, to further complicate things).

If the string comes from the strings table I do believe that it is valid to assume equality on a matching address though. Unless you start overwriting the strings table in the engine, in which case all bets are off.

[Spike]

strcmp(blah, blah) == 0.
they're the same, even though they may not meant to be

[mh]

I think we're talking about different things again...

[Spike]

at the instruction level its already too late.

[OneManClan]

I just realised, although the 'real world' difference might be meaningless, technically speaking the following code should run faster:

switch(str)
{
case "Paul": self.member_of_beatles = 1; break;
case "George": self.member_of_beatles = 1; break;
case "Ringo": self.member_of_beatles = 1; break;
case "John": self.member_of_beatles = 1;

}

Looks messier, but by providing a 'break' in every case, we don't have to process every single comparison, unless (as in this example) the players name is "John".