meshcat-dev / meshcat

Remotely-controllable 3D viewer, built on top of three.js
MIT License
250 stars 44 forks source link

Viewer.set_property can reference nested properties #177

Closed SeanCurtis-TRI closed 7 months ago

SeanCurtis-TRI commented 7 months ago

This comes with more feedback about attempts to set properties that don't exist (or can't be set).

Resolves #168


This change is Reviewable

SeanCurtis-TRI commented 7 months ago

src/index.js line 715 at r3 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Every single console warning or error deserves attention from a developer who is trying to figure out why their code isn't working. We should expect them to read all of them to make progress. If there are so many that they need to start filtering things to find the wheat in the chaff, then we have much bigger problems. I can buy the argument about "reporting real problems as warnings" is underselling. That's why I suggested error for everything. There's maybe something to be said that the two different colors (for different severities) will be easier to read somehow, but again -- there should be very few of these messages overall. I don't buy that adding more control flow for that reason is a good trade-off. > When the user references something that doesn't exist, this might reflect a typo or a case of out-of-order operations. There's not anything _intrinsically_ bad about what they're doing. This deserves a warning. I don't buy that calling "set_property" and have it do nothing because the arguments were invalid can really be termed a "warning" in any real way. They tried to set a property, but nothing happened. Doing calls "out of order" is a mistake.

One last question: given the two very different reasons for failure; what error message would you print?