yade-dev team mailing list archive

[Jan Stránský <honzik.stransky@xxxxxxxxx>]

Hi Anton,

thanks for the feedback.



>
> 1) Please, add an option to CMakeLists, it can be a "feature" probably,
>     disabled by default. There are a lot of examples there.
>

ok, before I will properly test it :-)


>
> 2) [2]
> + bool maskOk(int mask) const {return (mask==0 || (bool)(groupMask &
> mask));}
> + bool maskCompatible(int mask) const { return (bool)(groupMask & mask); }
> +#ifdef BODY_GROUP_MASK_ARBITRARY_PRECISION
> + bool maskOk(const boost::python::long_& mask) const {return (mask==0
> || (bool)(groupMask & mask));}
> + bool maskCompatible(const boost::python::long_& mask) const { return
> (bool)(groupMask & mask); }
> +#endif
>
> Do you want to have both of these definitions if
> BODY_GROUP_MASK_ARBITRARY_PRECISION
> enabled ?
>
> [2]
> https://github.com/yade/trunk/commit/15bca1969f43e883bfe9835d4a985a2ead337a77#diff-e6b228e5bbf5122a3adb4478cf4effa2R78


I am soory, I am not sure what exactly is the point here :-) The duplicate
code in maskOk and maskCompatible? int argument is necessary as some other
classes uses ints (like NewtonIntegrator or
collider::avoidSelfInteractionMask), bp::long_ is optionally for body-body
evaluation. Probably I could template it.


>
>
> 3) [3]
> + namespace bp = boost::python;
> You are there already in a namespace boost, you can just use
> python:: instead of bp::.
>
> [3 ]
> https://github.com/yade/trunk/commit/15bca1969f43e883bfe9835d4a985a2ead337a77#diff-e6b228e5bbf5122a3adb4478cf4effa2R144


Ok, thanks, I did not pay much attention to namespaces..


The only thing which is missing is automatic conversion from int to
bp::long_, I was not able to make it work..

cheers
Jan