Skip to content
This repository has been archived by the owner on Sep 7, 2022. It is now read-only.

conflict of name 'name' #91

Open
sjpt opened this issue Aug 4, 2017 · 5 comments
Open

conflict of name 'name' #91

sjpt opened this issue Aug 4, 2017 · 5 comments

Comments

@sjpt
Copy link

sjpt commented Aug 4, 2017

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
Copy link
Contributor

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
Copy link

mrdoob commented Aug 4, 2017

I would suggest using Object3D's userData instead.

@xinaesthete
Copy link
Contributor

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
Copy link

PasGl commented Aug 5, 2017

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
Copy link
Contributor

xinaesthete commented Aug 6, 2017

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.

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

No branches or pull requests

4 participants