convert common-code query parameters to QueryStringMachine

[pixelzoom]

Factored out of https://github.com/phetsims/query-string-machine/issues/15#issuecomment-256709865.

Consolidate "post-load" common-code query parameters (and their documentation) in chipper.QueryParameters. For places where those query parameters are needed, add CHIPPER to config.js and use a requirejs statement to access them. E.g.:

var QueryParameters = require( 'CHIPPER/QueryParameters' );
...
if ( QueryParameters.dev ) { ... }

For preload query parameters, leave those parameters and the initialization of phet.chipper.getQueryParameter in initialize-globals.js.

[pixelzoom]

This turned into a mess because chipper.QueryParameters needs to have chipper.js Namespace, and that fails because preloads create window.phet.chipper.

[pixelzoom]

Having separate preload and post-load query parameters also seems problematic. Some of the preload query parameters are used by both preload and post-load code. And then there's the documentation issue - would be nice if all query parameters were documented in 1 place.

[pixelzoom]

3.5 hours into this so far. I checked in intermediate work on QueryParameter, the schema has been tested, but it's not ready for production use. And I'm blocked until we can work out the namespace issue. Labeling this for discussion at developer meeting because I wasn't getting needed feedback on Skype.

[samreid]

I consider it odd that chipper would have only 1 module that gets loaded by requirejs, it could be confusing. It seems vestigial that query parameters were historically dealt with in chipper, perhaps now it would make more sense to have this in another repo, such as Joist.

How many "preload" query parameters are there? If it is a small number, it might make sense to handle them separately.

[samreid]

Let's keep everything as preload, so all query parameters can be kept together, but making it a global instead of a requirejs module.

[samreid]

@jonathanolson and @andrewadare said we should give that a try.

[pixelzoom]

Since I intended to do this in master, further work is on hold pending discussion of https://github.com/phetsims/phet-io/issues/831.

[pixelzoom]

I just discovered a boatload of query parameters in phet-io that are being loaded using QueryStringMachine.get. Presumable those should be moved to the master QueryStringMachine schema? And and accessed via phet.chipper.queryParameters? @samreid please advise.

[pixelzoom]

Or perhaps we should remove phet-io query parameters from chipper.QueryParameters and phet.chipper.queryParameters. Make phet-io have its own schema and phet.phetio.queryParameters.

[samreid]

phet-io should have its own schema for query parameters that apply to most wrappers. But many of the query parameters are specific to a single wrapper. For those cases, the wrapper should define the schema. @andrewadare, @zepumph and I can take care of porting the phet-io query parameters to QSM.

EDIT: fixed typos

[pixelzoom]

I added this stub in initialize-globals.js:

    window.phet.chipper.queryParameters = QueryStringMachine.getAll( {
      dev: { type: 'flag' }
    } );

[pixelzoom]

Re phetsims/phet-io#831... we decided OK to continue working in master. Removing "on-hold" label.

[pixelzoom]

Lots of progress, but a few fundamental issues to be discussed:

(1) The semantics of defaultValue seem to be "the value to use when a query parameter doesn't include a value, or when the query parameter is altogether absent". This makes it difficult to determine whether a query parameter was actually provides and should be used to override program defaults.

(2) Query parameters of type 'string' should perhaps default to null? This is related to (1). Specifying null as the default value has so far been a panacea for not being able to know whether the query parameter is present.

(3) fuzzMouse and webglContextLossTimeout are proving to be problematic because they are trying to be both a flag and a number. Again related to (1), we can't tell whether the query parameter was absent, or whether its value was missing. Perhaps we should split these each into 2 query parameters (e.g. fuzzMouse and fuzzMouseEventsPerFrame).

(4) phet-io and phet-metacog use getQueryParameter, they need to be converted.

[pixelzoom]

I'd like to discuss the above issue with @samreid and @jonathanolson, at your convenience.

[jonathanolson]

Discussed via call, removing assignment.

[samreid]

I fixed and created and re-fixed a few problems in the above commits. Let me know if you need anything else from me.

[pixelzoom]

The final usages of getQueryParameter are in phetsims/phet-io#856 and phetsims/phet-metacog#15, assigned to @samreid. After those are addressed, the deprecated code (marked with TODO items) can be deleted from initialize-globals.js.

EDIT: depreciated => deprecated

[pixelzoom]

@samreid Please assign back to me when the phet-io and phet-metacog usages have been addressed.

[samreid]

@pixelzoom I've reassigned both of those issues to you for discussion, and created a new issue for discussion with @mattpen in https://github.com/phetsims/phet-io-website/issues/82

The issue https://github.com/phetsims/phet-io-website/issues/82 can proceed in parallel with removing getQueryParameter from chipper, because it does not use the chipper implementation at all.

[pixelzoom]

@samreid Also waiting on this one: https://github.com/phetsims/scenery/issues/583

[samreid]

@pixelzoom volunteered to tackle the aforementioned issue in https://github.com/phetsims/scenery/issues/583#issuecomment-260697626 and I recommended how it could be done in https://github.com/phetsims/scenery/issues/583#issuecomment-260699694

[pixelzoom]

All conversion to QueryStringMachine completed, getQueryParameter and related code have been removed.

All that remains is to document a few query parameters in initialize-globals.js, as indicated by TODO items.

[pixelzoom]

All query parameters have been documented. All work completed, ready for review. Volunteers?

[pixelzoom]

> phet.joist.random.shuffle( ['AA', 'JB', 'JG', 'JO', 'MK', 'MP', 'SR' ])[0]
"JG"

Randomly assigned to @jessegreenberg to review.

[pixelzoom]

@jessegreenberg It's probably sufficient to review QUERY_PARAMETERS_SCHEMA in initialize-globals.js, and test a few query parameters. Since this was an extensive/invasive change, I expect that we'll need to shake out any other problems as they appear.

[pixelzoom]

I'm going to go ahead and close this. It's been pending review for 17 days, so we've sort of missed our window to catch problems before they impacted someone. The good news that that no one has reported significant problems, so I guess we'll just deal with problems if/when they occur.