phetsims / normal-modes

"Normal Modes" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
0 stars 2 forks source link

Prototype workaround for lack of multitouch support. #79

Closed pixelzoom closed 3 years ago

pixelzoom commented 3 years ago

For https://github.com/phetsims/qa/issues/733. Discovered while working on #78.

This sim will have serious problems with multitouch. It assumes that there is only 1 mass being dragged at a time, and does nothing to prevent that.

In OneDimensionalModel.js:

    // @public {Property.<number>} the index of the mass being dragged
    this.draggingMassIndexProperty = new NumberProperty( 0, {
      tandem: options.tandem.createTandem( 'draggingMassIndexProperty' )
    } );

In TwoDimensionalModel.js:

    // @public {Property.<number[]|null>} the indexes of the mass being dragged (an object with and 'i' and a 'j')
    this.draggingMassIndexesProperty = new Property( null, {
      tandem: options.tandem.createTandem( 'draggingMassIndexesProperty' )
    } );

This is an unreasonable constraint. The sim should support dragging multiple masses simultaneously. But if you try to drag multiple masses (e.g. on an iPad) you'll see that the sim behaves very oddly. It doesn't crash the sim, but it's certainly not usable.

It's not possible to properly address this for the prototype. A band aid would be to allow only 1 mass to be dragged at a time. If you start dragging a 2nd mass, dragging will be canceled for the 1st mass.

@arouinfar Does this need to be addressed for the prototype? We are now in RC testing.

@KatieWoe I'm wondering how this slipped through dev testing. Are multitouch devices not included in dev-lite testing?

KatieWoe commented 3 years ago

Dev-Lite tests two devices at random for around 2 hours. In the case of the las dev lite it was a Mac and a Windows, and so touch was not included. A dev lite is a fast test when we don't expect much to go wrong.

KatieWoe commented 3 years ago

If touch/specific devices/in depth testing is needed then a regular dev test is more appropriate.

pixelzoom commented 3 years ago

This is an unreasonable constraint. The sim should support dragging multiple masses simultaneously. If that's not possible at this late stage, then starting a drag on a mass should interrupt (cancel) any other mass drag that is in progress.

Definitely not possible to properly support multitouch for the prototype. And I don't see any easy way to properly interrupt dragging, but I could probably put together a hack.

pixelzoom commented 3 years ago

In the above commits, I implemented a workaround (in master and 1.0) that only allows 1 mass to be dragged at a time. If you start dragging a second mass, the drag for the first one is interrupted.

@arouinfar please review in master, on a touch device, and verify that it's acceptable for the prototype. Then assign back to me.

KatieWoe commented 3 years ago

For documentation, this is what the badly behaving look like. It did seem like normal behavior returned when the masses were released. @pixelzoom if this isn't the behavior you saw let me know and I'll try to reproduce again.

https://user-images.githubusercontent.com/41024075/140827796-6bde6a8f-72e2-46e4-9f47-28455e75b91f.MP4

pixelzoom commented 3 years ago

@KatieWoe Yes, that's the behavor that I was seeing. And thanks for documenting via a movie.

arouinfar commented 3 years ago

@pixelzoom the workaround is good enough for the prototype and the behavior is reasonable on my phone, so I think we can close this one. Thanks!