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

Parameters for vertex() too confusing. #350

Open
Brahvim opened this issue Aug 20, 2022 · 7 comments
Open

Parameters for vertex() too confusing. #350

Brahvim opened this issue Aug 20, 2022 · 7 comments

Comments

@Brahvim
Copy link

Brahvim commented Aug 20, 2022

Issue description

The parameters are too confusing! v has two meanings.

URL(s) of affected page(s)

https://processing.org/reference/vertex_.html

Proposed fix

Please replace vertex(v) with something like vertex(a), then state that a is a float[].

@SableRaf SableRaf transferred this issue from processing/processing-docs Jan 6, 2023
@CubeTures
Copy link

Hi @SableRaf, I would love to take on this task. I think this should be a good starting task for a newcomer like me.

@CubeTures
Copy link

Okay, after looking into this issue, I have discovered additional problems with the documentation page. I have listed them here along with my suggested solutions.

Issues:

  • Two parameters titled "v," both of which are uninformative
  • The first "v" parameter has an option to be just a float, which does not actually compile.
  • The second"v" parameter has an option to be a float[], which does not compile either.

Solutions:

  • Change the first "v" parameter to be called "point" or "coordinate"
  • Remove the option for float for the first "v" parameter, as it does not work
  • Change the "u" parameter to be "xTexture"
  • Change the second"v" parameter to be called "yTexture"
  • Remove the option for float[] for the second"v" parameter, as it does not work

I will begin to work on getting a merge created. I believe this change will also require changing the parameter names in the actual proccesing4.

@Brahvim
Copy link
Author

Brahvim commented Mar 9, 2023

Hey there @CubeTures - just saying: thank you very much for noticing this!
Pretty stupid of me that I didn't just, think of doing this myself, back then...

I hope you like working on this, ":D!

@CubeTures
Copy link

Okay, that should do it. I hope I didn't miss any references of the method elsewhere. Please tell me if you notice anything!

@Brahvim
Copy link
Author

Brahvim commented Mar 10, 2023

Hello, @CubeTures.
How about you specify the parameters for the overload vertex(x, y, z, u, v) as vertex(x, y, z, s, t) instead?
(That is a well-known OpenGL convention!)

The last parameter in the "Parameters" section, v (float, float[]) is, as you called it, confusing, since it conflicts with the first one. How about removing it?

Also, as you mentioned, vertex(v) does not actually work with just a single float anywhere. Could you please also fix this error in the docs?

Also, an explanation for what VERTEX_FIELD_COUNT is, would be beneficial (and appreciated! ":D).

@CubeTures
Copy link

Alright, I redid my pull request to better reflect the conventions you listed, as well as clarified the vertex field count. Thanks for letting me know!

@Brahvim
Copy link
Author

Brahvim commented Mar 10, 2023

Thank you! I went through the commits, they look nice to me. Hope to see these changes on the site soon! ":D

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

No branches or pull requests

2 participants