search

spite / THREE.MeshLine

Mesh replacement for THREE.Line
MIT License
2.13k stars 379 forks source link

Confusing options documentation #64

Open Venryx opened 5 years ago

Venryx commented 5 years ago

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 commented 5 years ago

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