Improve Node Blocks #600

tpjnorton · 2018-02-07T17:13:58Z

address #586.

omichel · 2018-02-12T07:55:51Z

I am not sure about the [list of strings], [any string], [lookup table] as it seems redundant with the MFString, SFString and lookupTable information present on the same line... I would simply make no comment in these cases.

stefaniapedrazzi · 2018-02-12T07:57:56Z

Also note that a PROTO is a node and we would like that there is no difference between basic nodes and PROTO nodes for users, so I do not think it is good to specify it with [node, PROTO].

tpjnorton · 2018-02-12T08:01:21Z

My reason for using the [node, PROTO] notation is primarily that other fields can admit either one specific node, or any PROTO instance based on that node - the material field of a Shape node can admit either a Material node or any Material-based PROTO.

I think it's important to put PROTOs at the forefront as they're powerful and actually very useful. I wonder whether more people would take the time to learn and use them if we highlight them more like this.

stefaniapedrazzi · 2018-02-12T08:09:45Z

But they are all nodes.
A Material node is a node and a PROTO based on Material is a node.
It is good to specify which specific kind of nodes can be inserted. But to me it is useless and confusing to not treat PROTO as nodes but to specify them in a string like [node, PROTO].

If we want to have a user-friendly and easy-to-use interface a users should not care if he is using a PROTO or a node. For him in the scene tree they should look exactly the same..
Moreover only advanced users will ever create PROTO nodes and need to really know the difference.

But we can better discuss this during the meeting.

omichel · 2018-02-12T10:17:34Z

reference/background.md

+  MFString frontUrl  [ ]   # [list of strings]
+  MFString leftUrl   [ ]   # [list of strings]
+  MFString rightUrl  [ ]   # [list of strings]
+  MFString topUrl    [ ]   # [list of strings]


In that case, I believe we might be more precise, like: # [list of image URLs]

This sounds good to me!

tpjnorton · 2018-02-12T13:39:18Z

I've fixed a couple of latent problems with the doc css here too.

tpjnorton · 2018-02-12T13:39:44Z

👍

omichel · 2018-02-13T07:21:46Z

reference/background.md

+  MFString leftUrl   [ ]   # [list of Image URLs]
+  MFString rightUrl  [ ]   # [list of Image URLs]
+  MFString topUrl    [ ]   # [list of Image URLs]
+  MFColor  skyColor  [ 0 0 0 ]    # [0,1]


Missing space after coma.

omichel · 2018-02-13T07:28:52Z

reference/compass.md

+  SFBool  xAxis       TRUE   # [TRUE, FALSE]
+  SFBool  yAxis       TRUE   # [TRUE, FALSE]
+  SFBool  zAxis       TRUE   # [TRUE, FALSE]
+  SFFloat resolution  -1     # [-1, [0, inf]]


Woow, that's not obvious to understand... Maybe we should use the standard mathematical representation for sets and intervals, which would translate into: { -1, [0, inf) }. If we use this scientific formalism, which I believe is good, we should also change all the [TRUE, FALSE] to {TRUE, FALSE} and [0, inf] to [0, inf), [ImageTexture, PROTO] to {ImageTexture, PROTO}, ["symmetric", "active", "passive"] to {"symmetric", "active", "passive"}, etc. We should also remove the square brackets around text describing strings to avoid confusion.

See explanations here: https://en.wikipedia.org/wiki/Set_notation

tpjnorton · 2018-02-13T10:40:40Z

how is this?

tpjnorton · 2018-02-13T10:40:44Z

👍

omichel

We are getting closer

omichel · 2018-02-13T10:49:37Z

reference/emitter.md

+  SFString type       "radio"   # {"radio", "serial", "infra-red"}
+  SFFloat  range      -1        # {-1, [0, inf)}
+  SFFloat  maxRange   -1        # {-1, [0, inf)}
+  SFFloat  aperture   -1        # [-1 ,[0, 2*pi]]


That looks odd... Shouldn't it be {-1, [0, 2*pi]} instead?

omichel · 2018-02-13T10:52:00Z

reference/hingejointparameters.md

+  SFVec3f axis                      1 0 0   # [0, 1]
+  SFFloat suspensionSpringConstant  0       # [0, inf)
+  SFFloat suspensionDampingConstant 0       # [0, inf)
+  SFVec3f suspensionAxis            1 0 0   # [0, 1]


Instead of [0, 1] for axes, which is not very well understandable, I would simply write unit axis.

omichel · 2018-02-13T10:54:05Z

reference/lens.md

-  SFVec2f center                 0.5 0.5
-  SFVec2f radialCoefficients     0 0
-  SFVec2f tangentialCoefficients 0 0
+  SFVec2f center                 0.5 0.5   # [0, 1]


That looks odd, maybe it would be better to write [0, 1] [0, 1] instead?

omichel · 2018-02-13T10:56:53Z

reference/motor.md

+  SFFloat  consumptionFactor 10       # [0, inf)
+  SFVec3f  controlPID        10 0 0   # [0, inf)
+  SFFloat  minPosition       0        # [(-inf, inf), [-pi, pi]]
+  SFFloat  maxPosition       0        # [(-inf, inf), [-pi, pi]]


That looks odd, I would rather write something like (-inf, inf) or [-pi, pi]

omichel · 2018-02-13T10:58:21Z

reference/motor.md

+  SFFloat  maxPosition       0        # [(-inf, inf), [-pi, pi]]
+  SFFloat  maxVelocity       10       # [0, inf)
+  SFString sound             ""       # any string
+  MFNode   muscles           []       # {Muscle (or derived PROTO)}


That's not consistent with other cases.

omichel · 2018-02-13T11:00:14Z

reference/recognition.md

-  SFBool   occlusion      TRUE    # should occlusions between the camera and the objects be checked
-  SFColor  frameColor     1 0 0   # color of the object's frame in the overlay
-  SFInt32  frameThickness 1       # thickness of the object's frame in the overlay (pixels)
+  SFFloat  maxRange       100     # [0, inf]


omichel · 2018-02-13T11:00:24Z

reference/recognition.md

+  SFInt32  maxObjects     -1      # {-1, [0, inf)}
+  SFBool   occlusion      TRUE    # {TRUE, FALSE}
+  SFColor  frameColor     1 0 0   # [0, 1]
+  SFInt32  frameThickness 1       # [0, inf]


omichel · 2018-02-13T11:01:56Z

reference/solid.md

+  SFFloat  translationStep     0.01          # [0, inf)
+  SFFloat  rotationStep        0.261799387   # [0, inf)
+  SFVec3f  linearVelocity      0 0 0         # [0, inf)
+  SFVec3f  angularVelocity     0 0 0         # [0, inf)


That's odd, why can't it be any vector?

omichel · 2018-02-13T11:03:22Z

reference/spotlight.md

+  SFFloat cutOffAngle      0.785398   # [0, pi/2)
+  SFVec3f direction        0 0 -1     # (-inf, inf)
+  SFFloat intensity        1          # [0, 1]
+  SFVec3f location         0 0 10     # (-inf, inf)


We have a problem with SFVec3f. Either we write "any vector" or something like "(-inf, inf) (-inf, inf) (-inf, inf)" or simply nothing. Similar problem with SFVec2f and SFRotation...

omichel · 2018-02-13T11:05:16Z

reference/touchsensor.md

-  SFFloat  resolution  -1
+  SFString type        "bumper"                  # ["bumper", "force", "force-3d"]
+  MFVec3f  lookupTable [ 0 0 0, 5000 50000 0 ]   # [see below]
+  SFFloat  resolution  -1                        # [-1, (0, inf)]


These 3 lines are wrong / non consistent.

tpjnorton · 2018-02-13T11:33:45Z

closer and closer still. I've fixed your comments and tried to improve SFVec3f, SFVec2f and SFRotation fields.

tpjnorton · 2018-02-13T11:34:22Z

👍

omichel

Still...

omichel · 2018-02-13T11:51:32Z

reference/physics.md

+  SFFloat density       1000   # {-1, [0, inf)}
+  SFFloat mass          -1     # {-1, [0, inf)}
+  MFVec3f centerOfMass  [ ]    # {-1, [0, inf)}
+  MFVec3f inertiaMatrix [ ]    # {-1, [0, 1]}


Those two lines are not understandable to me...

omichel · 2018-02-13T11:53:59Z

reference/touchsensor.md

-  MFVec3f  lookupTable [ 0 0 0, 5000 50000 0 ]
-  SFFloat  resolution  -1
+  SFString type        "bumper"                  # {"bumper", "force", "force-3d"}
+  MFVec3f  lookupTable [ 0 0 0, 5000 50000 0 ]   # see below


Not consistent: you used "lookup table" previously.

omichel · 2018-02-13T11:55:10Z

reference/fluid.md

+  SFString description    ""        # any string
+  SFFloat  density        1000      # {-1, [0, inf)} 
+  SFFloat  viscosity      0.001     # [0, inf)
+  SFVec3f  streamVelocity 0 0 0     # any positive vector


Does it really have to be positive?

omichel · 2018-02-13T11:56:14Z

reference/jointparameters.md

-  SFFloat dampingConstant 0     # damping constant (Ns/m or Nms)
-  SFFloat staticFriction  0     # friction constant (Ns/m or Nms)
+  SFFloat position        0       # [0, inf)
+  SFVec3f axis            0 0 1   # [0, 1]


→ unit axis

omichel · 2018-02-13T11:56:48Z

reference/muscle.md

-  SFBool   visible     TRUE
+  SFDouble maxRadius   0.2     # [0, inf)
+  SFVec3f  startOffset 0 0 0   # any positive vector
+  SFVec3f  endOffset   0 0 0   # any positive vector


Not sure they should be positive...

omichel · 2018-02-13T11:57:13Z

reference/propeller.md

-  SFNode device           NULL  # RotationalMotor
-  SFNode fastHelix        NULL  # Solid node containing a graphical representation for rotation speed > 24 π rad/s (720 rpm)
-  SFNode slowHelix        NULL  # Solid node containing a graphical representation for rotation speed <= 24 π rad/s
+  SFVec3f shaftAxis       1 0 0   # [0, 1]


omichel · 2018-02-13T11:58:57Z

reference/track.md

-  SFNode  animatedGeometry NULL
-  SFInt32 geometriesCount  10
+  MFNode  device           [ ]    # {LinearMotor, PositionSensor, Brake, PROTO}
+  SFVec2f textureAnimation 0 0    # any positive vector


Not sure it should be positive...

omichel · 2018-02-13T11:59:56Z

reference/worldinfo.md

+  SFFloat  physicsDisableAngularThreshold 0.01      # [0, inf)
+  SFNode   defaultDamping                 NULL      # {Damping, PROTO}
+  SFFloat  inkEvaporation                 0         # [0, inf)
+  SFVec3f  northDirection                 1 0 0     # [-1, 1]


tpjnorton · 2018-02-13T13:23:41Z

👍

omichel

Sorry, there still problems...

omichel · 2018-02-13T13:29:11Z

reference/motor.md

+  SFFloat  maxPosition       0        # (-inf, inf) or [-pi, pi]
+  SFFloat  maxVelocity       10       # [0, inf)
+  SFString sound             ""       # any string
+  MFNode   muscles           []       # {Muscle (or derived PROTO)}


Why not {Muscle, PROTO} as done everywhere else?

omichel · 2018-02-13T13:29:40Z

reference/muscle.md

+  SFDouble maxRadius   0.2     # [0, inf)
+  SFVec3f  startOffset 0 0 0   # any vector
+  SFVec3f  endOffset   0 0 0   # any vector
+  MFColor  colors      [ ]     # {SFColor, PROTO}


It's a list of RGB colors here, no PROTO allowed, SFColor is not a node!

omichel · 2018-02-13T13:33:37Z

reference/pen.md

-  SFFloat leadSize    0.002
-  SFFloat maxDistance 0.0   # >= 0.0
-  SFBool  write       TRUE
+  SFColor inkColor    0 0 0   # [0, 1]


Similar problem as with SFVec3f, we should should use "any color" here.

omichel · 2018-02-13T13:35:01Z

reference/propeller.md

+  SFVec2f torqueConstants 1 0     # any vector
+  SFNode device           NULL    # {Solid (or derived)}
+  SFNode fastHelix        NULL    # {Solid (or derived)}
+  SFNode slowHelix        NULL    # {Solid (or derived)}


We should add , PROTO to be consistent with endPoint.

omichel · 2018-02-13T13:35:20Z

reference/lidar.md

+  SFFloat  defaultFrequency     10        # [minFrequency, maxFrequency]
+  SFFloat  minFrequency         1         # [0, maxFrequency)
+  SFFloat  maxFrequency         25        # [minFrequency, inf)
+  SFNode   rotatingHead         NULL      # {Solid (or derived)}


tpjnorton · 2018-02-13T13:50:41Z

I've fixed the last couple of comments and updated the color fields according to your suggestion.

Sorry, I'm finding this PR hard to verify myself.

tpjnorton · 2018-02-13T15:07:40Z

👍

stefaniapedrazzi

I quickly reviewed only the SFNode and MFNode additional information but there are still some errors and inconsistencies.

stefaniapedrazzi · 2018-02-13T15:39:10Z

reference/appearance.md

-  MFNode   shaders          NULL
-  SFString name             "appearance"
+  SFNode   material         NULL           # {Material, PROTO}
+  SFNode   texture          NULL           # {ImageTexture, PROTO}


Missing "MultiTexture" and "ComposedCubeMapTexture"

stefaniapedrazzi · 2018-02-13T15:39:40Z

reference/hinge2joint.md

-  SFNode  jointParameters2 NULL # JointParameters for second axis
-  MFNode  device2          [ ]  # RotationalMotor, PositionSensor and Brake
-  SFFloat position2        0    # initial position with respect to the second hinge (rad)
+  SFNode  jointParameters2 NULL   # {JointParameters2, PROTO}


JointParameters2 doesn't exist -> JointParameters

stefaniapedrazzi · 2018-02-13T15:40:10Z

reference/joint.md

@@ -2,8 +2,8 @@

 ```
 Joint {
-  SFNode jointParameters NULL # a joint parameters node
-  SFNode endPoint        NULL # Solid or SolidReference
+  SFNode jointParameters NULL   # {JointParameters, PROTO}


To be consistent JointParameters (or derived) -> depending on the type of joint JointParameters or HingeJointParameters is allowed

stefaniapedrazzi · 2018-02-13T15:40:22Z

reference/joint.md

-  SFNode jointParameters NULL # a joint parameters node
-  SFNode endPoint        NULL # Solid or SolidReference
+  SFNode jointParameters NULL   # {JointParameters, PROTO}
+  SFNode endPoint        NULL   # {Solid, Slot, PROTO}


To be consistent Solid (or derived) and missing SolidReference node.

stefaniapedrazzi · 2018-02-13T15:40:41Z

reference/propeller.md

+  SFVec3f centerOfThrust  0 0 0   # any vector
+  SFVec2f thrustConstants 1 0     # any vector
+  SFVec2f torqueConstants 1 0     # any vector
+  SFNode device           NULL    # {Solid (or derived), PROTO}


Wrong: only RotationalMotor is allowed -> {RotationalMotor, PROTO}

stefaniapedrazzi · 2018-02-13T15:41:04Z

reference/sliderjoint.md

@@ -4,8 +4,8 @@ Derived from [Joint](joint.md).

 ```
 SliderJoint {
-  MFNode  device   [ ] # linear motor or linear position sensor
-  SFFloat position 0   # initial position (m)
+  MFNode  device   [ ]   # {Linear Motor, Linear PositionSensor, PROTO}


To be consistent with the other descriptions (see Track/HingeJoint node for example):

"Linear Motor" -> "LinearMotor"

"Linear PositionSensor" -> "PositionSensor"

Missing Brake

stefaniapedrazzi · 2018-02-13T15:41:30Z

reference/track.md

-  SFInt32 geometriesCount  10
+  MFNode  device           [ ]    # {LinearMotor, PositionSensor, Brake, PROTO}
+  SFVec2f textureAnimation 0 0    # any vector
+  SFNode  animatedGeometry NULL   # {Shape, Group, PROTO}


Missing Slot and Transform

stefaniapedrazzi · 2018-02-13T15:41:39Z

reference/slot.md

-  SFString type     ""
-  SFNode   endPoint NULL
+  SFString type     ""     # any string
+  SFNode   endPoint NULL   # {Slot, Group (or derived), PROTO}


Missing PointLight, SpotLight, Shape

I'm tempted to put # {node, PROTO} here to be consistent with group.md.

tpjnorton · 2018-02-13T16:47:55Z

👍 (at this point I feel like the boy who cried "wolf")

tpjnorton added 4 commits February 7, 2018 16:04

boolean field descriptors

663956f

indentation correction

1a317d8

strings and nodes

7faafe4

sort out all node blocks

8cdd8b5

tpjnorton self-assigned this Feb 7, 2018

tpjnorton added the enhancement label Feb 7, 2018

tpjnorton added this to the R2018a.rev1 milestone Feb 7, 2018

tpjnorton added 3 commits February 7, 2018 18:15

cheat with WorldInfo

3772f2f

lookuptable entry

ff32ef0

fix variable width issue

e38fb6b

omichel reviewed Feb 12, 2018

View reviewed changes

tpjnorton added 4 commits February 12, 2018 11:24

use border-box to fix more width issues

b5fff3a

override bootstrap's nonsense

e375529

second clearer pass over node blocks

0683c9d

last fix for zoom

5e747a9

omichel suggested changes Feb 13, 2018

View reviewed changes

tpjnorton added 2 commits February 13, 2018 10:35

fix review comments

6d7ffaf

further fixing pass

fb45ca2

omichel suggested changes Feb 13, 2018

View reviewed changes

tpjnorton added 3 commits February 13, 2018 12:14

fix touchsensor

0b2fd99

fix review comments and vector descriptions

3349454

sfrotation fixes

36c9e2b

omichel suggested changes Feb 13, 2018

View reviewed changes

last (hopefully) review comments

5fe1685

omichel suggested changes Feb 13, 2018

View reviewed changes

i just can't get this pr right

881f409

remove bogus indentation

a94bbca

stefaniapedrazzi reviewed Feb 13, 2018

View reviewed changes

tpjnorton added 2 commits February 13, 2018 17:07

fix stefania's review comments

70fd2d8

Merge branch 'master' into cleanup-node-blocks

3924519

tpjnorton and others added 2 commits February 14, 2018 16:38

Merge branch 'master' into cleanup-node-blocks

ffaa9b0

Merge branch 'master' into cleanup-node-blocks

b4dddcb

omichel approved these changes Feb 16, 2018

View reviewed changes

Merge branch 'master' into cleanup-node-blocks

60c88da

tpjnorton merged commit a5bc79e into master Feb 16, 2018

tpjnorton deleted the cleanup-node-blocks branch February 16, 2018 08:25

DavidMansolino mentioned this pull request Mar 20, 2018

Units and range in documentation #180

Open

Improve Node Blocks #600

Improve Node Blocks #600

Conversation

tpjnorton commented Feb 7, 2018

omichel commented Feb 12, 2018

stefaniapedrazzi commented Feb 12, 2018 • edited Loading

tpjnorton commented Feb 12, 2018

stefaniapedrazzi commented Feb 12, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tpjnorton commented Feb 12, 2018

tpjnorton commented Feb 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tpjnorton commented Feb 13, 2018

tpjnorton commented Feb 13, 2018

omichel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tpjnorton commented Feb 13, 2018

tpjnorton commented Feb 13, 2018

omichel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tpjnorton commented Feb 13, 2018

omichel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tpjnorton commented Feb 13, 2018

tpjnorton commented Feb 13, 2018

stefaniapedrazzi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tpjnorton commented Feb 13, 2018

stefaniapedrazzi commented Feb 12, 2018 •

edited

Loading

stefaniapedrazzi commented Feb 12, 2018 •

edited

Loading