Point3D Comparison CrashBug-Fix

[Arahil]

I already posted this in the Bugtracker, but since no one seems to care since about a month i'm posting it here again to prevent some of you from a Servercrash.

The problem is located in Point3D.operator ==(Point3D, IPoint3D) and Point3D.operator !=(Point3D, IPoint3D). It is never checked if the IPoint3D is null (to be honest - it seems to me that method is just copied from the comparison Point3D-Point3D, which can't be null, since it is a value type).


The fix for that is pretty simple - just a check if the IPoint3D-value is null:

public static bool operator == ( Point3D l, IPoint3D r )
{
	if (r == null) return false;
	return l.m_X == r.X && l.m_Y == r.Y && l.m_Z == r.Z;
}

public static bool operator != ( Point3D l, IPoint3D r )
{
	if (r == null) return false;
	return l.m_X != r.X || l.m_Y != r.Y || l.m_Z != r.Z;
}

[Phantom]

I assume this requires a core recompile.

Yes the code should have a null, but use of it shouldn't even involve a null. Its one of those things, you shouldn't do ( any code I can think of, makes sure it would't happen ).

[Ohms_Law]

by adding the code that way, you'd never know if the comparisons (either way) were actually false, or if the code that was used to call it is simply malformed. what Phantom is (sorta) saying is that with base functions like this, you've gotta put the error checking onus on the code that calls the function.

think about this, you've got two basic values (number, letters, whatever). you wish to test if there equal or not. simple enough:
"if the value of A is equal to the value of B, do this"
where does the error checking go? to me, it shouldn't occur within that statement, but before that statement. so, instead of this:
"if the value that is in A is not null, then If the Value of A is equal to the value of what is in B, as long as B isn't a null value, do this"

doesn't it make more sence to do this:
"if the value of A is null, go get A"
"if the value of B is null, go get B"
"if the value of A is equal to the value of B, do this"

[Phantom]

by adding the code that way, you'd never know if the comparisons (either way) were actually false, or if the code that was used to call it is simply malformed. what Phantom is (sorta) saying is that with base functions like this, you've gotta put the error checking onus on the code that calls the function.

You got the general idea, what I really said was, if you were going to compare to Point3D variables, you would know if they were null or not ;-)

[Ohms_Law]

heh, yea
hey, I did end up saying that... in a round about way. lol

[Arahil]

Originally Posted by Phantom

I assume this requires a core recompile.

that's why it is in the core mod forum ^^

by adding the code that way, you'd never know if the comparisons (either way) were actually false, or if the code that was used to call it is simply malformed. what Phantom is (sorta) saying is that with base functions like this, you've gotta put the error checking onus on the code that calls the function.

think about this, you've got two basic values (number, letters, whatever). you wish to test if there equal or not. simple enough:
"if the value of A is equal to the value of B, do this"
where does the error checking go? to me, it shouldn't occur within that statement, but before that statement. so, instead of this:
"if the value that is in A is not null, then If the Value of A is equal to the value of what is in B, as long as B isn't a null value, do this"

doesn't it make more sence to do this:
"if the value of A is null, go get A"
"if the value of B is null, go get B"
"if the value of A is equal to the value of B, do this"

i hope i got the point out of this ... umm... quite confusing text.
yes, you are right
but nevertheless, if you forget to check your values before you call the function (resp. the IPoint3D value, since Point3D can't be null anyway) your server crashes. i can't fix someones coding skills, i just can make sure he does not crash the server

[Ohms_Law]

Originally Posted by Arahil

i hope i got the point out of this ... umm... quite confusing text.

yea, sorry...
I suck as a teacher.

i can't fix someones coding skills, i just can make sure he does not crash the server

that being said, education fixes everything. hehe

[noobie]

thats unnecessasy, the only way a get a null IPoint3D, to cast expilicitly something else as IPoint3D.

that means: unless you do something wrong, you dont get a null pointer exception..

but if you do, it is better to get that exception to debug..

[Phantom]

Originally Posted by noobie

thats unnecessasy, the only way a get a null IPoint3D, to cast expilicitly something else as IPoint3D.

that means: unless you do something wrong, you dont get a null pointer exception..

but if you do, it is better to get that exception to debug..

Which was one of my points, the only way to get a null Point3D is if you pass it to the argument.

If you do that, then its your fault as a programmer.