But, "between 1 and 65535" means 1 < x < 65355, so it's required to state 0 and SHORT_MAX + 1 as the limits (meaning x lies between those values, NOT including)!
:)

Test is now correct but I think between 0 and 65536 will confuse the most users. I think the will try to set 0 or 65536 and will get an error.
I think with min value 1 and max value 65535 is better here.
But I am no native English speaker I leave this to agoose77.

KX_ClientObjectInfo *

I think that sentence will confuse the most users. Maybe we should change it to.
bit field min value 1 and max value %i", USHRT_MAX)

same as above.

Same as for the other comment.

Same as for the other error messages.

KX_ClientObjectInfo *client

KX_ClientObjectInfo *client

KX_ClientObjectInfo *client

KX_ClientObjectInfo *client

This mask needs more explanation. It's not enough to know that it's a bit field. What is it compared with? Can every object have a custom collision mask? Or are the bits compared to layer numbers? This should be documented.

What is data? Everything could be called "data", so the name itself doesn't hold any information, so should be clarified with a comment.

The code rejects 0, but the error message allows 0.

Why not simply write "0 < collisionGroup <= %i" ? A bit of mathematical notation goes a long way in making things clear.

Why is there a need to perform such a pointer cast? Why not simply pass a RayCastData * directly? Some comments about this (probably in the header file) might be useful.

"data" is some extra arguments for this function. do you have a better name ? extraargs/args ?

The argument data is extra arguments so no special type, depends on the user. The struct RayCastData is only for the extra argument needed by KX_GameObject not byy KX_RaySensor…

Right now things are confusing. Further down, you use a cast to a certain type, implying that the data type is actually fixed. However, in this explanation you seem to imply that the data type is up to the user, and thus not fixed. Which one is it?

So does that mean that the cast can fail because there is other data passed in there? If so, use dynamic_cast instead, and do a runtime check. Otherwise a static_cast + good documentation is preferred.

see KX_RayCast.data doc and KX_GameObject.RayCastData

It's like client pointer, we don't know the type it depends… so we use a void *. The data argument is a temporary instance create only for the ray cast. in the case of KX_GameObject KX_GameObject::RayCastData is used only two time : in KX_GaleObject::NeedRayCast and in KX_Gameobject::RayHit. But there no reason to use the same think for RAS_Rasterizer::RayHit or KX_RaySensor::RayHit, these classes can use their own temporary ray cast extra info type. There no common point for this argument so an inheritance will be useless.
This behaviour was already implemented for RayHit but not for NeedRayCast so i just extend it (it should be for both functions already).

bool RayHit(KX_ClientObjectInfo *client, KX_RayCast *result, RayCastData *rayData);

Callback(T *_self, PHY_IPhysicsController *controller = NULL, dataT *_data = NULL, bool faceNormal = false, bool faceUV = false)

If it's always expecting to pass RayCastData then it makes sense to get rid of void* and use proper struct name in the declarations.

For raycast function in KX_ConstraintActuator we don't use the data argument so it's a void*.

val > ((1 << OB_MAX_COL_MASKS) - 1)) isnt normal logic here...

Typically you want to check "are any bits set besides OB_MAX_COL_MASKS"

So for that the check is: val & ~((1 << OB_MAX_COL_MASKS) - 1))

so you just want to check at least one bit is set, and that all bits are from 1-16

so...

if ((val == 0) || (val & ~((1 << OB_MAX_COL_MASKS) - 1))))

Would add (), if ((val == 0) || (val & ~((1 << OB_MAX_COL_MASKS) - 1))) {

otherwise fine.

I aree with Sergey; the type of a parameter doesn't depend on whether the parameter is used or not. It depends on the type of the data when it is used.

The 0 < mask < 65536 part of the comment seems odd. You could just note that its an int where 16 bits are used for layers.

There are still some things that could be described clearer. Mentioning "(16 layers)" just after the word "scene" could suggest that you're talking about scene layers. Furthermore, "a category of objects" is vague, and you don't explicitly describe how the mask is applied. I would write something like this:

"The collision mask (16 layers mapped to a 16-bit integer) is combined with each object's collision group, to hit only a subset of the objects in the scene. Only those objects for which collision_group & mask is true can be hit."