Please revert/fix d012eef00e1694
Brought to you by:
freddix
Menu
▾
▴
#8 Please revert/fix d012eef00e1694
- if ( DIST == 0.0f ){ DIST = 0.0000000001f; }
This does not work. It merely, at best papers over a real bug.
If someone passes something that causes DIST===0 to this function, then that is a BUG and you must log an error.
Just trying to pretend that this DIST is non-zero and is 1E-10 instead does not solve anything.
It just means that instead of crashing with a division by zero, you just pretend that all is well and return bogus data to the caller, from where it will propagate until something else break elsewhere, and now the only thing you have made is to move the crash/error from the obvious place to some unrelated place where it is much harder to debug or understand what happened..
For this case you NEED to log an error message.
Second bug here is with he floating points themself. You can in general not do checks like DIST == 0.0 with floating points since they will not check/do what you thin they do.
Instead you want to do something like comparing "is the value arbitrarily close to zero"
So you want something like
if (fabs(DIST) < 0.0000001) {
... log an error message because DIST is ~= 0 and return an error
}
fabs() because you dont really care abotu the sign of the value, only that it is "too close to zero".
Never ever compare a float to an exact value. It does not work the way you think.
Only compare if it is "close enough" and use fabs()
d012eef is broken, fix it.
I.e. you can not do "is value exactly xyx" with floating point. It does not work.
You must do "is value this close to xyz".
Please see some basic docs on how floating point work. It is not all that hard but you need to be aware of how they work and the right way to use them.
Currently, the engine is at alpha/beta state.
The "error message" system is NOT YET done.
I've changed DIST to make a "workaround" until the "error system" will be done.
When the error system will be done, I will add it to this function and remove the patch for DIST =0.
That's why I'll keep this ticket open.
Diff:
I think you should reconsider.
You should add some basic error logging right now, since it is right now when the code is alpha and unstable that it is most important to have logging.
Once the code is finished and production version, then you don't really need logging that much since hopefully there are few bugs by then.
It is now, when you are at alpha stage you need this the most, so that it is easier to find where in the code things go wrong and crashes.
So, you should begin adding simple error logging already from the beginning.
Error logging does not have to be complex or advanced. Something as simple as using some printf() to print some text to a file or to the con sole you start the program from is more than sufficient.
For simple logging, it is quite popular to use something like these simpple macros :
define AD_DEBUG(fmt, ...) { fprintf(stderr, "%s:%d:%s(): " fmt, FILE, LINE, func, VA_ARGS); } while(0)
define AD_FATAL(fmt, ...) { fprintf(stderr, "%s:%d:%s() FATAL: " fmt, FILE, LINE, func, VA_ARGS); exit(10); } while(0)
Maybe you can use these ?
Then everytime there is something that is wrong, you can generate a simple warning by just adding a line like this :
AD_DEBUG("Something unexpected happened and variable foo == %d\n", foo);
Or if something is completely wrong and you just want to print a message and then exit te application, you just do:
AD_FATAL("Something is really really wrong. Better just shutdown since we will
probably just crash if we continue.\n");
Error logging does not need to be any more advanced than this. And it will help you to track down bugs.
Hmm the markup has destroyed the c-code I will try some different markup :