search

facebookarchive / RakNet

RakNet is a cross platform, open source, C++ networking engine for game programmers.
Other
3.3k stars 1.02k forks source link

change abs to fabs #87

Open tsky1971 opened 8 years ago

tsky1971 commented 8 years ago

change abs to fabs to avoid error message (VC2015) add cast on test return

mbabuskov commented 8 years ago

Function in NatPunchthroughClient.cpp could simply be changed to return const char * instead of casting on every return. The returned char array is never modified, so const would be a better solution.

Luke1410 commented 6 years ago

The changes suggested to CCRakNetSlidingWindow.cpp we resolved in SLikeNet in the way suggested in #64 (was integrated in SLikeNet 0.1.0). IMO that solution is standard compliant and there's no need to use fabs there (note that obviously for C it would be a different matter --- pls state, if you think otherwise so we can reconsider our choice).

The changes (cast of string literals to non const pointers) we rejected. IMO you'd really not cast these into non-const pointers. We therefore went with the alternative also suggested by @mbabuskov (simply changing the return value) which is covered by #22 and was also incorporated in SLikeNet 0.1.0 already.

The change suggested to RakString::ToWideChar() is more problematic. It doesn't suffice to simply cast the returned empty string literal into a non-const wchar. The problem here is rather a questionable API design decision to return a non-const pointer and leaving it with the caller to free the allocated string again. The proper fix therefore would be to explicitly allocate an empty string. Thanks for your pointer here, this will be resolved in SLikeNet 0.2.0 (changes are already available on the master branch - internal case number SLNET-190). We'll also consider changing the API for this function in SLikeNet 2.0.0 to return a const pointer and no longer require the caller to free the returned pointer (internal case number SLNET-191).

Your changes to CMakeList.txt we tested but rejected. In our tests (CMake 3.7.2 / VS 2015) the generated project file is incorrect when omitting the quotes (i.e. only the first lib would be added to the nodefaultlib-list. Did you see different results when testing your changes to the cmake file? Was there a specific issue which led you to these changes? Unless you point out some issue with the test we did, we won't integrate that part of your pull request.

Luke1410 commented 6 years ago

Since we did an unplanned release of SLikeNet, we decided to incorporate the fix for SLNET-190 in SLikeNet 0.1.2 already which is available now on https://www.slikenet.com/ and on the GitHub project page: https://github.com/SLikeSoft/SLikeNet/releases/tag/v.0.1.2 .