search

dataarts / dat.guiVR

A flexible graphical user interface for changing variables within WebVR.
https://workshop.chromeexperiments.com/examples/guiVR/
Apache License 2.0
311 stars 50 forks source link

conflict of name 'name' #91

Open sjpt opened 6 years ago

sjpt commented 6 years ago

dat.guiVR uses a function name() to set the label. This conflicts with the pre-existing 'name' field in THREE objects.

This causes some issues, including breaking threejs inspector when inspecting dat.guiVR objects.

I suggest it be replaced by a get/set property 'label'; or (smaller change) renamed to 'label' or 'setName'. I can't see an easy way to do this without breaking upward compatibility.

xinaesthete commented 6 years ago

As I recall, this is not the only way in which dat.guiVR alters the properties of three objects, which could in general lead to issues like this emerging at unexpected times.

I somewhat considered refactoring so that the THREE objects would be members of the dat.guiVR objects, rather than changing them directly. I might still consider this, but I'm not sure how the original authors feel about it.

mrdoob commented 6 years ago

I would suggest using Object3D's userData instead.

xinaesthete commented 6 years ago

The library also redefines add function as well, for example, in a way that would break the similarity to regular dat.gui interface if it were changed.

PasGl commented 6 years ago

I stumbled over both naming-conflicts while working with dat.guiVR, causing me some headache. I'd prefer prioritizing threejs-compliance over datgui-similarity.

xinaesthete commented 6 years ago

I agree threejs compliance might be more important than similarity to the other API, especially given they're unlikely to remain completely equivalent. However, my proposed refactor would probably not take a massive amount of work to implement and would reduce the extent to which existing code using dat.guiVR was broken, as well as preserving the similarity. Apart from the effort required to implement and test it, that might be the best of both worlds.