Skip to content

Commit

Permalink
make disposal support (or lack thereof) explicit, see #336
Browse files Browse the repository at this point in the history
  • Loading branch information
jbphet committed Jul 14, 2023
1 parent 75b6e50 commit 8f24be2
Show file tree
Hide file tree
Showing 11 changed files with 43 additions and 23 deletions.
1 change: 1 addition & 0 deletions js/common/view/GreenhouseEffectCheckbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class GreenhouseEffectCheckbox extends Checkbox {
const options = optionize<GreenhouseEffectCheckboxOptions, SelfOptions, CheckboxOptions>()( {
iconNode: null,
maxLabelTextWidth: 180, // empirically determined, works well for most cases in Greenhouse
isDisposable: false,

// i18n
maxWidth: 250,
Expand Down
3 changes: 1 addition & 2 deletions js/common/view/ObservationWindowPDOMNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ class ObservationWindowPDOMNode extends Node {

protected constructor( model: LayersModel ) {
super( {

// pdom
isDisposable: false,
tagName: 'ul'
} );

Expand Down
3 changes: 2 additions & 1 deletion js/photons/PhotonsScreen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ class PhotonsScreen extends Screen<PhotonsModel, PhotonsScreenView> {
tandem: tandem,
name: GreenhouseEffectStrings.screen.photonsStringProperty,
descriptionContent: GreenhouseEffectStrings.a11y.photons.homeScreenDescriptionStringProperty,
createKeyboardHelpNode: () => new GreenhouseEffectKeyboardHelpContent( { includeFluxMeterHelp: true } )
createKeyboardHelpNode: () => new GreenhouseEffectKeyboardHelpContent( { includeFluxMeterHelp: true } ),
isDisposable: false
};

super(
Expand Down
3 changes: 2 additions & 1 deletion js/waves/WavesScreen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ class WavesScreen extends Screen<WavesModel, WavesScreenView> {
tandem: tandem,
name: GreenhouseEffectStrings.screen.wavesStringProperty,
descriptionContent: GreenhouseEffectStrings.a11y.waves.homeScreenDescriptionStringProperty,
createKeyboardHelpNode: () => new GreenhouseEffectKeyboardHelpContent()
createKeyboardHelpNode: () => new GreenhouseEffectKeyboardHelpContent(),
isDisposable: false
};

super(
Expand Down
6 changes: 6 additions & 0 deletions js/waves/model/EMWaveSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import greenhouseEffect from '../../greenhouseEffect.js';
import Wave, { WaveCreatorArguments } from './Wave.js';
import WaveSourceSpec from './WaveSourceSpec.js';
import WithRequired from '../../../../phet-core/js/types/WithRequired.js';
import Disposable from '../../../../axon/js/Disposable.js';

type SelfOptions = {

Expand Down Expand Up @@ -129,6 +130,11 @@ class EMWaveSource {
}
);
}

// These generally exist for the life of the sim, so disposal has not been implemented.
public dispose(): void {
Disposable.assertNotDisposable();
}
}

greenhouseEffect.register( 'EMWaveSource', EMWaveSource );
Expand Down
7 changes: 7 additions & 0 deletions js/waves/model/WaveAttenuator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import IOType from '../../../../tandem/js/types/IOType.js';
import NumberIO from '../../../../tandem/js/types/NumberIO.js';
import greenhouseEffect from '../../greenhouseEffect.js';
import Disposable from '../../../../axon/js/Disposable.js';

/**
* WaveAttenuator is a simple class that is used to keep track of points along a wave where attenuation (reduction in
Expand Down Expand Up @@ -43,6 +44,12 @@ class WaveAttenuator {
stateObject.distanceFromStart
)
} );

// Instances of this class are intended to be lightweight and own no Property instances, so disposal is unneeded and
// not supported.
public dispose(): void {
Disposable.assertNotDisposable();
}
}

export type WaveAttenuatorStateObject = {
Expand Down
7 changes: 7 additions & 0 deletions js/waves/model/WaveIntensityChange.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import NullableIO from '../../../../tandem/js/types/NullableIO.js';
import NumberIO from '../../../../tandem/js/types/NumberIO.js';
import ReferenceIO, { ReferenceIOState } from '../../../../tandem/js/types/ReferenceIO.js';
import greenhouseEffect from '../../greenhouseEffect.js';
import Disposable from '../../../../axon/js/Disposable.js';

/**
* WaveIntensityChange is a type that represents a change in a propagating wave's intensity at a point along a wave that
Expand Down Expand Up @@ -66,6 +67,12 @@ class WaveIntensityChange {
NullableIO( ReferenceIO( IOType.ObjectIO ) ).fromStateObject( stateObject.anchoredTo )
)
} );

// Instances of this class are intended to be lightweight and do not link to any Property instances, so disposal is
// unneeded and not supported.
public dispose(): void {
Disposable.assertNotDisposable();
}
}

// for phet-io
Expand Down
7 changes: 7 additions & 0 deletions js/waves/model/WaveSourceSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import Vector2 from '../../../../dot/js/Vector2.js';
import greenhouseEffect from '../../greenhouseEffect.js';
import Disposable from '../../../../axon/js/Disposable.js';

/**
* A simple class that specifies the X value for where waves will be produced and a direction of travel.
Expand All @@ -23,6 +24,12 @@ class WaveSourceSpec {
this.xPosition = xPosition;
this.propagationDirection = propagationDirection;
}

// Instances of this class are intended to be lightweight and do not link to any Property instances, so disposal is
// unneeded and not supported.
public dispose(): void {
Disposable.assertNotDisposable();
}
}

greenhouseEffect.register( 'WaveSourceSpec', WaveSourceSpec );
Expand Down
24 changes: 8 additions & 16 deletions js/waves/view/WavesCanvasNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ class WavesCanvasNode extends CanvasNode {
public constructor( model: WavesModel, modelViewTransform: ModelViewTransform2, providedOptions: WavesCanvasNodeOptions ) {

const options = optionize<WavesCanvasNodeOptions, SelfOptions, CanvasNodeOptions>()( {

// CanvasNodeOptions
isDisposable: false,
visiblePropertyOptions: { phetioFeatured: true }
}, providedOptions );

Expand Down Expand Up @@ -118,15 +117,13 @@ class WavesCanvasNode extends CanvasNode {
let nextIntensityChangePosition = this.getIntensityChangeXPosition(
nextIntensityChangeIndex,
wave,
amplitude,
wavelength
amplitude
);

// Get the amount of compensation needed in the x direction so that the wave will appear to originate from a
// horizontal region.
const compensatedStartingXValue = WavesCanvasNode.getXCompensationForTilt(
amplitude,
wavelength,
phaseOffsetAtStart,
wave.propagationDirection.getAngle()
);
Expand Down Expand Up @@ -168,8 +165,7 @@ class WavesCanvasNode extends CanvasNode {
nextIntensityChangePosition = this.getIntensityChangeXPosition(
nextIntensityChangeIndex,
wave,
amplitude,
wavelength
amplitude
);
}
}
Expand All @@ -184,9 +180,8 @@ class WavesCanvasNode extends CanvasNode {
* @param index - index of the intensity change of interest
* @param wave - wave on which the intensity change may exist
* @param amplitudeInView
* @param wavelengthInView
*/
private getIntensityChangeXPosition( index: number, wave: Wave, amplitudeInView: number, wavelengthInView: number ): number {
private getIntensityChangeXPosition( index: number, wave: Wave, amplitudeInView: number ): number {
const intensityChange = wave.intensityChanges[ index ];
const intensityChangeDistanceFromStart = intensityChange ?
intensityChange.distanceFromStart :
Expand All @@ -198,7 +193,6 @@ class WavesCanvasNode extends CanvasNode {
);
xPosition += WavesCanvasNode.getXCompensationForTilt(
amplitudeInView,
wavelengthInView,
phaseAtNominalXPosition,
wave.propagationDirection.getAngle()
);
Expand All @@ -212,20 +206,18 @@ class WavesCanvasNode extends CanvasNode {
* computational clipping. For more information on why this is necessary and what it does, please see
* https://github.com/phetsims/greenhouse-effect/issues/66.
*
* This is not an exact solution, it's an approximation that works resonably well. I (jbphet) spent a couple of hours
* trying to come up with an analytical, closed form solution, but didn't get there, and some poking around on line
* led me to believe that it's not an easy problem, so I came up with this, which seems to work well enough for the
* needs of this sim.
* This is not an exact solution, it's an approximation that works reasonably well. I (jbphet) spent a couple of
* hours trying to come up with an analytical, closed form solution, but didn't get there, and some poking around
* online led me to believe that it's not an easy problem, so I came up with this, which seems to work well enough for
* the needs of this sim.
*
* Note that this algorithm assumes the waves are generated by a sine function, not a cosine.
*
* @param amplitudeInView
* @param wavelengthInView
* @param phase - in radians
* @param propagationAngle - in radians, 0 is straight to the right
*/
private static getXCompensationForTilt( amplitudeInView: number,
wavelengthInView: number,
phase: number,
propagationAngle: number ): number {

Expand Down
2 changes: 1 addition & 1 deletion js/waves/view/WavesScreenSummaryContentNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const qualitativeAndQuantitativeTemperatureDescriptionPatternStringProperty = Gr
class WavesScreenSummaryContentNode extends Node {

public constructor( model: WavesModel ) {
super();
super( { isDisposable: false } );

const playAreaDescriptionNode = new Node( {
tagName: 'p',
Expand Down
3 changes: 1 addition & 2 deletions js/waves/view/WavesScreenView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ class WavesScreenView extends GreenhouseEffectScreenView {

// Create the observation window that will depict the ground, sky, light waves, etc.
const observationWindow = new WaveLandscapeObservationWindow( model, {

// phet-io
isDisposable: false,
tandem: tandem.createTandem( 'observationWindow' )
} );

Expand Down

0 comments on commit 8f24be2

Please sign in to comment.