Allow listeners to be attached to just the body + nozzle parts

[samreid]

In order to support the design for Bending Light (for phetsims/bending-light#339), we would like to attach a listener to the body + nozzle rather than the entire LaserPointerNode. I think this means we would like to make a new child that combines the bodyNode + nozzleNode accessible.

[samreid]

Implemented in the commit, tested in phetsims/bending-light#339 and a number of other sims that use LaserPointerNode. Ready for review. One question for the reviewer: instead of making bodyAndNozzleNode @public, should we add addBodyAndNozzleNodeInputListener methods? Assigning to the author of LaserPointerNode for review.

[pixelzoom]

I haven't reviewed this yet, because a regression seems to have slipped into LaserPointerNode. The beam location is now forward of the optional lens (hasGlass: true), which is not correct. See screenshot below. Back to @samreid before I review.

In scenery-phet demo:

[samreid]

I shifted the source of the light so it will go through the lens. I didn't track down the exact origination of this problem, but it appears it has been around for a while.

[pixelzoom]

I'm trying to figure out why this changed is needed.

Looking at phetsims/bending-light#339, you're using this feature in LaserNode, which uses LaserPointerNode via composition. You're conditionally adding a decoration to (knobImage), and then using bodyAndNozzleNode for touch area and input listener target Node:

    const knobImage = new Image( knobImageData, { scale: 0.58, rightCenter: laserPointerNode.leftCenter } );
    knobImage.touchArea = knobImage.localBounds.dilatedXY( 15, 27 ).shiftedX( -15 );
    hasKnob && laserPointerNode.addChild( knobImage );
    laserPointerNode.translate( laserPointerNode.width, laserPointerNode.height / 2 );
    if ( !hasKnob ) {
      laserPointerNode.bodyAndNozzleNode.touchArea = laserPointerNode.bodyAndNozzleNode.bounds.dilated( 8 ).shiftedX( -8 );
    }

    const translationTarget = hasKnob ? laserPointerNode.bodyAndNozzleNode : knobImage;
    const rotationTarget = hasKnob ? knobImage : laserPointerNode.bodyAndNozzleNode;

It's not clear to me - why can't you just use laserPointerNode?

[pixelzoom]

Discussed with @samreid on Slack. There is no need for bodyAndNozzleNode if:

(1) LaserNode changes laserPointerNode.addChild( knobImage ) to this.addChild( knobImage), and
(2) it's OK if the laser can be dragged by its button.

There is precedent for being able to drag by the button - see dropper in pH Scale. So if that's not OK, I'd like to hear why it's desirble in pH Scale, but not in Bending Light.

[samreid]

I reverted the change to LaserPointerNode that made body+nozzle public, and I updated bending light's LaserNode accordingly. @pixelzoom and I discussed adding the knobImage to this instead of to the laserPointerNode but that did not appear to be necessary, the input listeners seem to be doing the right thing with the knob a child of the laserPointerNode.

Also, for unknown reasons, the laser still cannot be dragged by the button.

@pixelzoom want to take a look?

[pixelzoom]

Reverts look good.

Also, for unknown reasons, the laser still cannot be dragged by the button.

The input listener on the button would need to be attach: false. Here's how that was done for EyeDropperNode:

    const button = new RoundMomentaryButton( false, true, this.dispensingProperty, {
      baseColor: 'red',
      radius: 18,
      listenerOptions: {
        // We want to be able to drag the dropper WHILE dispensing, see https://github.com/phetsims/ph-scale/issues/86
        attach: false
      },
      tandem: options.tandem.createTandem( 'button' )
    } );

So something similar would need to be added to const buttonOptions in LaserPointerNode.

And if you don't need to be able to drag by the button, then you're done!

[samreid]

@arouinfar should the laser pointer in Bending Light be able to be dragged by the button?

[arouinfar]

@samreid I don't think the laser needs to be draggable by the button. It may also end up being accidentally dragged by users on touch devices.

For PhET-iO we are going to want to be able to make the body of the lasers pickable:false while keeping the button interactive. The same issue came up in pH Scale, see phetsims/scenery#1041 and phetsims/scenery#1116. I don't know if that has any impact on the current issue, but I want to make you aware of this design requirement in case it does.

[samreid]

For PhET-iO we are going to want to be able to make the body of the lasers pickable:false while keeping the button interactive.

@pixelzoom will our proposed strategy work with that design constraint?

[pixelzoom]

No, it will not. And it does not currently work that way in ph-scale either, see phetsims/ph-scale#141.

And as I argued in phetsims/scenery#1041... I think that setting pickable is (in general) the wrong way to address this. We should be disabling specific input listeners, so that we can easily support exactly this type of requirements.

[samreid]

In phetsims/scenery#1041 (comment) it was argued:

Add enabledProperty to listeners. This is not a good solution because you may have to do one for the drag listener, and another for the keyboard listener. This in general would be clunky for the PhET-iO API client (studio user).

Is there a way to pair the pointer input with the keyboard input so they can be disabled together?

[pixelzoom]

@samreid It feels like we're now veering off into a discussion that belongs in phetsims/scenery#1041 (comment). I don't have time for that right now, and I think it needs to involve more parties.

I recommend closing this issue (since the non-PhET-iO requirments are met for bending-light), and opening a new bending-light issue for the PhET-iO requirement that @arouinfar mentioned (similar to phetsims/ph-scale#141);