phetsims / quadrilateral

"Quadrilateral" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
1 stars 3 forks source link

Rearrange tangible portion of QuadrilateralPreferencesModel #353

Closed jessegreenberg closed 1 year ago

jessegreenberg commented 1 year ago

QuadrilateralPreferencesModel Properties that involve the tangible should be moved into their own class.

Then, that class with tangible control Properties can be used by TangibleConnectionModel. Then, QuadrilateralBluetoothConnectionButton and QuadrilateralSerialConnectionButton can be decoupled from the QuadrilateralModel and only take the model components they require.

jessegreenberg commented 1 year ago

Here is a patch with progress toward this and #351

```patch Subject: [PATCH] Combine fireOnKeyDown and fireOnKeyUp options, see https://github.com/phetsims/scenery/issues/1520 --- Index: js/quadrilateral/view/QuadrilateralSerialConnectionButton.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/quadrilateral/view/QuadrilateralSerialConnectionButton.ts b/js/quadrilateral/view/QuadrilateralSerialConnectionButton.ts --- a/js/quadrilateral/view/QuadrilateralSerialConnectionButton.ts (revision 5469a65dd7a12f5e76934ac01f7e992697624c49) +++ b/js/quadrilateral/view/QuadrilateralSerialConnectionButton.ts (date 1675468741948) @@ -4,7 +4,7 @@ import QuadrilateralColors from '../../common/QuadrilateralColors.js'; import QuadrilateralConstants from '../../common/QuadrilateralConstants.js'; import quadrilateral from '../../quadrilateral.js'; -import QuadrilateralModel from '../model/QuadrilateralModel.js'; +import TangibleConnectionModel from '../model/prototype/TangibleConnectionModel.js'; import QuadrilateralSerialMessageController from './QuadrilateralSerialMessageController.js'; /** @@ -16,9 +16,9 @@ */ class QuadrilateralSerialConnectionButton extends TextPushButton { - public constructor( model: QuadrilateralModel ) { + public constructor( tangibleConnectionModel: TangibleConnectionModel ) { - const controller = new QuadrilateralSerialMessageController( model.quadrilateralShapeModel ); + const controller = new QuadrilateralSerialMessageController( tangibleConnectionModel ); super( 'Send Values', { textNodeOptions: QuadrilateralConstants.SCREEN_TEXT_OPTIONS, Index: js/quadrilateral/view/CalibrationContentNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/quadrilateral/view/CalibrationContentNode.ts b/js/quadrilateral/view/CalibrationContentNode.ts --- a/js/quadrilateral/view/CalibrationContentNode.ts (revision 5469a65dd7a12f5e76934ac01f7e992697624c49) +++ b/js/quadrilateral/view/CalibrationContentNode.ts (date 1675466826070) @@ -13,13 +13,13 @@ import Bounds2 from '../../../../dot/js/Bounds2.js'; import Utils from '../../../../dot/js/Utils.js'; import quadrilateral from '../../quadrilateral.js'; -import QuadrilateralModel from '../model/QuadrilateralModel.js'; import { Circle, Line, Rectangle, Text, VBox, VBoxOptions } from '../../../../scenery/js/imports.js'; import optionize, { EmptySelfOptions } from '../../../../phet-core/js/optionize.js'; import QuadrilateralConstants from '../../common/QuadrilateralConstants.js'; +import TangibleConnectionModel from '../model/prototype/TangibleConnectionModel.js'; class CalibrationContentNode extends VBox { - public constructor( model: QuadrilateralModel, providedOptions?: VBoxOptions ) { + public constructor( tangibleConnectionModel: TangibleConnectionModel, providedOptions?: VBoxOptions ) { const options = optionize()( { align: 'center' @@ -71,7 +71,7 @@ ]; super( options ); - model.tangibleConnectionModel.physicalModelBoundsProperty.link( physicalModelBounds => { + tangibleConnectionModel.physicalModelBoundsProperty.link( physicalModelBounds => { if ( physicalModelBounds !== null ) { leftSideLengthText.text = Utils.toFixed( physicalModelBounds.height, 2 ); } Index: js/quadrilateral/model/prototype/TangibleConnectionModel.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/quadrilateral/model/prototype/TangibleConnectionModel.ts b/js/quadrilateral/model/prototype/TangibleConnectionModel.ts --- a/js/quadrilateral/model/prototype/TangibleConnectionModel.ts (revision 5469a65dd7a12f5e76934ac01f7e992697624c49) +++ b/js/quadrilateral/model/prototype/TangibleConnectionModel.ts (date 1675468627179) @@ -28,6 +28,7 @@ import Tandem from '../../../../../tandem/js/Tandem.js'; import NullableIO from '../../../../../tandem/js/types/NullableIO.js'; import quadrilateral from '../../../quadrilateral.js'; +import QuadrilateralShapeModel from '../QuadrilateralShapeModel.js'; import MarkerDetectionModel from './MarkerDetectionModel.js'; class TangibleConnectionModel { @@ -55,24 +56,23 @@ // model space. private readonly modelBounds: Bounds2; - public constructor( modelBounds: Bounds2, tandem: Tandem ) { + public readonly shapeModel: QuadrilateralShapeModel; + public constructor( shapeModel: QuadrilateralShapeModel, modelBounds: Bounds2, tandem: Tandem ) { this.connectedToDeviceProperty = new BooleanProperty( false, { tandem: tandem.createTandem( 'connectedToDeviceProperty' ) } ); - this.physicalModelBoundsProperty = new Property( null, { tandem: tandem.createTandem( 'physicalModelBoundsProperty' ), phetioValueType: NullableIO( Bounds2.Bounds2IO ) } ); - this.isCalibratingProperty = new BooleanProperty( false, { tandem: tandem.createTandem( 'isCalibratingProperty' ) } ); this.markerDetectionModel = new MarkerDetectionModel( tandem.createTandem( 'markerDetectionModel' ) ); - this.modelBounds = modelBounds; + this.shapeModel = shapeModel; } /** @@ -92,6 +92,34 @@ this.physicalModelBoundsProperty.value = new Bounds2( 0, 0, maxLength, maxLength ); } + /** + * Sets the quadrilateral shape Vertex positions to good initial values after calibration. + * + * During calibration, we request the largest shape that can possibly be made from the device. So when + * calibration is finished, the tangible is as large as it can be and Vertex positions are positioned + * based on full width of the device. + */ + public finishCalibration(): void { + const physicalModelBounds = this.physicalModelBoundsProperty.value!; + assert && assert( physicalModelBounds && physicalModelBounds.isValid(), + 'Physical dimensions of device need to be set during calibration' ); + + this.setPositionsFromLengthAndAngleData( + physicalModelBounds.width, + physicalModelBounds.width, + physicalModelBounds.width, + physicalModelBounds.width, + Math.PI / 2, + Math.PI / 2, + Math.PI / 2, + Math.PI / 2 + ); + } + + public setPositionsFromLengthAndAngleData( topLength: number, rightLength: number, bottomLength: number, leftLength: number, p1Angle: number, p2Angle: number, p3Angle: number, p4Angle: number ): void { + this.shapeModel.setPositionsFromLengthAndAngleData( topLength, rightLength, bottomLength, leftLength, p1Angle, p2Angle, p3Angle, p4Angle ); + } + /** * Create a transform that can be used to transform between tangible and virtual space. The scaling only uses one * dimension because we assume scaling should be the same in both x and y. It uses height as the limiting factor for Index: js/quadrilateral/view/QuadrilateralScreenView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/quadrilateral/view/QuadrilateralScreenView.ts b/js/quadrilateral/view/QuadrilateralScreenView.ts --- a/js/quadrilateral/view/QuadrilateralScreenView.ts (revision 5469a65dd7a12f5e76934ac01f7e992697624c49) +++ b/js/quadrilateral/view/QuadrilateralScreenView.ts (date 1675467860804) @@ -188,56 +188,6 @@ const connectionComponents = []; - // Add a Dialog that will calibrate the device to the simulation (mapping physical data to modelled data). - const calibrationDialog = new Dialog( new CalibrationContentNode( model ), { - title: new Text( 'External Device Calibration', QuadrilateralConstants.PANEL_TITLE_TEXT_OPTIONS ) - } ); - - calibrationDialog.isShowingProperty.link( ( isShowing, wasShowing ) => { - tangibleConnectionModel.isCalibratingProperty.value = isShowing; - - if ( !isShowing && wasShowing !== null ) { - const physicalModelBounds = tangibleConnectionModel.physicalModelBoundsProperty.value; - - // it is possible that the Dialog was closed without getting good values for the bounds, only set - // positions if all is well - if ( physicalModelBounds && physicalModelBounds.isValid() ) { - shapeModel.setPositionsFromLengthAndAngleData( - physicalModelBounds.width, - physicalModelBounds.width, - physicalModelBounds.width, - physicalModelBounds.width, - Math.PI / 2, - Math.PI / 2, - Math.PI / 2, - Math.PI / 2 - ); - } - } - } ); - - // this is the "sim", add a button to start calibration - const calibrationButton = new TextPushButton( 'Calibrate Device', { - listener: () => { - calibrationDialog.show(); - }, - - textNodeOptions: QuadrilateralConstants.SCREEN_TEXT_OPTIONS, - baseColor: QuadrilateralColors.screenViewButtonColorProperty - } ); - connectionComponents.push( calibrationButton ); - - if ( QuadrilateralQueryParameters.bluetooth ) { - const bluetoothButton = new QuadrilateralBluetoothConnectionButton( model, tandem.createTandem( 'quadrilateralBluetoothConnectionButton' ) ); - connectionComponents.push( bluetoothButton ); - } - - if ( QuadrilateralQueryParameters.serial ) { - const sendValuesButton = new QuadrilateralSerialConnectionButton( model ); - sendValuesButton.leftBottom = gridNode.leftTop; - connectionComponents.push( sendValuesButton ); - } - deviceConnectionParentNode.children = connectionComponents; deviceConnectionParentNode.top = gridNode.top; deviceConnectionParentNode.left = resetAllButton.left; Index: js/quadrilateral/view/QuadrilateralBluetoothConnectionButton.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/quadrilateral/view/QuadrilateralBluetoothConnectionButton.ts b/js/quadrilateral/view/QuadrilateralBluetoothConnectionButton.ts --- a/js/quadrilateral/view/QuadrilateralBluetoothConnectionButton.ts (revision 5469a65dd7a12f5e76934ac01f7e992697624c49) +++ b/js/quadrilateral/view/QuadrilateralBluetoothConnectionButton.ts (date 1675468857646) @@ -47,7 +47,7 @@ private topLeftAngle = 0; private topRightAngle = 0; - public constructor( quadrilateralModel: QuadrilateralModel, tandem: Tandem ) { + public constructor( tangibleConnectionModel: TangibleConnectionModel, tandem: Tandem ) { // TODO: Handle when device does not support bluetooth with bluetooth.getAvailability. // TODO: Handle when browser does not support bluetooth, presumablue !navigator.bluetooth @@ -56,7 +56,6 @@ baseColor: QuadrilateralColors.screenViewButtonColorProperty } ); - this.model = quadrilateralModel; this.tangibleConnectionModel = this.model.tangibleConnectionModel; this.addListener( this.requestQuadDevice.bind( this ) ); Index: js/quadrilateral/view/QuadrilateralSerialMessageController.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/quadrilateral/view/QuadrilateralSerialMessageController.ts b/js/quadrilateral/view/QuadrilateralSerialMessageController.ts --- a/js/quadrilateral/view/QuadrilateralSerialMessageController.ts (revision 5469a65dd7a12f5e76934ac01f7e992697624c49) +++ b/js/quadrilateral/view/QuadrilateralSerialMessageController.ts (date 1675468650425) @@ -5,17 +5,20 @@ * "p5 serial connection" prototype. * * TODO: More documentation about this Prototype, how it works, and what is expected. + * + * TODO: Rename to QuadrilateralSerialMessageSender (not really a controller) */ import Utils from '../../../../dot/js/Utils.js'; import quadrilateral from '../../quadrilateral.js'; import QuadrilateralShapeModel from '../model/QuadrilateralShapeModel.js'; +import TangibleConnectionModel from '../model/prototype/TangibleConnectionModel.js'; class QuadrilateralSerialMessageController { private readonly shapeModel: QuadrilateralShapeModel; - public constructor( quadrilateralShapeModel: QuadrilateralShapeModel ) { - this.shapeModel = quadrilateralShapeModel; + public constructor( tangibleConnectionModel: TangibleConnectionModel ) { + this.shapeModel = tangibleConnectionModel.shapeModel; } /** ```
jessegreenberg commented 1 year ago

There is also a Property reducedStepSizeProperty that should never change after startup now and can be deleted.

jessegreenberg commented 1 year ago

This is done and looking much nicer, closing.