phetsims / sugar-and-salt-solutions Goto Github PK

"Sugar And Salt Solutions" is an educational simulation in HTML5, by PhET Interactive Simulations.

License: GNU General Public License v3.0

JavaScript 99.61% HTML 0.39%

sugar-and-salt-solutions's Introduction

Sugar And Salt Solutions

"Sugar And Salt Solutions" is an educational simulation in HTML5, by PhET Interactive Simulations at the University of Colorado Boulder.

This simulation is under development and has not been published.

Documentation

The PhET Development Overview is the most complete guide to PhET Simulation Development. This guide includes how to obtain simulation code and its dependencies, notes about architecture & design, how to test and build the sims, as well as other important information.

License

See the LICENSE

sugar-and-salt-solutions's People

Contributors

Stargazers

Watchers

sugar-and-salt-solutions's Issues

beaker tick labels aren't shown

In macro tab, the beaker does not have tick labels. It should look similar to the beaker in beer's law.

Port the strings from Java to JS

adjust touchArea options for HSlider thumb

In phetsims/sun#254, the touchArea defaults for HSlider thumb were changed so that they default to pixels, not a percentage of the thumb size. Sims that use HSlider's thumbSize option will therefore need to also specify thumbTouchAreaXDilation and thumbTouchAreaYDilation.

Can we avoid using translatable strings in HTMLText?

See https://github.com/phetsims/special-ops/issues/18. If using them is a requirement, then let me know (I'll need to look into sanitization of translated strings).

Move the images from the Java project to the JS project

Refactor SugarAndSaltSolutionModel to extend PropertySet

Refactor SugarAndSaltSolutionModel to extend PropertySet. Numerous properties are declared in the constructor. Getters, setters and reset() will be simpler through PropertySet subclassing.

branch: slater

This branch was created by @samreid. What is it? Plans for it?

convert from PropertySet to Property

PropertySet is deprecated and should be replaced by Property, see phetsims/axon#102. Since this sim hasn't been published yet, this should be done before RC testing and publishing.

TypeError: Cannot read property 'sizeScale' of undefined

TypeError: Cannot read property 'sizeScale' of undefined
    at Nitrogen.SphericalParticle [as constructor] (http://192.168.0.2:8080/sugar-and-salt-solutions/js/common/model/sphericalparticles/SphericalParticle.js?bust=1447097528259:26:97)
    at new Nitrogen (http://192.168.0.2:8080/sugar-and-salt-solutions/js/common/model/sphericalparticles/Nitrogen.js?bust=1447097528259:30:23)
    at http://192.168.0.2:8080/sugar-and-salt-solutions/js/micro/model/sodiumnitrate/SodiumNitrateConstants.js?bust=1447097528259:17:35
    at Object.context.execCb (http://192.168.0.2:8080/sherpa/lib/require-2.1.11.js:1650:25)
    at Object.Module.check (http://192.168.0.2:8080/sherpa/lib/require-2.1.11.js:866:35)
    at Object. (http://192.168.0.2:8080/sherpa/lib/require-2.1.11.js:1113:20)
    at http://192.168.0.2:8080/sherpa/lib/require-2.1.11.js:132:17
    at http://192.168.0.2:8080/sherpa/lib/require-2.1.11.js:1156:11
    at each (http://192.168.0.2:8080/sherpa/lib/require-2.1.11.js:57:23)
    at Object.Module.emit (http://192.168.0.2:8080/sherpa/lib/require-2.1.11.js:1155:9)

Code should be auto-formatted

When applying the https://github.com/phetsims/joist/blob/master/util/phet-idea-codestyle.xml code style formatting, I see 12 modified files. The files should be formatted using IDEA's autoformatter to standardize formatting, simplify diffs and reviews by other PhET team members.

I normally leave IDEA's "reformat before committing" checkbox checked, though I sometimes have to remember to uncheck that when working with 3rd party code that isn't formatted with our auto-formatter.

Namespaces are missing

e.g., Assertion failed: not namespaced: sugarAndSaltSolutions.DispenserType

sugar-and-salt-solutions fails to build

grunt
…
Writing HTML
Fatal error: Cannot read property 'value' of undefined
%

Assertion failed: x not finite

Uncaught Error: Assertion failed: x not finite
Error: Assertion failed: x not finite
    at window.assertions.assertFunction (http://192.168.0.2:8080/assert/js/assert.js:22:13)
    at SphericalParticleNode.extend.prependTranslation (http://192.168.0.2:8080/scenery/js/nodes/Node.js?bust=1433968108614:1373:25)
    at SphericalParticleNode.extend.translate (http://192.168.0.2:8080/scenery/js/nodes/Node.js?bust=1433968108614:1207:16)
    at SphericalParticleNode.extend.setX (http://192.168.0.2:8080/scenery/js/nodes/Node.js?bust=1433968108614:1270:12)
    at SphericalParticleNode.x (http://192.168.0.2:8080/scenery/js/nodes/Node.js?bust=1433968108614:1273:27)
    at new Bar (http://192.168.0.2:8080/sugar-and-salt-solutions/js/common/view/barchart/Bar.js?bust=1433968108614:77:14)
    at http://192.168.0.2:8080/sugar-and-salt-solutions/js/micro/view/MicroConcentrationBarChart.js?bust=1433968108614:52:55
    at Function.St (http://192.168.0.2:8080/sherpa/lib/lodash-2.4.1.min.js?bust=1433968108614:23:149)
    at MicroConcentrationBarChart.inherit.setBars (http://192.168.0.2:8080/sugar-and-salt-solutions/js/micro/view/MicroConcentrationBarChart.js?bust=1433968108614:50:9)
    at ExpandableConcentrationBarChartNode.inherit.setBars (http://192.168.0.2:8080/sugar-and-salt-solutions/js/micro/view/ExpandableConcentrationBarChartNode.js?bust=1433968108614:59:41)

build failure

Noted in phetsims/chipper#382

Where method overloading was used in Java, create separate names for methods in the JS

For instance, we saw with method today in Particle.js

    /**
     * Convenience method to translate a particle by the specified model delta (in meters)
     * @param {Vector2 or number} delta
     * double {number} dy
     */
    translate: function( delta, dy ) {
      if ( !_.isNaN( dy ) ) {
        this.setPosition( this.position.plusXY( delta, dy ) );
      }
      else {
        this.setPosition( this.position.plus( delta ) );
      }
    },

This came from an overloaded Java method translate that had 2 signatures. It would be simpler/more straightforward/higher performance to create separate methods here, such as translateVector and translateNumber.

@AshrafSharf said there is a similar occurrence in Particle.stepInTime.

Properties should have `Property` suffix

Examples of anti pattern:

SugarAndSaltSharedProperties: sizeScale: new Property( 1.0 )
Dispenser: thisDispenser.enabled = new Property( false );
Dispenser: thisDispenser.angle = new Property( angle );
Dispenser: thisDispenser.center = new Property( new Vector2( x, y ) );
SugarAndSaltSolutionMode: thisModel.dispenserType = new Property( DispenserType.SALT );

Why does the submerged in water node contain the conductivity tester toolbox?

In the macro tab, we come across

self.submergedInWaterNode.addChild( self.conductivityToolboxNode );

It seems an odd choice of parent-child for a node but maybe I am missing something.

Assertion failure on launch

Error: Assertion failed: Matrix was suspicious
    at window.assertions.assertFunction (http://localhost/phet/git/assert/js/assert.js:22:13)
    at Transform3.setMatrix (http://localhost/phet/git/dot/js/Transform3.js?1423091571269:40:32)
    at Transform3.prependTranslation (http://localhost/phet/git/dot/js/Transform3.js?1423091571269:76:12)
    at Text.extend.prependTranslation (http://localhost/phet/git/scenery/js/nodes/Node.js?1423091571269:1246:23)
    at Text.extend.translate (http://localhost/phet/git/scenery/js/nodes/Node.js?1423091571269:1090:16)
    at Text.extend.setX (http://localhost/phet/git/scenery/js/nodes/Node.js?1423091571269:1157:12)
    at Text.x (http://localhost/phet/git/scenery/js/nodes/Node.js?1423091571269:2256:27)
    at SaltShakerNode.inherit.updateTransform (http://localhost/phet/git/sugar-and-salt-solutions/js/common/view/DispenserNode.js?1423091571269:102:24)
    at http://localhost/phet/git/sugar-and-salt-solutions/js/common/view/DispenserNode.js?1423091571269:71:16
    at new Multilink (http://localhost/phet/git/axon/js/Multilink.js?1423091571269:48:16)

The transform matrix trying to be applied:

6.123233995736766e-17 -1 -Infinity
1 6.123233995736766e-17 0
0 0 1

Caused by DispenserNode's imageNode being an empty Node (empty bounds), and fails line 102 of DispenserNode.

Review the 1st screen

@ariel-phet asked if I could do a 1-hour review of the 1st screen and report back on the status. Elevating priority so I can try to get to it before Wednesday.

add to active-runnables when development resumes

Removed this sim from chipper/data/active-runnables until development resumes, so that we don't see issues with automated testing. Add it back in when development resumes.

Formatting around line comments

According to the PhET Style Guidelines https://docs.google.com/document/d/1odXkliRagq0zuf1_NdOtQ2BrkC9hwlISnPi2y-dKdrk/edit# there should be a blank line preceding a section of line comments, and a whitespace between // and the first letter of the comment.

Formatting Title Case

For phetsims/tasks/issues/953
Shuld "Show values" be in title case?

error when using Evaporation slider

I noticed this while testing a refactor in phetsims/scenery-phet#140, it occurs in master on all platforms.

To reproduce:

start sim
go to 'Micro' screen
shake out some sodium (so that molecules are visible)
move the Evaporation slider, the sim will throw an error

Here's the console output:

fix string var names

... that fail ESLint. See phetsims/chipper#396 (comment).

CT: Cannot read property 'set' of undefined

sugar-and-salt-solutions : xss-fuzz : run
Uncaught TypeError: Cannot read property 'set' of undefined
TypeError: Cannot read property 'set' of undefined
    at FormulaUnit.moveTogether (https://bayes.colorado.edu/continuous-testing/snapshot-1540180835952/sugar-and-salt-solutions/js/micro/model/dynamics/FormulaUnit.js?bust=1540202752702:46:24)
    at SodiumChlorideCrystalGrowth.towardNewCrystal (https://bayes.colorado.edu/continuous-testing/snapshot-1540180835952/sugar-and-salt-solutions/js/micro/model/dynamics/CrystalGrowth.js?bust=1540202752702:226:20)
    at SodiumChlorideCrystalGrowth.allowCrystalGrowth (https://bayes.colorado.edu/continuous-testing/snapshot-1540180835952/sugar-and-salt-solutions/js/micro/model/dynamics/CrystalGrowth.js?bust=1540202752702:66:16)
    at MicroModel.updateModel (https://bayes.colorado.edu/continuous-testing/snapshot-1540180835952/sugar-and-salt-solutions/js/micro/model/MicroModel.js?bust=1540202752702:478:40)
    at ConstantDtClock.tick (https://bayes.colorado.edu/continuous-testing/snapshot-1540180835952/sugar-and-salt-solutions/js/common/model/ConstantDtClock.js?bust=1540202752702:70:12)
    at ConstantDtClock.constantStep (https://bayes.colorado.edu/continuous-testing/snapshot-1540180835952/sugar-and-salt-solutions/js/common/model/ConstantDtClock.js?bust=1540202752702:49:12)
    at EventTimer.eventCallback (https://bayes.colorado.edu/continuous-testing/snapshot-1540180835952/sugar-and-salt-solutions/js/common/model/ConstantDtClock.js?bust=1540202752702:34:12)
    at EventTimer.step (https://bayes.colorado.edu/continuous-testing/snapshot-1540180835952/phet-core/js/EventTimer.js?bust=1540202752702:126:14)
    at ConstantDtClock.step (https://bayes.colorado.edu/continuous-testing/snapshot-1540180835952/sugar-and-salt-solutions/js/common/model/ConstantDtClock.js?bust=1540202752702:45:23)
    at MicroModel.step (https://bayes.colorado.edu/continuous-testing/snapshot-1540180835952/sugar-and-salt-solutions/js/common/model/AbstractSugarAndSaltSolutionsModel.js?bust=1540202752702:38:20)
id: Bayes Chrome
Approximately 10/21/2018, 10:00:35 PM

In macro tab, the title overlaps the bar chart

Conductivity sensor appears at the bottom of the screen when initially grabbed.

This appears related to a similar issue in charges and fields : phetsims/charges-and-fields#146

Assigning to @jonathanolson

The reset button on macro tab does not reset the entire sim

sim violates convention for naming fields of type axon.Property

I noticed this while working on a common-code refactor. This sim is not following the PhET convention of naming vars of type axon.Property with a 'Property' suffix. For example, in Dispenser:

    thisDispenser.angle = new Property( angle );
    thisDispenser.center = new Property( new Vector2( x, y ) );

This should be:

    thisDispenser.angleProperty = new Property( angle );
    thisDispenser.centerProperty = new Property( new Vector2( x, y ) );

Searching for 'new Property', I find 24 occurrences, and all but 1 violates this convention.

This is unfortunately not called out specifically in the 'PhET Development Overview', but it is shown in the example related to constructor and function documentation:

@param {Property.} hoursProperty - cumulative hours worked

incorrect options pattern in SphericalParticle

Here's the first part of SphericalParticle's constructor:

/**
   * @param {Object} [options] ( {number} radius, {Vector2} position, {Color} color, {number} charge, {Color} chargeColor) )
   * refer the constructor signature in the table given in the design doc and to-do doc,
   * @constructor
   */
  function SphericalParticle( options ) {
    if ( !options.position ) { //If position is not explicitly given, consider the radius to be in PM, inferred from Java Code
      options.radius = Units.picometersToMeters( options.radius ) * SugarAndSaltSharedProperties.sizeScale.get();
      options.position = new Vector2();
    }

Problems:

The options are not in fact optional. If you don't provide options to this constructor, it will crash. Use this pattern to provide default options:

options = _.extend( {
  // define options and their default values here
}, options );

Documentation of @param options should not attempt to enumerate the options. That is best done where the options are defined, in the the _.extend call. In this case, something like:

options = _.extend( {
  radius: 10, // {number}
  position: Vector2.ZERO, // {Vector2}
  atomColor: 'black', // {Color|string}
  charge: 0, // {number}
  chargeColor: 'black' // {Color|string}
}, options );

dispenser.reset is not a function

This error occurs when pressing reset all on the 1st screen.

"Matrix was suspicious" assertion error when launching with ?ea

I tried running the simulation with ?ea to enable assertions and it failed to start, reporting a "matrix was suspicious" assertion error.

sim fails lint

% grunt
Running "jshint:allFiles" (jshint) task
Linting ../chipper/js/grunt/reportUnusedMedia.js ...ERROR
[L37:C68] W033: Missing semicolon.
  console.log( global.phet.chipper.licenseEntries.images );/report/XXX
Linting ../chipper/js/grunt/reportUnusedMedia.js ...ERROR
[L37:C71] W033: Missing semicolon.
  console.log( global.phet.chipper.licenseEntries.images );/report/XXX 
Linting ../chipper/js/grunt/reportUnusedMedia.js ...ERROR
[L37:C68] W117: 'XXX' is not defined.
  console.log( global.phet.chipper.licenseEntries.images );/report/XXX

Warning: Task "jshint:allFiles" failed. Use --force to continue.

Aborted due to warnings.
%

chem symbols should not be translatable

See phetsims/molecules-and-light#124.

Cannot read property 'x' of undefined: Assertions are triggered in Micro View

While running sugar and salt solution (Macro Tab) with assertions enabled and fuzzMouse, it takes a few seconds before getting the following error.

Particle.js:24 Uncaught TypeError: Cannot read property 'x' of undefined
    at OpenSite.Particle [as constructor] (Particle.js:24)
    at new OpenSite (OpenSite.js:29)
    at Crystal.js:162
    at u (lodash-4.17.4.min.js:5)
    at Function.ru (lodash-4.17.4.min.js:67)
    at Crystal.js:156
    at Array.forEach (<anonymous>)
    at ItemList.forEach (ObservableArray.js:301)
    at SodiumChlorideCrystal.getOpenSites (Crystal.js:155)
    at Crystal.js:131

Uncaught TypeError: window.phetSplashScreenDownloadComplete is not a function

Caught with test-server:

Uncaught TypeError: window.phetSplashScreenDownloadComplete is not a function
TypeError: window.phetSplashScreenDownloadComplete is not a function
    at new Sim (http://localhost:8080/joist/js/Sim.js?bust=1513027732832:77:12)
    at http://localhost:8080/sugar-and-salt-solutions/js/sugar-and-salt-solutions-main.js?bust=1513027732832:33:15
    at window.phetLaunchSimulation (http://localhost:8080/joist/js/SimLauncher.js?bust=1513027732832:47:11)
    at doneLoadingImages (http://localhost:8080/joist/js/SimLauncher.js?bust=1513027732832:69:18)
    at Object.launch (http://localhost:8080/joist/js/SimLauncher.js?bust=1513027732832:123:9)
    at http://localhost:8080/sugar-and-salt-solutions/js/sugar-and-salt-solutions-main.js?bust=1513027732832:32:15
    at Object.execCb (http://localhost:8080/sherpa/lib/require-2.1.11.js:1650:25)
    at Module.check (http://localhost:8080/sherpa/lib/require-2.1.11.js:866:35)
    at Module. (http://localhost:8080/sherpa/lib/require-2.1.11.js:1113:20)
    at http://localhost:8080/sherpa/lib/require-2.1.11.js:132:17

Move MacroModel.lineIntersectsBounds to dot.Util

Might be good for this math to be in dot.

  /**
   *
   * @param {number} x1
   * @param {number} y1
   * @param {number} x2
   * @param {number} y2
   * @param {Bounds2} r
   * @returns {*}
   */
  function lineIntersectsBounds( x1, y1, x2, y2, r ) {
    return Util.lineSegmentIntersection( x1, y1, x2, y2, r.minX, r.minY, r.maxX, r.minY ) ||
           Util.lineSegmentIntersection( x1, y1, x2, y2, r.maxX, r.minY, r.maxX, r.maxY ) ||
           Util.lineSegmentIntersection( x1, y1, x2, y2, r.maxX, r.maxY, r.minX, r.maxY ) ||
           Util.lineSegmentIntersection( x1, y1, x2, y2, r.minX, r.maxY, r.minX, r.minY ) ||
           (r.containsCoordinates( x1, y1 ) && r.containsCoordinates( x2, y2 ));
  }

filename prefixes don't follow PhET conventions

The convention for filename prefixes is to use either the full name of the repository, or the abbreviated acronym for the repository. E.g for expression-exchange, we would have either ExpressionExchangeConstants.js or EEConstants.js. The choice is up to the developer, should be applied consistently, and should not duplicate an abbreviation that is already used by another repository.

sugar-and-salt-solutions uses "SugarAndSaltSolution" (singular), "SugarAndSalt", and "SugarSalt" as prefixes, which don't follow either convention.