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

Confusing options documentation #64

Open
Venryx opened this issue Jul 30, 2018 · 1 comment
Open

Confusing options documentation #64

Venryx opened this issue Jul 30, 2018 · 1 comment

Comments

@Venryx
Copy link

Venryx commented Jul 30, 2018

In the Readme, there are confusing descriptions of the options.

One example:

sizeAttenuation - makes the line width constant regardless distance (1 unit is 1px on screen) (0 - attenuate, 1 - don't attenuate)`
lineWidth - float defining width (if sizeAttenuation is true, it's world units; else is screen pixels)

There's three things that are confusing here.

  1. The name sizeAttenuation, taken on its own, makes it sounds like true would enable attenuation, and false would disable it. That's apparently not the case: the parentheses section says 0/false enables attenuation, and 1/true disables attenuation. This "meaning opposite of what the name implies" gives uncertainty to the reader of whether it's actually reversed, or just a typo.
  2. Why does the parentheses say to use 0 or 1, when the line below it references sizeAttenuation as possibly being true? I'm guessing it accepts either 0/1 or true/false . However, if that's the case, you should pick one and use it consistently in the documentation instead of saying 0/1 some places and true/false others.
  3. Assuming the usages of 0/1 and true/false are standardized in the readme, and the "meaning reverse of the option name" is fixed, it seems like it would be good to standardize on true/false since it reinforces the assumption that the value directly maps in meaning to the option's name. In other words, "sizeAttenuation: true" is slightly more obvious in its function of enabling size-attenuation than "sizeAttentuation: 1". (since 0/1 gives the possibility that there's other options like 2 or 3, or that the meanings are reversed, as seems to currently be the case)

Also, it would be nice if the readme showed the default values for the options. As it stands, I have to manually set the values for options which are probably already the default, just because I don't know whether they're actually the default -- and other options depend on their being a certain value.

Example:
lineWidth: [...] (if sizeAttenuation is true, it's world units; else is screen pixels)
near: [...] (REQUIRED if sizeAttenuation set to false)
But I don't know whether sizeAttenuation defaults to true or false, so I have to set it manually to use either of the settings above, even though it shouldn't be necessary in one of those cases.

@elifer5000
Copy link

This is indeed confusing. The name sizeAttenuation implies that with a true value, it would scale the line with the model, but the opposite is true.
If you look in the shader, the only check for this value is if( sizeAttenuation == 1. ), so any value other than 1 would be considered false.
https://github.com/spite/THREE.MeshLine/blob/master/src/THREE.MeshLine.js#L302

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