Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Made MAX_WIND_INTENSITY configurable. Fixed links to canvas_layer.js in the examples. #9

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

paarthneekhara
Copy link

Making MAX_WIND_INTENSITY configurable allows plotting vectors other than wind of different scale (say sea currents).

@@ -83,7 +80,9 @@ export class Renderer {
// Line width of a drawn particle.
particleWidth: 2,
// Reduce particle count to this fraction (improves FPS).
particleReduction: 0.1
particleReduction: 0.1,
// Wind velocity at which particle intensity is maximum (m/s).
Copy link
Owner

@dannycochran dannycochran Aug 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense to have this option configurable, but you also need to add a line after 141 so this gets updated when a user calls start or update:

this.config_.maxWindIntensity = config.maxWindIntensity || this.config_.maxWindIntensity;

Can you also add a line to wind/typedefs.js after line 52 that indicates this option is configurable? Something like:

* maxWindIntensity: (!number=) An optional integer for changing the max wind intensity.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Thanks for pointing that out. I'll fix it in a day or two.

@dannycochran
Copy link
Owner

Apologies for the slow reply, thanks for sending this along. Had just a few comments -- will merge when I hear back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants