Skip to content

To radians function#47

Open
webocs wants to merge 3 commits into
metafizzy:masterfrom
webocs:degrees
Open

To radians function#47
webocs wants to merge 3 commits into
metafizzy:masterfrom
webocs:degrees

Conversation

@webocs

@webocs webocs commented Jun 5, 2019

Copy link
Copy Markdown

Description

Added a toRadians function and a corresponding demo for this Feature Request #34.

Changes

  • Added toRadians function in Boilerplate.js
  • Added a demo for the function
  • Added new dist using makefile script

+ Added toRadians function in Boilerplate.js
+ Added a demo for the function
+ Added new dist using makefile script
@desandro

desandro commented Jun 5, 2019

Copy link
Copy Markdown
Member

Thanks for this contribution! I'm still evaluating if this feature is worth adding to Zdog and how. I think I may prefer Chris Gannon's suggestion of setting a Zdog-wide property like useDegrees. Otherwise, doing this conversion can be done on developer-side, with either your own helper function or inline:

rotate: { y: 45/360 * Zdog.TAU }

@webocs

webocs commented Jun 5, 2019

Copy link
Copy Markdown
Author

I see! no problem, this was pretty straightforward so I figured I could PR it even if it wasn't used!. Are you planning on converting to radians for each operation if the variable is set, thus leaving the engine as it is now. Or are you planning to change the math of the rotateProperty function?

function rotateProperty( vec, angle, propA, propB ) {
  if ( !angle || angle % TAU === 0 ) {
    return;
  }
  // It could be as simple as this??.  unless you want to change the math
  angle =  useDegrees? angle/360 * Zdog.TAU : angle;

  var cos = Math.cos( angle );
  var sin = Math.sin( angle );
  var a = vec[ propA ];
  var b = vec[ propB ];
  vec[ propA ] = a*cos - b*sin;
  vec[ propB ] = b*cos + a*sin;
}

@desandro

desandro commented Jun 5, 2019

Copy link
Copy Markdown
Member

Yeah, I was thinking a library-wide change in rotateProperty. I'm waffling on this feature as it feels un-Zdog-like in that its extra stuff for personal preference. I'm now thinking a good approach is to make rotateProperty a proper Vector method so you could duck punch it.

// if Vector.prototype.rotateProperty were a thing
Zdog.Vector.prototype.rotateProperty = function ( angle, propA, propB ) {
  if ( !angle || angle % 360 === 0 ) {
    return;
  }
  angle = angle / 360 * Zdog.TAU;
  var cos = Math.cos( angle );
  var sin = Math.sin( angle );
  var a = vec[ propA ];
  var b = vec[ propB ];
  this[ propA ] = a*cos - b*sin;
  this[ propB ] = b*cos + a*sin;
};

Another solution is to add type checking. Any string ending with deg would be treated as a degree. You could pass in '180deg'. So Zdog would support both degrees and radians interchangably.

function rotateProperty( vec, angle, propA, propB ) {
  var isDegree = typeof angle == 'string' && angle.match( /deg$/ );
  if ( isDegree ) {
    angle = parseFloat( angle ) / 360 * TAU;
  }
  if ( !angle || angle % TAU === 0 ) {
    return;
  }
  var cos = Math.cos( angle );
  var sin = Math.sin( angle );
  var a = vec[ propA ];
  var b = vec[ propB ];
  vec[ propA ] = a*cos - b*sin;
  vec[ propB ] = b*cos + a*sin;
}

@webocs

webocs commented Jun 5, 2019

Copy link
Copy Markdown
Author

The second option sounds great and more natural if you ask me. I imagine some people copying demos from the internet that won't work because they missed the config line at the beginning. This approach will eliminate that hazard.

@chrisgannon

chrisgannon commented Jun 7, 2019

Copy link
Copy Markdown

I think passing a string like '180deg' is cumbersome and unnecessary - passing a number for degrees would be ideal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants