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

[p5.js 2.0 RFC Proposal]: API audit #7460

Open
3 of 21 tasks
GregStanton opened this issue Jan 8, 2025 · 14 comments
Open
3 of 21 tasks

[p5.js 2.0 RFC Proposal]: API audit #7460

GregStanton opened this issue Jan 8, 2025 · 14 comments

Comments

@GregStanton
Copy link
Collaborator

GregStanton commented Jan 8, 2025

Increasing access

This proposal would make p5.js 2.0 and later versions more beginner friendly, in three stages:

  1. Establish consensus on a set of criteria and guidelines for the API.
  2. Resolve complications in the current API.
  3. Implement a simple process for preventing complications in the future.

Which types of changes would be made?

  • Breaking change (Add-on libraries or sketches will work differently even if their code stays the same.)
  • Systemic change (Many features or contributor workflows will be affected.)
  • Overdue change (Modifications will be made that have been desirable for a long time.)
  • Unsure (The community can help to determine the type of change.)

Most appropriate sub-area of p5.js?

  • Accessibility
  • Color
  • Core/Environment/Rendering
  • Data
  • DOM
  • Events
  • Image
  • IO
  • Math
  • Typography
  • Utilities
  • WebGL
  • Build process
  • Unit testing
  • Internationalization
  • Friendly errors
  • Other (specify if possible)

What's the problem?

The p5.js API contains quite a few inconsistencies and other forms of complexity. This complexity makes p5.js harder to learn and harder to use, since users experience it directly through its API. While some parts of the API have been simplified for 2.0, such as the API for vertex functions, other complexities remain. This is because identifying and preventing complexities in a large API is difficult without clear guidelines and a practical quality assurance process.

What's the solution?

1. Establish criteria and guidelines

In order to identify and prevent unnecessary complexity in the p5 API, we can develop a list of criteria we want it to satisfy. We can also provide guidelines for common types of features, such as getters and setters. Initial criteria and guidelines are listed below.

Criteria

  1. Consistency: Provide a similar interface for similar features and different interfaces for different features.
  2. Readability: Design features that are easy to read and understand.
  3. Writability: Make features easy to use and hard to misuse.
  4. Predictability: Respect the principle of least surprise, so that users can guess how features work.
  5. Extensibility: Hide implementation details and lay a foundation for add-on libraries.
  6. Economy: Limit redundant features, and consider add-on libraries for new features.

For a given feature, these design criteria may be considered with respect to naming, syntax, and behavior.

Guidelines

  • Getters and setters:
    • A function or method with the same name as a setting, such as stroke() and fill(), should act jointly as a getter (when no argument is passed) and a setter.
    • General setters such as vertexProperty(name, value) should act as getters when value is omitted.
    • For consistency with commonly used functions such as stroke(), joint getters/setters should be used when possible. However, if separate getters and setters are to be used, they should begin with “get” and “set,” except for getters for Booleans, which should have names such as isLooping().
    • Something about setting fields directly? (Guidance might prevent inconsistencies like using the field disableFriendlyErrors to toggle friendly errors and using functions like fullscreen() and autoSized() to toggle other settings.)

These guidelines are included as examples. During the second phase, when specific kinds of issues are dealt with, the guidelines can be fleshed out.

References:

2. Perform an audit (a scavenger hunt!) and fix problems

Under ideal circumstances, the community could hold a scavenger hunt for features that don't adhere to p5's API criteria. Beginners could make significant contributions by spotting naming inconsistencies, for example. However, since p5.js 2.0 will be released relatively soon, I performed a rapid audit of the full API, to get the ball rolling.

Methodology

I skimmed through the whole reference and looked for issues that jumped out to me as potential areas of concern (I didn’t systematically apply all criteria to each feature). I also skipped over some things that I didn’t suspect were problematic.

Caveats:

Results

For illustration, only a partial sample of results from the rapid audit are included here. Skimming this section should give a rough sense of what's at stake.

Note: The sample includes examples of potential problems with naming, syntax, and semantics (broadly defined). Each example is tagged with the criteria that fail to be satisfied.

This partial sample of API problems can be replaced with separate GitHub issues, with one issue for each affected section of the reference. Those issues can be linked from here, in a checklist.

Note: The sample only includes issues that may require breaking changes, but the audit did reveal other issues that could be fixed with non-breaking changes after the release of p5.js 2.0.

Naming
  • color()
    • Relevant criteria: Consistency, predictability
    • Problem: Unlike other constructor functions in p5, this function does not start with a “create” prefix, leading users to guess that it’s a setter such as stroke().
    • Potential fix: Replace color() with createColor().
  • getTargetFrameRate()/frameRate()
    • Relevant criteria: Consistency, predictability
    • Problem: In this getter-setter pair, the getter and setter use different names for the same setting. The getter uses the name “target frame rate” but the setter uses the name “frame rate.” Also, the getter is prefixed with “get” but the setter is not prefixed with “set.”
    • Potential fix: Remove getTargetFrameRate() and turn frameRate() into a joint getter/setter.
  • setAttributes()
    • Relevant criteria: Consistency, readability, predictability
    • Problem: This feature has a couple of issues. First, it has a plural name, but setAttributes(key, value) only sets a single attribute. Second, the name is ambiguous, since it only sets attributes of the WebGL drawing context, but this isn't advertised in the name.
    • Potential fix: Both of these issues could potentially be fixed by replacing setAttributes() with, say, canvasProperty(key, value).
  • onended()
    • Relevant criteria: Consistency, readability
    • Problem: This feature’s name doesn’t use camel case, the name is awkward linguistically, and it’s inconsistent with similar methods of p5.Element, like touchEnded().
    • Potential fix:: Replace onended() with playEnded(). (However, there is a deeper issue, which is that these functions take a callback function, whereas event handlers like doubleClicked() are defined directly by the user. If one of these two behaviors cannot be used in all cases, then they should be distinguished by their names. For example, a name like onTouchEnd() could be used for one type of behavior and a name like touchEnded() could be used for another.)
Syntax
  • createModel(), loadModel()
    • Relevant criteria: Consistency, readability, writability
    • Problem: The same parameters in these functions appear in different orders (normalize and fileType are out of order in one signature, fileType is out of order in another, and fileType is missing from one signature).
    • Potential fix: One option is to make the parameter lists consistent. Another is to choose a different API that allows the parameters to be specified in any order (and there are potentially multiple such APIs).
  • spotLight(), directionalLight() and pointLight()
    • Relevant criteria: Readability, economy
    • Problem: As an example, spotLight() has eight signatures. It accepts three vectors as input, but in some signatures, some of these vectors are specified as p5.Vector objects whereas others are not. This makes the documentation unnecessarily complicated and leads to code that’s hard to read. The other listed features have the same type of issue.
    • Potential fix: A potential fix is to remove the signatures that mix componentwise specifications and p5.Vector specifications. The pure componentwise signature of spotLight() has 11 parameters and is very hard to read, but keeping both the pure componentwise and pure p5.Vector signatures reduces breaking changes and keeps these features consistent with other functions that take multiple vectors as input, such as quad(). (Since quad() takes up to a whopping 14 parameters, and since point() already accepts both componentwise and p5.Vector input, it makes sense to add support to quad() and all other 2D primitive functions for both types of input.)
  • p5.Graphics, p5.Framebuffer
    • Relevant criteria: Consistency, predictability
    • Problem: The reference says these classes are “similar,” and it’s true. Semantically, they are similar. However, syntactically, they’re significantly different. Requiring users to learn different interfaces for the same kinds of features reduces accessibility. If these aren’t reconciled, complications are likely to compound (e.g. the p5.Shape class developed for 2.0 has an interface that’s closer to that of p5.Graphics, and it may be exposed to users in a future version).
    • Potential fix: Requires discussion.
Semantics
  • addClass(), class()
    • Relevant criteria: Readability, economy
    • Problem: The p5.Element class has addClass() and class(), both of which add a class to the element. The difference is that class() is a joint getter/setter that seems to make addClass() unnecessary. Having both is likely to confuse users.
    • Potential fix: Remove addClass().
  • millis()
    • Relevant criteria: Consistency, readability, predictability
    • Problem: Unlike all other time and date functions, millis() returns the time since the sketch started running; all other functions, including second(), return the current time (e.g. if a sketch is run 32 seconds after noon, then when the sketch starts running, second() will return 32 whereas millis() will return 0). Also, “millis” is an abbreviation, unlike the other function names; that doesn’t seem like enough to convey the difference in behavior.
    • Potential fix: Requires discussion.
  • debugMode()
    • Relevant criteria: Readability, writability, extensibility
    • Problem: This function has multiple signatures, which take up to nine numerical parameters. Extending this feature is desirable but impractical, given that it’s already complex. Also, “mode” is a problematic description of this feature, since it actually provides different helpers (a grid helper and an axes helper) which can be combined, rather than modes, which are typically mutually exclusive.
    • Potential fix: The three.js library provides a GridHelper, an AxesHelper, and a variety of other helpers (a camera helper, a spotlight helper, etc.). In p5, having separate axisHelper() and gridHelper() functions should be an improvement. We might also consider functions like axesHelperProperty(key, value) for greater readability and writability (effectively allowing named parameters).
  • 2D Primitives
    • Relevant criteria: Consistency, predictability
    • Problem: Whereas quad() has multiple signatures and can create a shape in 2D or 3D, triangle() only works in 2D. Other issues include shapes that can be drawn in WebGL mode but only by specifying xy-coordinates rather than xyz-coordinates. Some of these shapes support a detail parameter while others don’t. Some support p5.Vector arguments and others don’t. Rounding corners in rect() and square() adds another complication.
    • Potential fix: The first thing is to make a table with one row per feature, and columns for dimensions, vector arguments, and so on. This will help us identify the exact issues that need to be addressed, and which of them would require breaking changes to fix (@davepagurek and I mostly finished the table).

3. Implement quality assurance process via issue templates

Bugs can be fixed in any version of p5. API problems cannot be fixed outside of a major version release (if at all), if they're like any of the problems listed above. To avoid introducing such problems in the future, we can implement a basic quality assurance process via the issue templates for new features and existing feature enhancements:

  1. Include a section for code examples. The community can help with filling this in, so that early-stage ideas are not discouraged.
  2. Include a checklist, to be completed before releasing the feature and closing the issue. The checklist can indicate criteria and guidelines. For each checklist item, “good” and “bad” examples can be provided (either directly on the issue template, or via a link to the contributor docs).
  3. Indicate what to expect after submitting a new-feature request:
    a. Documentation will be required before release (and encouraged before implementation).
    b. Review will be required by multiple stakeholders.

Volunteers

If you're interested in helping with this issue, you can leave a comment, and I can add your username here. Once separate issues have been created for the affected sections of the reference, I'll add a comment on this issue and tag everyone listed to let them know.

@dhruvinjs, @ImRAJAS-SAMSE, @Vaivaswat2244

Pros (updated based on community comments)

  1. Consistency: Inconsistencies will be fixed or prevented.
  2. Readability: Confusion about the meaning of code will be reduced or prevented.
  3. Writability: Features will be easier to use and harder to misuse.
  4. Predictability: Unpleasant surprises will be eliminated or prevented.
  5. Extensibility: Features will be more extensible, both for the core p5 library and for add-ons.
  6. Economy: Confusion or cognitive overload from redundant features will be reduced or prevented.

Cons (updated based on community comments)

  1. Breaking changes: Existing code may break. (However, we can address this with a compatibility add-on. Also, since we've already made some breaking changes, the marginal cost of new breaking changes for users is lower: users who are aware of the breaking changes we've already made will already know about the compatibility add-on and may already have a process for fixing legacy code.)
  2. Time expenditure: Resolving this issue will take some time. (However, we can speed it up by inviting more contributors. This issue is largely beginner friendly. Beginners can help by telling us when features are confusing to them, by identifying complexities, or by implementing changes such as name changes.)

Proposal status

Under review

@davepagurek
Copy link
Contributor

Thanks for getting these all together Greg! Generally agreed with everything. Here are a few comments on some of the points:


color()

  • Potential fix: Replace color() with createColor().

This definitely makes sense consistency, but this is also a pretty old API that has been around since the start of Processing. It likely not called create* then because it would return a value data type instead of an object data type, but now in p5, it is in fact an object with state. Although we generally shy away from aliases, this one might be important enough to keep an alias for it if we change it.

frameRate() vs getTargetFrameRate

  • Potential fix: Remove getTargetFrameRate() and turn frameRate() into a joint getter/setter.

frameRate() with no arguments is actually already a getter, but it returns the actual frame rate of the sketch, not the frame rate you asked for (e.g. if you called frameRate(60) but your sketch is lagging, frameRate() might return something lower like 10, but getTargetFrameRate() would return 60 still.) For consistency, we'd maybe want to make frameRate() return whatever target rate you asked for, remove getTargetFrameRate, and make a separate method called measureFrameRate() or something like that to indicate it's different?

media.onended()

  • Problem: This feature’s name doesn’t use camel case, the name is awkward linguistically, and it’s inconsistent with similar methods of p5.Element, like touchEnded().

  • Potential fix:: Replace onended() with playEnded(). (However, there is a deeper issue, which is that these functions take a callback function, whereas event handlers like doubleClicked() are defined directly by the user. If one of these two behaviors cannot be used in all cases, then they should be distinguished by their names. For example, a name like onTouchEnd() could be used for one type of behavior and a name like touchEnded() could be used for another.)

Agreed that this should be camel-cased at least, and maybe have the on removed. I don't think the callback behaviour is inconsistent though: you can define function doubleClicked() { ... } globally to handle double clicks anywhere, and you can also call it with a callback on a specific p5.Element to handle the event on that element alone with yourElement.doubleClicked(() => { ... }).

millis()

  • Problem: Unlike all other time and date functions, millis() returns the time since the sketch started running; all other functions, including second(), return the current time (e.g. if a sketch is run 32 seconds after noon, then when the sketch starts running, second() will return 32 whereas millis() will return 0). Also, “millis” is an abbreviation, unlike the other function names; that doesn’t seem like enough to convey the difference in behavior.

This is a good point -- millis() is used to control the timing of your animation in real time as an alternative to frameCount, which drops frames, but it's grouped with a bunch of other date and time related methods.

Context: why you might want to use millis vs frameCount Say you want it to take 5s for a circle to move across the canvas. If your animation is supposed to be 60fps but it can only keep up at 30fps, if you time everything based on `millis()`, after 5s have passed, the circle will still be where you expect it, it'll just look choppier as it animates there since it will only have redrawn 150 times instead of 300. If you instead say that the circle should be at the other side after 5\*60=300 frames, then if frames are drawing more slowly, it takes 10s to complete the animation, but it still draws all 300 frames.

The former is good if you want realtime playback. The latter is good if you want to do an export and ensure every frame gets rendered.

Maybe the thing to do here is keep millis() for its current purpose, and make millisecond() for date/time related things, to be consistent with the naming of the others? And then move millis() to the Environment category (where frameCount is), and keep millisecond() in the date/time category?

2D vs 3D in primitives

  • Problem: Whereas quad() has multiple signatures and can create a shape in 2D or 3D, triangle() only works in 2D. Other issues include shapes that can be drawn in WebGL mode but only by specifying xy-coordinates rather than xyz-coordinates. Some of these shapes support a detail parameter while others don’t. Some support p5.Vector arguments and others don’t. Rounding corners in rect() and square() adds another complication.

If we want to add 3D coordinate support as overloads to these methods, I believe the only one that would cause problems is rect(), because of the corner radii. e.g. these two signatures have the same number of arguments, so we can't tell them apart:

rect(x, y, z, w, h)
rect(x, y, w, h, r)

This could maybe be addressed by changing the syntax for corner radius, e.g.:

rect(x, y, z, { r: 4 }) // set all at once
rect(x, y, z, { tl: 4, tr: 4, br: 8, bl: 8 }) // set individually

p5.Framebuffer vs p5.Graphics APIs

  • Problem: The reference says these classes are “similar,” and it’s true. Semantically, they are similar. However, syntactically, they’re significantly different. Requiring users to learn different interfaces for the same kinds of features reduces accessibility. If these aren’t reconciled, complications are likely to compound (e.g. the p5.Shape class developed for 2.0 has an interface that’s closer to that of p5.Graphics, and it may be exposed to users in a future version).

For some extra context for others reading this on why framebuffers have a different syntax than graphics:

  • There is a performance cost when switching between different draw targets, so the API encourages drawing to framebuffers in batches
  • Framebuffer as a concept live within the rendering context and share all other state. To avoid setting lots of state back and forth to make it appear to the user that they are separate, the API also is set up to share state with the main context
  • Not having to prefix every call is a feature too:
    • it's easy to forget the prefix
    • not having them makes it easier to refactor code that used to draw to the main canvas to now draw to a layer
    • for addons, it's possible to transparently draw to a layer under the hood without the user's involvement at all (see the p5.FilterRenderer addon for examples of APIs that take advantage of this)

The first two points are framebuffer-specific, and probably mean framebuffers won't get an API that looks like graphics. The latter could apply to both, and is possibly a reason to give graphics a framebuffer-like API in the future (people asking how to do that for p5.Graphics comes up in the Discord every once in a while, anecdotally.)

@GregStanton
Copy link
Collaborator Author

GregStanton commented Jan 8, 2025

Thanks @davepagurek! I really appreciate your thorough explanations.

In case anyone else has comments, we're planning to move discussion of particular API changes to separate issues for each affected section of the API. I'll also make a separate issue for the quality assurance process and link to it from this issue. We'll dedicate discussion in the current issue to the API design criteria and guidelines.

P.S. I like your use of a dropdown for extra context. I've been using footnotes, which violate the spatial contiguity principle that we discussed haha

@dhruvinjs
Copy link

I would like to work on it as i am new here so can u please help and guide me !

@ImRAJAS-SAMSE
Copy link

ImRAJAS-SAMSE commented Jan 11, 2025

Hi, I’m Rajas, I’ve been using p5.js for the past three months. I’ve grown to enjoy its creative potential. Over the past week, I’ve been focused on understanding and contributing to the p5.js community.

I came across an issue related to naming conventions that I believe can be improved.


Naming

  • Target: elt

    • Relevant Criteria: Clarity, readability

    • Problem:
      In my opinion, the name elt is unclear and non-descriptive, making it less accessible to us new developers. While it likely stands for "element," this abbreviation is inconsistent with the more descriptive naming conventions used elsewhere in the library.
      For example, functions like createCanvas() and createDiv() use clear and descriptive names that help us understand their purpose immediately. However, elt as a property of p5.Element objects doesn’t provide the same level of clarity.

    • Potential Fix:
      Replace elt with a more descriptive name such as domElement or htmlElement. This change would:

      • Align it with the descriptive naming convention used in the library.

@GregStanton
Copy link
Collaborator Author

GregStanton commented Jan 13, 2025

Thanks @dhruvinjs and @ImRAJAS-SAMSE! I've added your usernames in the body of the proposal under a new Volunteers section. I'm planning to make separate GitHub issues for each affected section of the reference, and another issue for discussion of the quality assurance process. Once I do, I can tag you in a comment to let you know. This way, we can keep the discussion in this issue focused on API design criteria and guidelines. Does that all sound okay?

@dhruvinjs: We'd be glad to help you get started! There are two great ways you can help out. First, you could help identify potential problems. When I make the separate GitHub issues, I'll include the potential problems that I've already found. Based on those examples, you can try to find other potential issues that I missed during the rapid audit. Second, you could potentially make a contribution to the source code by implementing any changes that are approved; this could be quite helpful, and many of these changes may be relatively straightforward (for example, if they only involve changing the name of a function).

@ImRAJAS-SAMSE Thanks so much! I appreciate you bringing this to everyone's attention, and it's helpful that you used the same format as for the issues that were already noted :) The consistency criterion may also be relevant in this case since the name of the class is p5.Element not p5.Elt.

Abbreviations: A few other examples

Abbreviations are potentially an important area for improvement. In the Conversion section of the reference, "string" is abbreviated as str() whereas "Boolean" is not abbreviated as bool(), for example. Another example is the String Functions section, which includes the relatively cryptic abbreviations nf(), nfc(), nfp(), and nfs() (presumably "nf" is for "number format," but this may not be clear when reading code). Some other examples I found during the rapid audit are the mag() and setMag() methods of p5.Vector(), and millis() under Time & Date (it's the only one of the functions in that section that's abbreviated).

Abbreviations: Opportunity for new contributors

Here's a helpful thing you could both help with: create an inventory of all abbreviations in the p5 API. Perhaps you and @dhruvinjs could divide up the work for that? Let us know if you're interested, but before you spend all the time to do that, it would be good if we can reach a consensus on a guideline for abbreviated names.

Abbreviations: Guidelines?

I'm planning to propose other guidelines on naming and syntax, but for now, do you have any thoughts on a guideline for avoiding abbreviations in names @ksen0, @limzykenneth, @davepagurek? Some rough options to consider are (a) avoiding all abbreviations, (b) avoiding nonstandard abbreviations, or (c) allowing all abbreviations. I tend to agree with @ImRAJAS-SAMSE that abbreviations should generally be avoided. We may need to weigh that against the cost of breaking changes, but we can also consider options for reducing the impact of breaking changes.

Policy for reducing the impact of breaking changes?

Although it's a slight digression from the current conversation, it might be helpful to first settle on a policy for reducing the impact of breaking changes. Here's one policy that might be reasonable:

  1. Deprecation: We could use the deprecation tag for features with a name that's changed and features that are planned for removal. In the reference, the page for the deprecated feature could clearly notify users that support may be fully dropped in a potential p5.js 3.0. (Perhaps deprecated features could be grouped into a "Deprecated" section.)
  2. Compatibility add-on: For situations where a feature's name stays the same but its syntax or behavior changes, a compatibility add-on could be provided and supported for a limited time, per 1.x Compatibility Addon #7230.
  3. Aliasing: This could be reserved for cases where multiple names need to coexist indefinitely; it may be best to avoid this entirely if possible, but it might be good to keep it as an option in exceptional cases that (for some reason) can't be effectively handled by one of the first two options.

@ImRAJAS-SAMSE
Copy link

ImRAJAS-SAMSE commented Jan 13, 2025

Thanks for the update, @GregStanton The plan sounds great, I'm happy to collaborate on the tasks and contribute as needed.

About the inventory of abbreviations in the p5 API, I'd be glad to help create it. Once you create the GitHub issue for this, please tag me, and I'll get started. If there's an opportunity to coordinate with others, like @dhruvinjs, yes we can divide the work for better efficiency.

As for the guidelines on abbreviations, I think it's an important area to address, and I'm looking forward to hearing everyone's thoughts on it.

@dhruvinjs
Copy link

Thanks for the update @GregStanton I'll start contributing the source code and get familiar with you code base

@Vaivaswat2244
Copy link

Hi everyone! I'm new to p5.js community and interested in contributing. I'd like to help with this API improvement initiative.

I'm particularly interested in helping with:

  • Creating the inventory of abbreviations in the API, which was mentioned as a good task for new contributors
  • Implementing approved name changes or other straightforward code updates

Please let me know if any of these would be helpful, or if there are other ways I could contribute as someone new to the project.

@dhruvinjs
Copy link

Hey @GregStanton
Is the color() and getTargetFrameRate()/frameRate()
Are these issue fixed, if not then can i fix them?

@dhruvinjs
Copy link

Changes to p5.js Library: Introducing createColor

Summary

This document outlines the changes made to the p5.js library by replacing the color() method with createColor() for better modularity and extensibility. The changes include updates to the core implementation, dependent methods, and relevant test cases.


Changes Made

1. New Method: createColor

The color() method was replaced with createColor() in the p5.prototype. The new method retains the functionality of the original color() method while ensuring the naming reflects modern conventions.

Updated Code:

p5.prototype.createColor = function (...args) {
  p5._validateParameters('color', args);
  if (args[0] instanceof p5.Color) {
    return args[0]; // Do nothing if argument is already a color object.
  }

  const arg = Array.isArray(args[0]) ? args[0] : args;
  return new p5.Color(this, arg);
};

2. Changes to Dependent Methods

All methods in p5.js that relied on color() were updated to use createColor() instead.

Examples:

  • Before:
    p5.prototype.alpha = function(c) {
      return this.color(c)._getAlpha();
    };
  • After:
    p5.prototype.alpha = function(c) {
      return this.createColor(c)._getAlpha();
    };
  • LerpColor method with createColor()
p5.prototype.lerpColor = function(c1, c2, amt) {
  p5._validateParameters('lerpColor', arguments);

  if (!(c1 instanceof p5.Color)) {
    c1 = this.createColor(c1); // Replaced color() with createColor()
  }
  if (!(c2 instanceof p5.Color)) {
    c2 = this.createColor(c2); // Replaced color() with createColor()
  }

  const mode = this._colorMode;
  const maxes = this._colorMaxes;
  let l0, l1, l2, l3;
  let fromArray, toArray;

  if (mode === constants.RGB) {
    fromArray = c1.levels.map(level => level / 255);
    toArray = c2.levels.map(level => level / 255);
  } else if (mode === constants.HSB) {
    c1._getBrightness(); // Cache hsba so it definitely exists.
    c2._getBrightness();
    fromArray = c1.hsba;
    toArray = c2.hsba;
  } else if (mode === constants.HSL) {
    c1._getLightness(); // Cache hsla so it definitely exists.
    c2._getLightness();
    fromArray = c1.hsla;
    toArray = c2.hsla;
  } else {
    throw new Error(`${mode} cannot be used for interpolation.`);
  }

  // Prevent extrapolation.
  amt = Math.max(Math.min(amt, 1), 0);

  // Define lerp here itself if user isn't using math module.
  if (typeof this.lerp === 'undefined') {
    this.lerp = (start, stop, amt) => amt * (stop - start) + start;
  }

  // Perform interpolation.
  if (mode === constants.RGB) {
    l0 = this.lerp(fromArray[0], toArray[0], amt);
  } else {
    if (Math.abs(fromArray[0] - toArray[0]) > 0.5) {
      if (fromArray[0] > toArray[0]) {
        toArray[0] += 1;
      } else {
        fromArray[0] += 1;
      }
    }
    l0 = this.lerp(fromArray[0], toArray[0], amt);
    if (l0 >= 1) { l0 -= 1; }
  }
  l1 = this.lerp(fromArray[1], toArray[1], amt);
  l2 = this.lerp(fromArray[2], toArray[2], amt);
  l3 = this.lerp(fromArray[3], toArray[3], amt);

  // Scale components.
  l0 *= maxes[mode][0];
  l1 *= maxes[mode][1];
  l2 *= maxes[mode][2];
  l3 *= maxes[mode][3];

  return this.createColor(l0, l1, l2, l3); // Updated here as well
};

Let me know if you need help with implementing or debugging the createColor() function!
Similar changes were applied to methods like:

  • brightness()
  • blue()
  • red()
  • green()

I apologize for bothering you @GregStanton , but as I’m new to open-source contributions, I’m a bit confused about where I should test these changes. Could you kindly guide me on how to proceed? I assure you that once I have clarity on this, I won’t disturb you with other issues unless I face a problem.

@GregStanton
Copy link
Collaborator Author

Thanks so much @Vaivaswat2244! I've added your name to the list of volunteers (in the body of this issue) and will tag you in a comment once everything is a bit more organized. There may be other things you'll be able to help with, like an inventory of getters and setters. I have a few things happening this week, but I'll try to have things ready by week's end.

@GregStanton
Copy link
Collaborator Author

Hi @dhruvinjs! No problem at all. Your help is much appreciated! The contributor docs are a good way for you to get oriented.

For context, this issue about the API audit is currently part of work on p5.js 2.0, which is currently being prioritized. That's the next major version of p5.js, and it introduces some changes that may not be fully documented in the contributor docs. But, most of the contributor docs are still accurate. So, if you're interested to learn more about the contribution process, I'd recommend reading over them.

If you get to the section on prerequisites and aren't sure if you have enough knowledge, you might try working through Git and GitHub for Poets. This will introduce you to Git and GitHub, in case you're not already familiar with them.

The basic process is that the community will first agree on changes to be made, and after that, work can be assigned to individual contributors like you. Once you're assigned to work on a task, we can help you with any details related to 2.0 that aren't already documented. But basically, you'll make and test code changes on your own computer, and then you'll submit a pull request, so that your changes can be merged into the official version of p5 here on GitHub.

If you have any other questions, please don't hesitate to ask!

@limzykenneth
Copy link
Member

At this stage, we are not considering additional breaking API changes as we are moving closer to releasing p5.js 2.0 beta with the main aim of identifying bugs and other critical fixes for the full release. We have also gone through a full advisory process for the breaking changes that are included in 2.0 currently and for any additional breaking changes, especially those proposed here, it should have the same level of scrutiny before we can implement them.

That is to say at this late stage, it is unlikely that we will be accepting the proposed changes into 2.0.


For individual comments on proposals, see below

color

This will likely stay as is regardless because of backwards compatibility, not just with past version of the library but also with the wealth of teaching/learning materials out there for both p5.js and Processing. It will be incredibly disruptive to change this and we have received similar comments regarding this kind of changes from the Advisory Committee regarding similar proposed changes as well.

If required, an alias to createColor may be created but it is unlikely that color will be removed.

getTargetFramerate()

getTargetFramerate() and framerate() returns two different things, the former the target framerate as set by the user and the latter the measured framerate of the sketch. Both will likely need to exist instead of one replacing another.

addClass()/class()

These two also have different behaviours, addClass() adds a single class to the HTML element's class attributes and is the counterpart to removeClass(), class() sets the class attribute of the HTML element which completely replaces all existing classes on the HTML element.


There will be future opportunity to introduce potential breaking changes to p5.js but for 2.0 we are moving towards the final phase of the project and as such we don't want to start up new breaking changes proposals now. Feel free to continue the discussions here as needed and the new library syntax for p5.js 2.0 should also be flexible enough if anyone wants to implement the proposed changes here as an addon. Thanks.

@dhruvinjs
Copy link

So I just saw your message @limzykenneth, now I am currently retracting the changes from color to createColor() which I made and what are the other issues that I should fix !

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

No branches or pull requests

6 participants