Bugfix - different from original c++ code

[JYKeith123]

thanks for the great porting!

[Robmaister]

For the change in Heightfield.cs, why change Math.Ceiling to Math.Round? Wouldn't it make sense to always round up to ensure no geometry is missed? The values should always be positive, so there's no need to worry about rounding to or away from zero either...

As for the second change, nice catch! I'll wait to figure out what's going on with the Heightfield change before pulling this in.

[JYKeith123]

Actually, I'm not sure for the Heightfield.cs change. But it seems to cause unit test failure in my case.

Currently, I'm making rts style game which needs deterministic programming. So I changed all float value to this fixed-point data type. Then ran all tests.
There were a few failures, and found several code differences I committed into this repo between original C++ code and SharpNav code. After fixing it, unit tests were all passed.

The original code below.
void rcCalcGridSize(const float* bmin, const float* bmax, float cs, int* w, int* h)
{
*w = (int)((bmax[0] - bmin[0])/cs+0.5f);
*h = (int)((bmax[2] - bmin[2])/cs+0.5f);
}

[Robmaister]

interesting, do you have separate unit tests for SharpNav? If so I'd be happy to pull in more for the SharpNavTests project.

Looking at Recast's tests, this would be an incompatibility between the two, but the project is not supposed to be a straight port - a lot of things are moved around structurally, and I generally prefer correctness over slight memory savings.

The only difference is that SharpNav should have an extra column and row that contains the last 0.5m of the world that Recast does not. I would rather not pull in that change unless there's a specific issue caused by having those extra cells. Does that make sense?

[JYKeith123]

No, All unit tests I did are from SharpNavTests project. I just changed float type to fixed-point type.

Okay. I agree with you since there is no specific reason. I'll let you know if I catch other issues about this. Cheers :-)

[Robmaister]

I'll do some testing and see why ceiling isn't working for you but round does. My guess is it has something to do with round using banker's rounding (in the case of 0.5, round to the nearest even), still not sure why ceiling wouldn't work for you though.

If you have a copy of the projects set up with the Fix64 type, could you send that my way to speed up the process? I'm fine with a zipped folder over email or whatever if you don't have it up on a public repo somewhere.

[JYKeith123]

Hi, Here is the link.

I noticed that I converted SharpNav into .Net 3.5 as well, to share with unity project.
(My project will be used for both unity client and .net server)
And also changed NUnit to MSTest for integrating with my other tests.
Sorry for not mentioning this.

If you run all tests from the solution, you can see this 3 test failures while ceiling the values.

BuildRegions_Success, DistanceField_Medium_Success, DistanceField_Simple_Success

And you change Heightfield.cs ceiling to round, then all tests will succeed.

Please see files & folders explanation below.
Files

• Fix64Converter.cs : JsonConverter for Fix64 type.
• Vector2Converter.cs : JsonConverter for my own Vector2.
• Vector2.cs : Vector2 with Fix64.
• Vector3.cs : Vector3 with Fix64. (just used SharpNav.Geometry.Vector3)

Folders

• CSharExtension : To use Tuple<,>, Action in .net 3.5
• FixedMath : Fix64 type
  (https://github.com/asik/FixedMath.Net)
• Newtonsoft.Json : To use Newtonsoft.Json in .net3.5 and unity3D (https://github.com/SaladLab/Json.Net.Unity3D)
• SharpNav : changed all float type and values into Fix64

[Robmaister]

Thanks, I'll take a peek at this probably early next week