Jump to content

  • Log In with Google      Sign In   
  • Create Account

how to use or ||


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
13 replies to this topic

#1 FGFS   Members   -  Reputation: 196

Like
0Likes
Like

Posted 06 July 2014 - 10:24 AM

Hi

this seem to be the trouble:

                                    if (rainP <= 0.3 || temperatureC < 0 || cloudAlt <= altibaro){
                                            alSourceStop(Sources[0]);
                                    }else{
                                        alSourcePlay(Sources[0]);
                                    }

I want to stop the sound if either rain is less 0.3 or temp below zero or cloud altitude is below.

But the sound keeps playing even if temp is below zero. Am I doing something wrong and

how to write the above?

Thanks

 



Sponsor:

#2 Buckeye   Crossbones+   -  Reputation: 4902

Like
4Likes
Like

Posted 06 July 2014 - 10:36 AM

Not sure if it's the problem, but always collect boolean terms in parentheses:

if ( (rainP <= 0.3) || (temperatureC < 0) || (cloudAlt <= altibaro) ){ ...

Calculation order depends on operator priority. That is, if " || " has a higher priority precedence than " <= ," you'll get strange results.

 

EDIT: Taking a second look, what values are you getting for those terms when you debug? Perhaps a better approach if your code appears correct: if you're not getting the results you think you should, you should look at the actual data, rather than guessing/hacking at code. If the data is correct, then fix the code. I.e.,

bool rain = rainP <= 0.3;
bool temp = temperatureC < 0;
bool cloud = cloudAlt <= altibaro;
if( ... ) { ... }

Ninja'd by (of course) unbird. biggrin.png


Edited by Buckeye, 06 July 2014 - 03:25 PM.

Please don't PM me with questions. Post them in the forums for everyone's benefit, and I can embarrass myself publicly.


#3 unbird   Crossbones+   -  Reputation: 4974

Like
8Likes
Like

Posted 06 July 2014 - 10:39 AM

That should actually work (with the operator precedence there should be no confusion, so no brackets needed).
 
There must be something else off (maybe the sound switch off?). Go through with your debugger and inspect those values.
 
As a general hint: If you don't know/trust what's going on, declare a couple of temporary variables (or even functions), and inspect those, e.g:
 
bool b1 = rainP <= 0.3;
bool b2 = temperatureC < 0;
bool b3 = cloudAlt <= altibaro;
bool b = b1 || b2 || b3;

Edit: Thinking about it, using parentheses explicitly (even if not needed) isn't that bad. Not only for readability.

Edit2: @Buckeye: Question now is rather who has ninja'd whom ? wink.png

Edited by unbird, 06 July 2014 - 10:48 AM.


#4 Wooh   Members   -  Reputation: 622

Like
1Likes
Like

Posted 06 July 2014 - 11:20 AM

Do you run this code repeatedly while the sound is playing?

#5 aregee   Members   -  Reputation: 1022

Like
0Likes
Like

Posted 06 July 2014 - 03:45 PM

Check your values like Buckeye and unbird suggests, also Wooh is hinting at something here:

 

Why don't you make a flag, a boolean telling if the sound is playing or not, and track state changes rather than be calling playsound or stopsound on every iteration?



#6 rip-off   Moderators   -  Reputation: 8212

Like
3Likes
Like

Posted 06 July 2014 - 04:00 PM

But the sound keeps playing even if temp is below zero.

Are you 100% sure the temperature actually goes below zero? Does the code work as expected with the other conditions?

#7 Phil123   Members   -  Reputation: 661

Like
0Likes
Like

Posted 06 July 2014 - 09:33 PM

That should actually work (with the operator precedence there should be no confusion, so no brackets needed).
 
There must be something else off (maybe the sound switch off?). Go through with your debugger and inspect those values.
 
As a general hint: If you don't know/trust what's going on, declare a couple of temporary variables (or even functions), and inspect those, e.g:
 

bool b1 = rainP <= 0.3;
bool b2 = temperatureC < 0;
bool b3 = cloudAlt <= altibaro;
bool b = b1 || b2 || b3;
Edit: Thinking about it, using parentheses explicitly (even if not needed) isn't that bad. Not only for readability.

Edit2: @Buckeye: Question now is rather who has ninja'd whom ? wink.png

 

 

This is a fantastic idea.  In fact, const bools are often used in UE4 for readability purposes.  Variables and constants may be self-documenting by themselves, but you can take it a step further as in the below example.  For example:

// Before:
if (someDescriptiveVariable < someDescriptiveNumber) {
    // Do stuff
}

// After:
const bool bSomeDescriptiveBool = (someDescriptiveVariable < someDescriptiveNumber);
if (bSomeDescriptiveBool) {
    // Do stuff
}

It may seem unnecessary to some, but let me tell you that it makes for very clean and self-documenting code that other people (not just yourself) can read.  I also agree that expressions are generally cleaner when they're surrounded by parentheses.

 

As for the actual topic, the best thing to do here (as previously mentioned) is to display the values in one way or another.


Edited by Phil123, 06 July 2014 - 09:33 PM.


#8 Chris_F   Members   -  Reputation: 2360

Like
2Likes
Like

Posted 06 July 2014 - 11:06 PM

It should be noted that performing your comparisons using multiple Boolean variables means that you cannot take advantage of short-circuit evaluation.


Edited by Chris_F, 07 July 2014 - 02:29 AM.


#9 FGFS   Members   -  Reputation: 196

Like
0Likes
Like

Posted 07 July 2014 - 12:22 AM

Thanks. || was ok but a displaced clamp.  Below seems fine:

 

                                    if(state != AL_PLAYING)
                                    {
                                        SourcesPos[0][0] = 0.0;
                                        SourcesPos[0][1] = 0.0;
                                        SourcesPos[0][2] = 0.0;

                                        alSourcefv(Sources[0], AL_POSITION, SourcesPos[0]);
                                    }
                                    if (rainP <= 0.3 || temperatureC < 0 || cloudAlt <= altibaro){
                                            alSourceStop(Sources[0]);
                                    }else if(state != AL_PLAYING){
                                        alSourcePlay(Sources[0]);
                                    }



#10 Khatharr   Crossbones+   -  Reputation: 3000

Like
4Likes
Like

Posted 09 July 2014 - 02:06 AM

It should be noted that performing your comparisons using multiple Boolean variables means that you cannot take advantage of short-circuit evaluation.

 

VS2012, compiled with default release mode settings:

#include <random>
#include <chrono>
#include <iostream>

int main() {
  std::srand(time(NULL));
  int val = rand(); //prevent compiler from just generating the result at compile time

  bool a = val < 10;
  bool b = val > 100;
  bool c = val > 50;
  bool d = val < 60;
  bool result = a || b || (c && d);

  std::cout << result << std::endl; //prevent compiler from skipping process due to unused result
  return 0;
}

Resulting disassembly:

#include <random>
#include <chrono>
#include <iostream>

int main() {
002F1270  push        ebp  
002F1271  mov         ebp,esp  
002F1273  push        ecx  
  std::srand(time(NULL));
002F1274  push        0  
  std::srand(time(NULL));
002F1276  call        dword ptr ds:[2F30BCh]  
002F127C  push        eax  
002F127D  call        dword ptr ds:[2F30C8h]  
002F1283  add         esp,8  
  int val = rand();
002F1286  call        dword ptr ds:[2F30CCh]  
  bool a = val < 10;
002F128C  lea         ecx,[eax-0Ah]  
002F128F  cmp         ecx,5Ah  

  bool result = a || b || (c && d);
002F1292  ja          main+34h (02F12A4h)  
  bool b = val > 100;
  bool c = val > 50;
002F1294  cmp         eax,32h  

  bool result = a || b || (c && d);
002F1297  jle         main+2Eh (02F129Eh)  
  bool d = val < 60;
002F1299  cmp         eax,3Ch  

  bool result = a || b || (c && d);
002F129C  jl          main+34h (02F12A4h)  
002F129E  mov         byte ptr [result],0  
002F12A2  jmp         main+38h (02F12A8h)  
002F12A4  mov         byte ptr [result],1  

  std::cout << result << std::endl;
002F12A8  push        dword ptr ds:[2F3024h]  
002F12AE  mov         ecx,dword ptr ds:[2F303Ch]  
002F12B4  push        dword ptr [result]  
002F12B7  call        dword ptr ds:[2F302Ch]  
002F12BD  mov         ecx,eax  
002F12BF  call        dword ptr ds:[2F3028h]  

  return 0;
002F12C5  xor         eax,eax  
}
002F12C7  mov         esp,ebp  
002F12C9  pop         ebp  
002F12CA  ret  

Compilers are very, very smart.


void hurrrrrrrr() {__asm sub [ebp+4],5;}

There are ten kinds of people in this world: those who understand binary and those who don't.

#11 Brother Bob   Moderators   -  Reputation: 8193

Like
3Likes
Like

Posted 09 July 2014 - 04:27 AM

 

It should be noted that performing your comparisons using multiple Boolean variables means that you cannot take advantage of short-circuit evaluation.

 

VS2012, compiled with default release mode settings:

..

Resulting disassembly:

...

Compilers are very, very smart.

 

They are, hopefully, also very smart not to do that when the evaluations of the temporary variables have side effects because that would change the meaning of the program. Your example works because the individual conditions does not have any observable side effects. If the expressions have side effects, or otherwise relies on short circuiting to prevent the evaluation of an expression in the condition (for example checking for null-pointer before accessing a member of an object), expanding the expressions to temporary variables before the if-statement won't work.



#12 Khatharr   Crossbones+   -  Reputation: 3000

Like
0Likes
Like

Posted 09 July 2014 - 07:11 PM

 

 

It should be noted that performing your comparisons using multiple Boolean variables means that you cannot take advantage of short-circuit evaluation.

 

VS2012, compiled with default release mode settings:

..

Resulting disassembly:

...

Compilers are very, very smart.

 

They are, hopefully, also very smart not to do that when the evaluations of the temporary variables have side effects because that would change the meaning of the program. Your example works because the individual conditions does not have any observable side effects. If the expressions have side effects, or otherwise relies on short circuiting to prevent the evaluation of an expression in the condition (for example checking for null-pointer before accessing a member of an object), expanding the expressions to temporary variables before the if-statement won't work.

 

 

OP's code does not include side effects.

 

On the other hand, in the case that the statements do (or may) have side effects, the compiler will obviously not screw it over.

 

In that case, though, it's better to just write the test sequence out clearly rather than relying on obscure details of the spec. If you want to do something obvious like null checking, you can just do that into the temp:

bool a = (ptr && ptr->something);
bool b = stuff;
if(a || b) {etc}

void hurrrrrrrr() {__asm sub [ebp+4],5;}

There are ten kinds of people in this world: those who understand binary and those who don't.

#13 Chris_F   Members   -  Reputation: 2360

Like
0Likes
Like

Posted 09 July 2014 - 08:55 PM


Compilers are very, very smart.

 

But they don't perform magic. I'm specifically referring to cases such as this:

 

#include <iostream>

bool SomeExpensiveComputation()
{
    std::cout << "D'oh!" << std::endl;
    return true;
}

int main()
{
    int value = 42;
   
    if ( value > 0 || SomeExpensiveComputation() )
    {
        std::cout << "Example 1" << std::endl;
    }
   
    bool b1 = value > 0;
    bool b2 = SomeExpensiveComputation();
   
    if ( b1 || b2 )
    {
        std::cout << "Example 2" << std::endl;
    }
}

 

http://coliru.stacked-crooked.com/a/339dc09bdbc542dc



#14 Khatharr   Crossbones+   -  Reputation: 3000

Like
0Likes
Like

Posted 10 July 2014 - 12:43 AM

That's just begging for a long debug session some time in the distant future. Stuff like that is hard to spot if you aren't looking for it.

bool execute = false;
if(value > 0) {execute = true;}
else if(SomeExpensiveComputation()) {execute = true;}
if(execute) {std::cout << "Example 1" << std::endl;}

^ Compiles the same, but is far more clear about what's going on.

 

Alternatively, it may be a good idea to at least leave a comment indicating that you're making use of a short circuit.


Edited by Khatharr, 10 July 2014 - 03:52 AM.

void hurrrrrrrr() {__asm sub [ebp+4],5;}

There are ten kinds of people in this world: those who understand binary and those who don't.




Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS