https://developer.playcanvas.com/en/api/pc.html#createCylinder
states
opts.radius Number The radius of the tube forming the body of the cylinder (defaults to 0.5).
but in procedural.js
the code uses baseRadius not radius.
pc.createCylinder = function (device, opts) {
...
var baseRadius = opts && opts.baseRadius !== undefined ? opts.baseRadius : 0.5;
max
#2
Feel free to make PR to the engine: https://github.com/playcanvas/engine
will
#3
Yep, should be radius
instead of baseRadius
in the code.
Won’t changing opts.baseRadius
to opts.radius
be a breaking change? Doesn’t this need to be done defensively, deprecating opts.baseRadius
?
e.g.
if(opts.baseRadius !== undefined) {
console.log("'baseRadius' deprecated! Please use radius.")
if(opts.radius === undefined)
opts.radius = opts.baseRadius;
}
max
#5
Logging should be behind DEBUG define, like:
// #ifdef DEBUG
if (opts.hasOwnProperty('baseRadius') && ! opts.hasOwnProperty('radius'))
console.warn('DEPRECATED: "baseRadius" in arguments, use "radius" instead');
// #endif
And for simplicity of logic bellow:
It makes no sense to have radius of 0
, so not checking for property, but value.
opts.radius = opts.radius || opts.baseRadius;
And yes, no breaking is allowed, we have to make sure old way still works.
This won’t issue a warning if both radius
and baseRadius
are supplied. Not a problem?
max
#7
Not a problem, same as if you provide any other random property, there won’t be warning for it.
Done.
Took a while, still getting used to GitHub/Desktop.
1 Like
will
#9
Thanks for your contribution! U rock! 
Aw shucks! ‘Tweren’t nuthin’.