N.A.N value ---SOLVED---

[THe Floating Brain]

Sorry for making so many post's lately. Iv been working on re-structuring the framework of my game engine. Anyway I have been using this vector calculation method (as ugly as it is)

Code: Select all

#define Magn(x, y, Mag) Mag = ((x)*(x) + (y)*(y))
namespace Math
{
	namespace Vector
	{
		class Vector_Calculator
		{
		public:
			Containers::Base::XYZ Vector_Calculation( float GotoY, float GotoX, float X, float Y, float Speed )
			{
				Containers::Base::XYZ A;
				//Calculate Magnitude.//
				GotoX -= X;
					GotoY -= Y;
						float Magnitude;
							Magn(GotoX, GotoY, Magnitude);
								Magnitude *= Magnitude;
									A.setX(GotoX);
										A.setY(GotoY);
											//Normilise//
										Magnitude = sqrt(Magnitude);
										GotoX /= Magnitude;
								GotoY /= Magnitude;
								//Go The Desired Speed.//
							GotoX *= Speed;
						GotoY *= Speed;
					A.setX(GotoX);
				A.setY(GotoY);
				return A;
			}
		};
	}
}

But I am receiving a N.A.N value from it which throws off my entire collision calculation (im using this to calculate vectors for collision detection). Since the value is N.A.N my calculation method continues to loop because the magnitude of a line is neither incrementing nor decrementing the value (the value it is suppose to increment/decrement being a value from calculated from this method). I am wondering how to handle the N.A.N variable after I detect it as N.A.N.
( How to detect a N.A.N value )

Code: Select all

bool Is_NAN( float Value )
{
   //IEEE designed its floating point to make this statement false
   //if it is nan.//
   if( Value != Value )
     return true;
   //Since it passed the if condition we now know the value is not
   //N.A.N therefore we return false saying that the statement "This value is N.A.N" is false.//
   return false;
}

Thank you for reading :-D
P.s I know I can sometimes be unclear as to my purpose, if my purpose is unclear and further clarification is needed please let me know.

[short]

[quote="THe Floating Brain"]Sorry for making so many post's lately. Iv been working on re-structuring the framework of my game engine. Anyway I have been using this vector calculation method (as ugly as it is)

Code: Select all

#define Magn(x, y, Mag) Mag = ((x)*(x) + (y)*(y))
namespace Math
{
	namespace Vector
	{
		class Vector_Calculator
		{
		public:
			Containers::Base::XYZ Vector_Calculation( float GotoY, float GotoX, float X, float Y, float Speed )
			{
				Containers::Base::XYZ A;
				//Calculate Magnitude.//
				GotoX -= X;
					GotoY -= Y;
						float Magnitude;
							Magn(GotoX, GotoY, Magnitude);
								Magnitude *= Magnitude;
									A.setX(GotoX);
										A.setY(GotoY);
											//Normilise//
										Magnitude = sqrt(Magnitude);
										GotoX /= Magnitude;
								GotoY /= Magnitude;
								//Go The Desired Speed.//
							GotoX *= Speed;
						GotoY *= Speed;
					A.setX(GotoX);
				A.setY(GotoY);
				return A;
			}
		};
	}
}

I mean this in the nicest way possible, WTF is up with the formatting of your code hahaha.

[THe Floating Brain]

Sorry for making so many post's lately. Iv been working on re-structuring the framework of my game engine. Anyway I have been using this vector calculation method (as ugly as it is)

Code: Select all

#define Magn(x, y, Mag) Mag = ((x)*(x) + (y)*(y))
namespace Math
{
	namespace Vector
	{
		class Vector_Calculator
		{
		public:
			Containers::Base::XYZ Vector_Calculation( float GotoY, float GotoX, float X, float Y, float Speed )
			{
				Containers::Base::XYZ A;
				//Calculate Magnitude.//
				GotoX -= X;
					GotoY -= Y;
						float Magnitude;
							Magn(GotoX, GotoY, Magnitude);
								Magnitude *= Magnitude;
									A.setX(GotoX);
										A.setY(GotoY);
											//Normilise//
										Magnitude = sqrt(Magnitude);
										GotoX /= Magnitude;
								GotoY /= Magnitude;
								//Go The Desired Speed.//
							GotoX *= Speed;
						GotoY *= Speed;
					A.setX(GotoX);
				A.setY(GotoY);
				return A;
			}
		};
	}
}

I mean this in the nicest way possible, WTF is up with the formatting of your code hahaha.

The "lets tinker with this code until we can at least pretend it looks good" format XD

[TheBuzzSaw]

[OCD]

Code: Select all

inline bool Is_NAN( float Value ) { return Value != Value; }

[/OCD]

[Falco Girgis]

I'm going to do my average, routine critiques that I do to everybody in the interest of making their code better. I don't have anything against you personally or your code. I'm not judging you. </disclaimer>

Sorry, I have to say that so that all of the forum hippies don't go on their usual "OMFG FALCO IS AN ARROGANT PRICK" bitchspree... If you're rewriting your engine, I would assume that you are interested in producing a quality product and would much prefer realistic, practical advice over sitting around a fire singing kumbaya with me...

Code: Select all

#define Magn(x, y, Mag) Mag = ((x)*(x) + (y)*(y))

This is actually magnitude SQUARED or the dot product of vector <x,y> with itself (same thing). Your macro is misnamed.

Code: Select all

  Magnitude *= Magnitude;

You're squaring that value AGAIN before taking the square root. Your magnitude is incorrect.

I take it that this function is to return a vector in the direction from <X, Y> to <GotoX, GotoY> with a magnitude of the desired speed. The problem is that your vector is incorrect, because your "squared magnitude" was squared yet again before taking the square root.

Also, I highly recommend you rethink your design here. This is a critical, core mathematics class. In my opinion, you're over-encapsulating your vector data by making it a class rather than a structure. You should always be wary of get/set accessor methods in classes like this. Speed is critical here, especially considering the fact that you're gaining nothing but overhead by encapsulating three floats in your vector...

Now to the main question...

But I am receiving a N.A.N value from it which throws off my entire collision calculation (im using this to calculate vectors for collision detection). [..]I am wondering how to handle the N.A.N variable after I detect it as N.A.N.

You're going about it incorrectly. The correct way is not to have special logic to handle a NAN. The correct way is to have special logic to prevent a NAN.

NAN is not something that should ever happen in well-designed code. Certain architectures may not even gracefully return a NAN, they might shit their pants.

The only place that this function can even potentially produce a NAN (that I can see) is during the floating point division (divide-by-zero). That's only going to happen when vector <GotoX, GotoY> == <X, Y>, which means that you're already at the location that you're trying to head to. You can much more gracefully add that check BEFORE the mathematics and return early (preventing you from even having to calculate anything in that case).

[Ginto8]

Looking at your code... you're entering design hell. It seems that you're trying to encapsulate TOO MUCH, and fucking yourself over in the process. A Vector class should overload operators (or just have its own methods) for its calculations, rather than have a completely separate class do them for it. Also, as falco said, your Magn macro calculates the SQUARE of the magnitude, and it has violated one of the most important rules of macros: never allow the order of operations to get in the way. Thus,

Code: Select all

#define Magn(x, y, Mag) Mag = ((x)*(x) + (y)*(y))

should be

Code: Select all

#define Magn(x, y, Mag) (Mag = ((x)*(x) + (y)*(y)))

Also, you're calling A.setX/A.setY with GotoX and GotoY, then promptly ignoring A and setting them again later.

Long story short, this function needs to be fixed, and your overall design probably does too. If you need help designing a vector class, or just want to use one from another library, I'd like to point out that SFML is open source and has a very useful Vector class.

[THe Floating Brain]

I am thankful for the advice ( Falco + Ginto ) :-D I would like to say that (thankfully) this is not what the rest of my code looks like. I made this function 6 months ago and have been copy/pasting it ever since and I very much agree that it needs to be re - written.

I'm going to do my average, routine critiques that I do to everybody in the interest of making their code better. I don't have anything against you personally or your code. I'm not judging you. </disclaimer>

Sorry, I have to say that so that all of the forum hippies don't go on their usual "OMFG FALCO IS AN ARROGANT PRICK" bitchspree... If you're rewriting your engine, I would assume that you are interested in producing a quality product and would much prefer realistic, practical advice over sitting around a fire singing kumbaya with me...

Code: Select all

#define Magn(x, y, Mag) Mag = ((x)*(x) + (y)*(y))

This is actually magnitude SQUARED or the dot product of vector <x,y> with itself (same thing). Your macro is misnamed.

Code: Select all

  Magnitude *= Magnitude;

You're squaring that value AGAIN before taking the square root. Your magnitude is incorrect.

Ooooh so thats why

Code: Select all

Magnitude = sqrt(Magnitude);

never worked.

I take it that this function is to return a vector in the direction from <X, Y> to <GotoX, GotoY> with a magnitude of the desired speed. The problem is that your vector is incorrect, because your "squared magnitude" was squared yet again before taking the square root.

Also, I highly recommend you rethink your design here. This is a critical, core mathematics class. In my opinion, you're over-encapsulating your vector data by making it a class rather than a structure. You should always be wary of get/set accessor methods in classes like this. Speed is critical here, especially considering the fact that you're gaining nothing but overhead by encapsulating three floats in your vector...

I agree, but wont using a struct and making float X, Y, Z public get all the data jumbled up?
Another thing I could probably do is make XYZ a friend class.

Now to the main question...

But I am receiving a N.A.N value from it which throws off my entire collision calculation (im using this to calculate vectors for collision detection). [..]I am wondering how to handle the N.A.N variable after I detect it as N.A.N.

You're going about it incorrectly. The correct way is not to have special logic to handle a NAN. The correct way is to have special logic to prevent a NAN.

NAN is not something that should ever happen in well-designed code. Certain architectures may not even gracefully return a NAN, they might shit their pants.

The only place that this function can even potentially produce a NAN (that I can see) is during the floating point division (divide-by-zero). That's only going to happen when vector <GotoX, GotoY> == <X, Y>, which means that you're already at the location that you're trying to head to. You can much more gracefully add that check BEFORE the mathematics and return early (preventing you from even having to calculate anything in that case).

That makes a lot of sense because before I changed my code a bit the original vector was 0, 0, 0 after changing some logic I got the NAN.

Looking at your code... you're entering design hell. It seems that you're trying to encapsulate TOO MUCH, and fucking yourself over in the process. A Vector class should overload operators (or just have its own methods) for its calculations, rather than have a completely separate class do them for it. Also, as falco said, your Magn macro calculates the SQUARE of the magnitude, and it has violated one of the most important rules of macros: never allow the order of operations to get in the way. Thus,

Code: Select all

#define Magn(x, y, Mag) Mag = ((x)*(x) + (y)*(y))

should be

Code: Select all

#define Magn(x, y, Mag) (Mag = ((x)*(x) + (y)*(y)))

I am not sure I understand the difference in the two pieces of code? Does the second one distribute? ( Sorry I am horrible with pre-processor commands ).

Also, you're calling A.setX/A.setY with GotoX and GotoY, then promptly ignoring A and setting them again later.

Yaaa, kinda a bad design.

Long story short, this function needs to be fixed, and your overall design probably does too. If you need help designing a vector class, or just want to use one from another library, I'd like to point out that SFML is open source and has a very useful Vector class.

I could probably use some help, unless I could just make this a friend class? (Not sure if that is the proper course of action or not).

[Falco Girgis]

I agree, but wont using a struct and making float X, Y, Z public get all the data jumbled up?

I have no idea what you're talking about...

I am not sure I understand the difference in the two pieces of code? Does the second one distribute? ( Sorry I am horrible with pre-processor commands ).

The difference is enforcing the order of operations... Imagine you used your code as such:

Code: Select all

Magn(x,y,Mag)*7.0f;

What is going to be the value of Mag? It's not what it should be, because multiplication has higher precedence than assignment... Without using Ginto's fix, you'll run into shit like that.

You should also really consider passing the return vector by reference instead of incurring the overhead of copy constructing 3 floats every time... I normally wouldn't bitch, but as I said, this is core math functionality.

[THe Floating Brain]

I agree, but wont using a struct and making float X, Y, Z public get all the data jumbled up?

I have no idea what you're talking about...

You suggested I use a struct to store my values, by default a struct is public rather than privet infering that you want me to make my float's public. Im just wondering if the data will get messed up that way.

I am not sure I understand the difference in the two pieces of code? Does the second one distribute? ( Sorry I am horrible with pre-processor commands ).

The difference is enforcing the order of operations... Imagine you used your code as such:

Code: Select all

Magn(x,y,Mag)*7.0f;

What is going to be the value of Mag? It's not what it should be, because multiplication has higher precedence than assignment... Without using Ginto's fix, you'll run into shit like that.

You should also really consider passing the return vector by reference instead of incurring the overhead of copy constructing 3 floats every time...

Are you suggesting that I change my argument to

Code: Select all

Vector_Calculator( &XYZ Goto, &XYZ Position )

The only reason im not doing that is because the calculations im doing all get allocated into a single std::vector. The vector calculations im doing can generate tens of thousands to hundreds of thousands of values, and I do not want the data to become corrupted.

I normally wouldn't bitch, but as I said, this is core math functionality.

Its fine I appreciate the advice ( I obviously need it ).

<!-----------------EDIT-----------------!>
Is this better?

Code: Select all

#define Magn(x, y, Mag) (Mag = ((x)*(x) + (y)*(y)))
namespace Math
{
	namespace Vector
	{
		class Vector_Calculator
		{
			Containers::Base::XYZ Vector;
			Containers::Base::XYZ Default;
			float x, y, Magnitude;
		public:
			Vector_Calculator()
			{
				Containers::Base::XYZ temp;
				Default = temp;
			}
			Containers::Base::XYZ Vector_Calculation( Containers::Base::XYZ Goto, Containers::Base::XYZ Position, float Speed )
			{
				//Make sure that the vector doesent have any stray values lying around.//
				Vector = Default;
				//Calculate Magnitude.//
				x = Goto.getX() - Position.getX();
				y = Goto.getY() - Position.getY();
				Magn( x, y, Magnitude );
				if( x == 0 && y == 0 && Vector.getZ() == 0 )
					return Vector;
				//Normilise//
				Magnitude = sqrt( Magnitude );
				x /= Magnitude;
				y /= Magnitude;
				//Go The Desired Speed.//
				x *= Speed;
				y *= Speed;
				Vector.setX( x );
				Vector.setY( y );
				return Vector;
			}
		};
	}
}

It seems to do the job:

OMG IT WORKS!!.PNG (28.78 KiB) Viewed 1912 times

[short]

Two quick observations in between studying for my algorithms midterm tomorrow...

First of all, bookmark this. It's good to have in case you ever forget C++ operator precedence order:

This link is worth bookmarking, it is always nice to look up the C++ order of operator precedence.

http://en.cppreference.com/w/cpp/langua ... precedence

Code: Select all

if( x == 0 && y == 0 && Vector.getZ() == 0 )

Technically, you should use float comparison

Define the following operator and you can directly compare a vector and an integer

Code: Select all

#include <cfloat>

// not a member function of the vector class, assuming you followed the above advice and made your vector class a struct
inline bool operator==(const Vector &lhs, const int r) {
    return ((lhs.x - r) < FLT_EPSILON)  && ((lhs.y - r) < FLT_EPSILON) && ((lhs.z - r) < FLT_EPSILON);
}

Code: Select all

#define Magn(x, y, Mag) Mag = ((x)*(x) + (y)*(y))

unless I am missing something important here .... ............. why not have your macro be this ....

Code: Select all

#define Magn(x, y, Mag) Mag = sqrt(((x)*(x) + (y)*(y)))

or preferable ...

Code: Select all

 inline float magn(const vector &v) { return sqrt((v.x)*(v.x) + (v.y)*(v.y)); } 

I mean Magn should return the magnitude of your vector, no?

[avansc]

Code: Select all

#define Magn(x, y, Mag) Mag = ((x)*(x) + (y)*(y))

unless I am missing something important here .... ............. why not have your macro be this ....

Code: Select all

#define Magn(x, y, Mag) Mag = sqrt(((x)*(x) + (y)*(y)))

or preferable ...

Code: Select all

 inline float magn(const vector &v) { return sqrt((v.x)*(v.x) + (v.y)*(v.y)); } 

I mean Magn should return the magnitude of your vector, no?

I have not followed the thread, so sorry if my foot is in my ass, or something like that.

its just x*x + y*y and not sqrt(that) for speed reasons.

if you do something like circle collision detection, you do if( (vecPoint - circ.center).mag2 < circ.radius*circ.radius) COLLISION
that way its faster.

[Falco Girgis]

Code: Select all

#define Magn(x, y, Mag) Mag = sqrt(((x)*(x) + (y)*(y)))

It'd still have to be

Code: Select all

#define Magn(x, y, Mag) (Mag = sqrt((x)*(x) + (y)*(y)))

to avoid any operator precedence ambiguities like

Code: Select all

Magn(x,y,Mag)*7.0f;

You suggested I use a struct to store my values, by default a struct is public rather than privet infering that you want me to make my float's public. Im just wondering if the data will get messed up that way.

Then don't mess up your data?

Are you suggesting that I change my argument to

Code: Select all

Vector_Calculator( &XYZ Goto, &XYZ Position )

The only reason im not doing that is because the calculations im doing all get allocated into a single std::vector. The vector calculations im doing can generate tens of thousands to hundreds of thousands of values, and I do not want the data to become corrupted.

Yes, as well as the outvector also being part of that argument list (rather than a copy constructed return). What is it with you and "data corruption"? You seem to be throwing that out there a bunch, and I don't understand where it's coming from. Data doesn't just get "corrupted" out of nowhere. If you don't screw something up, your data won't get screwed up.

[THe Floating Brain]

Two quick observations in between studying for my algorithms midterm tomorrow...

First of all, bookmark this. It's good to have in case you ever forget C++ operator precedence order:

This link is worth bookmarking, it is always nice to look up the C++ order of operator precedence.

http://en.cppreference.com/w/cpp/langua ... precedence

ty

Code: Select all

if( x == 0 && y == 0 && Vector.getZ() == 0 )

Technically, you should use float comparison

I did not get a compiler warning. Vector.getZ() also returns a float.

Define the following operator and you can directly compare a vector and an integer

Code: Select all

#include <cfloat>

// not a member function of the vector class, assuming you followed the above advice and made your vector class a struct
inline bool operator==(const Vector &lhs, const int r) {
    return ((lhs.x - r) < FLT_EPSILON)  && ((lhs.y - r) < FLT_EPSILON) && ((lhs.z - r) < FLT_EPSILON);
}

Looks a lot easier than typing that whole if statement

Code: Select all

#define Magn(x, y, Mag) Mag = ((x)*(x) + (y)*(y))

unless I am missing something important here .... ............. why not have your macro be this ....

Code: Select all

#define Magn(x, y, Mag) Mag = sqrt(((x)*(x) + (y)*(y)))

or preferable ...

Code: Select all

 inline float magn(const vector &v) { return sqrt((v.x)*(v.x) + (v.y)*(v.y)); } 

I mean Magn should return the magnitude of your vector, no?

Agreed that I should probably put my magnitude calculation into a function/method so it can return the magnitude. The reason for not sqrt'ing it is so I can use the if statement to make sure my number does not come out N.A.N.

Code: Select all

#define Magn(x, y, Mag) Mag = ((x)*(x) + (y)*(y))

unless I am missing something important here .... ............. why not have your macro be this ....

Code: Select all

#define Magn(x, y, Mag) Mag = sqrt(((x)*(x) + (y)*(y)))

or preferable ...

Code: Select all

 inline float magn(const vector &v) { return sqrt((v.x)*(v.x) + (v.y)*(v.y)); } 

I mean Magn should return the magnitude of your vector, no?

I have not followed the thread, so sorry if my foot is in my ass, or something like that.

its just x*x + y*y and not sqrt(that) for speed reasons.

if you do something like circle collision detection, you do if( (vecPoint - circ.center).mag2 < circ.radius*circ.radius) COLLISION
that way its faster.

^ Also that :-P

Code: Select all

#define Magn(x, y, Mag) Mag = sqrt(((x)*(x) + (y)*(y)))

It'd still have to be

Code: Select all

#define Magn(x, y, Mag) (Mag = sqrt((x)*(x) + (y)*(y)))

to avoid any operator precedence ambiguities like

Code: Select all

Magn(x,y,Mag)*7.0f;

You suggested I use a struct to store my values, by default a struct is public rather than privet infering that you want me to make my float's public. Im just wondering if the data will get messed up that way.

Then don't mess up your data?

Are you suggesting that I change my argument to

Code: Select all

Vector_Calculator( &XYZ Goto, &XYZ Position )

The only reason im not doing that is because the calculations im doing all get allocated into a single std::vector. The vector calculations im doing can generate tens of thousands to hundreds of thousands of values, and I do not want the data to become corrupted.

Yes, as well as the outvector also being part of that argument list (rather than a copy constructed return). What is it you and "data corruption"? You seem to be throwing that out there a bunch, and I don't understand where it's coming from. Data doesn't just get "corrupted" out of nowhere. If you don't screw something up, your data won't get screwed up.

Lol we replied at the same time XD

The & operator can refrence any random pice of memory (as long as it is the same type). There for if I utilize the & operator it could reference any instance of Containers::Base::XYZ, anywhere in memory, correct?

Same with public members right?

====Edit====

btw changed the pre-possessor command into a method :-D

Code: Select all

namespace Math
{
	namespace Formulas
	{
		class Pythagoras
		{
		public:
			float Pythagorean_Theorem( float X, float Y, float Mag )
			{
				return ( Mag = ( ( X ) * ( X ) + ( Y ) * ( Y ) ) );
			}
		};
	}
[...]

[Falco Girgis]

It's still misnamed.

[THe Floating Brain]

It's still misnamed.

Should the name be dot product?