[SOLVED] createCylinder radius opt parameter inconsistent in code and documentation


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;

Feel free to make PR to the engine: https://github.com/playcanvas/engine

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?


if(opts.baseRadius !== undefined) {
  console.log("'baseRadius' deprecated! Please use radius.")
  if(opts.radius === undefined)
    opts.radius = opts.baseRadius;

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?

Not a problem, same as if you provide any other random property, there won’t be warning for it.


Took a while, still getting used to GitHub/Desktop.

1 Like

Thanks for your contribution! U rock! :guitar:

Aw shucks! ‘Tweren’t nuthin’.