phetsims / phetcommon

Code that is common to all PhET simulations.
MIT License
3 stars 5 forks source link

Combine all "check-***.js files? #23

Closed samreid closed 9 years ago

samreid commented 9 years ago

We have several (3?) check-***.js files. Should they be combined into a single "init" script?

samreid commented 9 years ago

This would have made adding check-arch.js easier.

samreid commented 9 years ago

At today's meeting we decided this is a good plan. Perhaps if someone's time is freed up, or the next time we need to visit all sim files.

pixelzoom commented 9 years ago

...perhaps in a file named check-namespaces.js.

samreid commented 9 years ago

I'm not sure everything there is a "namespace"--perhaps it should be check-globals.js or initializeGlobals.js

pixelzoom commented 9 years ago

Might be nice to put this in there?

window.phet = window.phet || {};

samreid commented 9 years ago

Maybe even something like:

if (window.phet){
  throw new Error('window.phet should not be defined at this point');
}
window.phet = {};

Thoughts @pixelzoom ?

pixelzoom commented 9 years ago

That looks OK to me.

samreid commented 9 years ago

Upon reflection, I prefer "initializeGlobals.js". OK @jbphet @jonathanolson @pixelzoom ?

jbphet commented 9 years ago

Sounds good to me.

pixelzoom commented 9 years ago

initializeGlobals.js sounds good to me too.

Since we decided to do this "if someone's time is freed up, or the next time we need to visit all sim files", I'm going to unassign this issue. Feel free to self-assign.

pixelzoom commented 9 years ago

Oh... I guess @samreid wants @jonathanolson to comment, so I'll assign to him.

jonathanolson commented 9 years ago

Sounds good to me. Also, since we've been hyphenating phetcommon (preload) names, would it be initialize-globals.js? (I don't have a strong preference except for some consistency).

samreid commented 9 years ago

initialize-globals.js sound good to me (though if we go to a single preload file, then convention won't matter at all). We are rolling query-parameters into this since it is global, right?

samreid commented 9 years ago

Just want to double check with @pixelzoom regarding query-parameters question above before starting work on this.

pixelzoom commented 9 years ago

@samreid asked:

We are rolling query-parameters into this since it is global, right?

That sounds right. Someone (@jonathanolson?) suggested that initialize-globals.js should live in chipper. If that's the case, you'll need to change phet.phetcommon.getQueryParameter to phet.chipper.getQueryParameter across all repos.

samreid commented 9 years ago

I'm presuming assert.js won't get rolled into this because we want to keep it separate. Right @jonathanolson @pixelzoom ?

samreid commented 9 years ago

Muchas commits forthcoming....

samreid commented 9 years ago

While I was working on this, I noticed some odd (broken?) paths to assert files in the molecule-shapes playground files. @jonathanolson do you want to take a look?

samreid commented 9 years ago

In the above commits, I moved the phetcommon/check-**.js files and query-parameters.js to chipper. I tested

I tried launching: http://localhost:8080/joist/tests/test-sims.html?duration=6000

and did not see any issues directly attributable to this work in the multi-sim fuzzer test, here is the output

Errors (tested in Chrome):

area-builder
Uncaught Error: Assertion failed: Error: Specified shape to be replaced does not appear to be present.
Error: Assertion failed: Error: Specified shape to be replaced does not appear to be present.
    at window.assertions.assertFunction (http://localhost:8080/assert/js/assert.js:22:13)
    at ShapePlacementBoard.inherit.replaceShapeWithUnitSquares (http://localhost:8080/area-builder/js/common/model/ShapePlacementBoard.js?bust=1423631042556:905:45)
    at inherit.addUserCreatedMovableShape.movableShape.userControlledProperty.lazyLink.decomposeShape (http://localhost:8080/area-builder/js/game/model/AreaBuilderGameModel.js?bust=1423631042556:88:44)
    at http://localhost:8080/area-builder/js/game/model/AreaBuilderGameModel.js?bust=1423631042556:91:69
    at Array.inherit.once.wrapper (http://localhost:8080/axon/js/Property.js?bust=1423631042556:224:11)
    at Property.inherit._notifyObservers (http://localhost:8080/axon/js/Property.js?bust=1423631042556:103:29)
    at Property.inherit._setAndNotifyObservers (http://localhost:8080/axon/js/Property.js?bust=1423631042556:89:14)
    at Property.inherit.set (http://localhost:8080/axon/js/Property.js?bust=1423631042556:62:16)
    at MovableShape.inherit.addGetterAndSetter.Object.defineProperty.set [as animating] (http://localhost:8080/axon/js/PropertySet.js?bust=1423631042556:133:43)
    at MovableShape.inherit.setDestination (http://localhost:8080/area-builder/js/common/model/MovableShape.js?bust=1423631042556:110:24)
bending-light
Uncaught Error: Assertion failed: Text should be defined and non-null. Use the empty string if needed.
Error: Assertion failed: Text should be defined and non-null. Use the empty string if needed.
    at window.assertions.assertFunction (http://localhost:8080/assert/js/assert.js:22:13)
    at Text.inherit.setText (http://localhost:8080/scenery/js/nodes/Text.js?bust=1423631100569:99:25)
    at http://localhost:8080/bending-light/js/common/view/IntensityMeterNode.js?bust=1423631100569:118:17
    at Property.inherit.link (http://localhost:8080/axon/js/Property.js?bust=1423631100569:156:11)
    at new BodyNode (http://localhost:8080/bending-light/js/common/view/IntensityMeterNode.js?bust=1423631100569:117:36)
    at new IntensityMeterNode (http://localhost:8080/bending-light/js/common/view/IntensityMeterNode.js?bust=1423631100569:174:21)
    at new ToolboxNode (http://localhost:8080/bending-light/js/common/view/ToolboxNode.js?bust=1423631100569:62:30)
    at new IntroView (http://localhost:8080/bending-light/js/intro/view/IntroView.js?bust=1423631100569:121:24)
    at IntroScreen.Screen.call.backgroundColor [as createView] (http://localhost:8080/bending-light/js/intro/IntroScreen.js?bust=1423631100569:26:34)
    at http://localhost:8080/joist/js/Sim.js?bust=1423631100569:388:28
capacitor-lab
Uncaught Error: Assertion failed: undefined
Error: Assertion failed: undefined
    at window.assertions.assertFunction (http://localhost:8080/assert/js/assert.js:22:13)
    at new ArrowNode (http://localhost:8080/scenery-phet/js/ArrowNode.js?bust=1423631136826:41:41)
    at new EFieldMeterNode (http://localhost:8080/capacitor-lab/js/capacitor-lab/view/meters/EFieldMeterNode.js?bust=1423631136826:92:17)
    at new CapacitorLabScreenView (http://localhost:8080/capacitor-lab/js/capacitor-lab/view/CapacitorLabScreenView.js?bust=1423631136826:127:23)
    at CapacitorLabScreen.Screen.call.backgroundColor [as createView] (http://localhost:8080/capacitor-lab/js/capacitor-lab/CapacitorLabScreen.js?bust=1423631136826:30:34)
    at http://localhost:8080/joist/js/Sim.js?bust=1423631136826:388:28
    at Function.St (http://localhost:8080/sherpa/lodash-2.4.1.min.js?bust=1423631136826:23:149)
    at new Sim (http://localhost:8080/joist/js/Sim.js?bust=1423631136826:378:7)
    at http://localhost:8080/capacitor-lab/js/capacitor-lab-main.js?bust=1423631136826:39:15
    at doneLoadingImages (http://localhost:8080/joist/js/SimLauncher.js?bust=1423631136826:31:9)
charges-and-fields
Uncaught Error: Assertion failed: the magnitude of the electric field is zero
Error: Assertion failed: the magnitude of the electric field is zero
    at window.assertions.assertFunction (http://localhost:8080/assert/js/assert.js:22:13)
    at ChargesAndFieldsModel.inherit.getNextPositionAlongEquipotentialWithElectricPotential (http://localhost:8080/charges-and-fields/js/charges-and-fields/model/ChargesAndFieldsModel.js?bust=1423631138219:561:58)
    at ChargesAndFieldsModel.inherit.getEquipotentialPositionArray (http://localhost:8080/charges-and-fields/js/charges-and-fields/model/ChargesAndFieldsModel.js?bust=1423631138219:641:38)
    at ChargesAndFieldsModel.inherit.addElectricPotentialLine (http://localhost:8080/charges-and-fields/js/charges-and-fields/model/ChargesAndFieldsModel.js?bust=1423631138219:863:48)
    at PencilButton.listener (http://localhost:8080/charges-and-fields/js/charges-and-fields/view/ElectricPotentialSensorPanel.js?bust=1423631138219:67:9)
    at http://localhost:8080/sun/js/buttons/PushButtonModel.js?bust=1423631138219:72:9
    at Array.forEach (native)
    at PushButtonModel.inherit.fire (http://localhost:8080/sun/js/buttons/PushButtonModel.js?bust=1423631138219:71:12)
    at http://localhost:8080/sun/js/buttons/PushButtonModel.js?bust=1423631138219:45:25
    at Array.inherit.onValue.onValueObserver (http://localhost:8080/axon/js/Property.js?bust=1423631138219:375:13)
charges-and-fields
Uncaught Error: Assertion failed: the magnitude of the electric field is zero
Error: Assertion failed: the magnitude of the electric field is zero
    at window.assertions.assertFunction (http://localhost:8080/assert/js/assert.js:22:13)
    at ChargesAndFieldsModel.inherit.getNextPositionAlongEquipotentialWithElectricPotential (http://localhost:8080/charges-and-fields/js/charges-and-fields/model/ChargesAndFieldsModel.js?bust=1423631138219:561:58)
    at ChargesAndFieldsModel.inherit.getEquipotentialPositionArray (http://localhost:8080/charges-and-fields/js/charges-and-fields/model/ChargesAndFieldsModel.js?bust=1423631138219:641:38)
    at ChargesAndFieldsModel.inherit.addElectricPotentialLine (http://localhost:8080/charges-and-fields/js/charges-and-fields/model/ChargesAndFieldsModel.js?bust=1423631138219:863:48)
    at PencilButton.listener (http://localhost:8080/charges-and-fields/js/charges-and-fields/view/ElectricPotentialSensorPanel.js?bust=1423631138219:67:9)
    at http://localhost:8080/sun/js/buttons/PushButtonModel.js?bust=1423631138219:72:9
    at Array.forEach (native)
    at PushButtonModel.inherit.fire (http://localhost:8080/sun/js/buttons/PushButtonModel.js?bust=1423631138219:71:12)
    at http://localhost:8080/sun/js/buttons/PushButtonModel.js?bust=1423631138219:45:25
    at Array.inherit.onValue.onValueObserver (http://localhost:8080/axon/js/Property.js?bust=1423631138219:375:13)
gravity-and-orbits
Uncaught Error: Assertion failed: Matrix was suspicious
Error: Assertion failed: Matrix was suspicious
    at window.assertions.assertFunction (http://localhost:8080/assert/js/assert.js:22:13)
    at Transform3.setMatrix (http://localhost:8080/dot/js/Transform3.js?bust=1423631240312:40:32)
    at Transform3.prependTranslation (http://localhost:8080/dot/js/Transform3.js?bust=1423631240312:76:12)
    at MassMenu.extend.prependTranslation (http://localhost:8080/scenery/js/nodes/Node.js?bust=1423631240312:1246:23)
    at MassMenu.extend.translate (http://localhost:8080/scenery/js/nodes/Node.js?bust=1423631240312:1090:16)
    at MassMenu.extend.setTranslation (http://localhost:8080/scenery/js/nodes/Node.js?bust=1423631240312:1224:12)
    at new MassMenu (http://localhost:8080/gravity-and-orbits/js/view/right-control-panel/mass-menu/MassMenu.js?bust=1423631240312:46:12)
    at new RightControlPanel (http://localhost:8080/gravity-and-orbits/js/view/right-control-panel/RightControlPanel.js?bust=1423631240312:39:7)
    at new GravityAndOrbitsView (http://localhost:8080/gravity-and-orbits/js/view/GravityAndOrbitsView.js?bust=1423631240312:37:22)
    at Screen.SimLauncher.launch.Screen.backgroundColor [as createView] (http://localhost:8080/gravity-and-orbits/js/gravity-and-orbits-main.js?bust=1423631240312:47:36)
isotopes-and-atomic-mass
Uncaught TypeError: Cannot read property 'viewToModelPosition' of undefined
TypeError: Cannot read property 'viewToModelPosition' of undefined
    at Object.options.start (http://localhost:8080/shred/js/view/BucketDragHandler.js?bust=1423631248310:30:39)
    at BucketDragHandler.inherit.startDrag (http://localhost:8080/scenery/js/input/SimpleDragHandler.js?bust=1423631248310:151:22)
    at BucketDragHandler.inherit.tryToSnag (http://localhost:8080/scenery/js/input/SimpleDragHandler.js?bust=1423631248310:178:14)
    at BucketDragHandler.inherit.down (http://localhost:8080/scenery/js/input/SimpleDragHandler.js?bust=1423631248310:195:12)
    at Input.inherit.dispatchToTargets (http://localhost:8080/scenery/js/input/Input.js?bust=1423631248310:798:31)
    at Input.inherit.dispatchEvent (http://localhost:8080/scenery/js/input/Input.js?bust=1423631248310:735:14)
    at Input.inherit.downEvent (http://localhost:8080/scenery/js/input/Input.js?bust=1423631248310:614:14)
    at Input.inherit.mouseDown (http://localhost:8080/scenery/js/input/Input.js?bust=1423631248310:292:14)
    at Display.extend.fuzzMouseEvents (http://localhost:8080/scenery/js/display/Display.js?bust=1423631248310:843:25)
    at animationLoop (http://localhost:8080/joist/js/Sim.js?bust=1423631248310:664:25)
molecule-shapes
Uncaught Error: Assertion failed: undefined
Error: Assertion failed: undefined
    at window.assertions.assertFunction (http://localhost:8080/assert/js/assert.js:22:13)
    at http://localhost:8080/molecule-shapes/js/common/view/3d/LabelWebGLView.js?bust=1423631275254:31:13
    at Object.context.execCb (http://localhost:8080/sherpa/require-2.1.11.js:1650:25)
    at Object.Module.check (http://localhost:8080/sherpa/require-2.1.11.js:866:35)
    at Object. (http://localhost:8080/sherpa/require-2.1.11.js:1113:20)
    at http://localhost:8080/sherpa/require-2.1.11.js:132:17
    at http://localhost:8080/sherpa/require-2.1.11.js:1156:11
    at each (http://localhost:8080/sherpa/require-2.1.11.js:57:23)
    at Object.Module.emit (http://localhost:8080/sherpa/require-2.1.11.js:1155:9)
    at Object.Module.check (http://localhost:8080/sherpa/require-2.1.11.js:917:18)
molecule-shapes-basics
Uncaught Error: Assertion failed: undefined
Error: Assertion failed: undefined
    at window.assertions.assertFunction (http://localhost:8080/assert/js/assert.js:22:13)
    at http://localhost:8080/molecule-shapes/js/common/view/3d/LabelWebGLView.js?bust=1423631279255:31:13
    at Object.context.execCb (http://localhost:8080/sherpa/require-2.1.11.js:1650:25)
    at Object.Module.check (http://localhost:8080/sherpa/require-2.1.11.js:866:35)
    at Object. (http://localhost:8080/sherpa/require-2.1.11.js:1113:20)
    at http://localhost:8080/sherpa/require-2.1.11.js:132:17
    at http://localhost:8080/sherpa/require-2.1.11.js:1156:11
    at each (http://localhost:8080/sherpa/require-2.1.11.js:57:23)
    at Object.Module.emit (http://localhost:8080/sherpa/require-2.1.11.js:1155:9)
    at Object.Module.check (http://localhost:8080/sherpa/require-2.1.11.js:917:18)
neuron
Uncaught Error: Script error for: SCENERY/layers/WebGLLayer
http://requirejs.org/docs/errors.html#scripterror
Error: Script error for: SCENERY/layers/WebGLLayer
http://requirejs.org/docs/errors.html#scripterror
    at makeError (http://localhost:8080/sherpa/require-2.1.11.js:166:13)
    at HTMLScriptElement.context.onScriptError (http://localhost:8080/sherpa/require-2.1.11.js:1681:26)
neuron
Uncaught Error: Script error for: SCENERY/layers/Renderer
http://requirejs.org/docs/errors.html#scripterror
Error: Script error for: SCENERY/layers/Renderer
http://requirejs.org/docs/errors.html#scripterror
    at makeError (http://localhost:8080/sherpa/require-2.1.11.js:166:13)
    at HTMLScriptElement.context.onScriptError (http://localhost:8080/sherpa/require-2.1.11.js:1681:26)
projectile-motion
Uncaught Error: Script error for: projectile-motion-config
http://requirejs.org/docs/errors.html#scripterror
Error: Script error for: projectile-motion-config
http://requirejs.org/docs/errors.html#scripterror
    at makeError (http://localhost:8080/sherpa/require-2.1.11.js:166:13)
    at HTMLScriptElement.context.onScriptError (http://localhost:8080/sherpa/require-2.1.11.js:1681:26)
sugar-and-salt-solutions
Uncaught Error: Assertion failed: Matrix was suspicious
Error: Assertion failed: Matrix was suspicious
    at window.assertions.assertFunction (http://localhost:8080/assert/js/assert.js:22:13)
    at Transform3.setMatrix (http://localhost:8080/dot/js/Transform3.js?bust=1423631367224:40:32)
    at Transform3.prependTranslation (http://localhost:8080/dot/js/Transform3.js?bust=1423631367224:76:12)
    at Text.extend.prependTranslation (http://localhost:8080/scenery/js/nodes/Node.js?bust=1423631367224:1246:23)
    at Text.extend.translate (http://localhost:8080/scenery/js/nodes/Node.js?bust=1423631367224:1090:16)
    at Text.extend.setX (http://localhost:8080/scenery/js/nodes/Node.js?bust=1423631367224:1157:12)
    at Text.x (http://localhost:8080/scenery/js/nodes/Node.js?bust=1423631367224:2256:27)
    at SaltShakerNode.inherit.updateTransform (http://localhost:8080/sugar-and-salt-solutions/js/common/view/DispenserNode.js?bust=1423631367224:102:24)
    at http://localhost:8080/sugar-and-salt-solutions/js/common/view/DispenserNode.js?bust=1423631367224:71:16
    at new Multilink (http://localhost:8080/axon/js/Multilink.js?bust=1423631367224:48:16)
samreid commented 9 years ago

This was a very large change and difficult to test all of the parts that may have broken because of this. I recommend all developers @pixelzoom @jonathanolson @jbphet @aaronsamuel137 @phetsims/bereaphysics test their current projects and make sure everything is working, and that one of the above do a review of my work.

samreid commented 9 years ago

Anybody want to review this? Math.random() suggested I assign it to @aaronsamuel137.

aaronsamuel137 commented 9 years ago

Well if Math.random() suggested me I don't think I can refuse to review this :)

I've just taken a glance around and everything looks good to me. I think it is safe to close this.