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

getTargetFrameRate issue #5762 #5839

Merged
merged 4 commits into from
Nov 13, 2022
Merged

Conversation

sz245
Copy link
Contributor

@sz245 sz245 commented Oct 19, 2022

Our team (@sz245, @Jorge-Mor, @4slk4) worked on issue #5762. Thank you to @donaldong, our mentor, for guiding us through the process.

Added a method that returns the frame that the user sets (_targetFrameRate) when calling the function getTargetFrameRate().
Resolves #5762.

Changes:

/src/core/environment.js - line 331-338

Code change:

/**
 * Returns the current _targetFrameRate.
 * @private
 * @return {Number} current _targetFrameRate
 */
p5.prototype.getTargetFrameRate = function() {
  return this._targetFrameRate;
};

PR Checklist

    let j = frameRate(20);
    console.log("frameRate(): ", j.getFrameRate())
    console.log("targetFrameRate(): ", j.getTargetFrameRate())

@welcome
Copy link

welcome bot commented Oct 19, 2022

🎉 Thanks for opening this pull request! Please check out our contributing guidelines if you haven't already. And be sure to add yourself to the list of contributors on the readme page!

@limzykenneth
Copy link
Member

Is this function meant to be part of the private? If it is intended for public use it would be great to have a more complete documentation as well as not marking it as @private.

@sz245
Copy link
Contributor Author

sz245 commented Oct 21, 2022

@limzykenneth The function is meant for public use and is now marked as @method. The in-line documentation has also been updated and an example has been included.

Copy link
Member

@limzykenneth limzykenneth left a comment

Choose a reason for hiding this comment

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

One more changes inline.

Would you be able to add unit tests for this functions? No worries if not, we can add it later.

src/core/environment.js Outdated Show resolved Hide resolved
@limzykenneth limzykenneth merged commit 6893593 into processing:main Nov 13, 2022
@limzykenneth
Copy link
Member

Looks good. Thanks!

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.

Add a function to get the target frame rate
2 participants