Add wires to floating terminals in the Test scene (important, but can be worked around) #31
Comments
|
I can't really speak to your priorities @samreid -- that's more of a question for @kathy-phet or @ariel-phet, but here's my two cents. I tend to agree with Shima here. If this were my study, I would want the black box wire stubs in both scenes for consistency. That said, I think this issue is somewhat contingent upon Shima's timeline (which I'm still waiting to hear back about). If she needs the sim in two weeks, it's probably not feasible to address this. But if the study is taking place in late February, I think that would give us enough time to address this issue. |
|
Possibly desired for #32 |
|
I spent about an hour on this, kind of got the wires working, but sometimes connections are impossible and current doesn't flow through. It may take 4-8 more hours to get something ready for QA testing. Here's my poor-man's branch: // Copyright 2015-2016, University of Colorado Boulder
// TODO: Review, document, annotate, i18n, bring up to standards
/**
* One scene for the black box screen, which focuses on a single black box and deals with the contents of the
* black box when the mode changes between investigate and build.
*
* @author Sam Reid (PhET Interactive Simulations)
*/
define( function( require ) {
'use strict';
// modules
var inherit = require( 'PHET_CORE/inherit' );
var circuitConstructionKitCommon = require( 'CIRCUIT_CONSTRUCTION_KIT_COMMON/circuitConstructionKitCommon' );
var CircuitConstructionKitModel = require( 'CIRCUIT_CONSTRUCTION_KIT_COMMON/common/model/CircuitConstructionKitModel' );
var CircuitStruct = require( 'CIRCUIT_CONSTRUCTION_KIT_COMMON/common/model/CircuitStruct' );
var Wire = require( 'CIRCUIT_CONSTRUCTION_KIT_COMMON/common/model/Wire' );
var Vertex = require( 'CIRCUIT_CONSTRUCTION_KIT_COMMON/common/model/Vertex' );
// phet-io modules
var TBoolean = require( 'ifphetio!PHET_IO/types/TBoolean' );
var TString = require( 'ifphetio!PHET_IO/types/TString' );
/**
* @param {CircuitStruct} trueBlackBoxCircuit - the circuit inside the black box (the true one, not the user-created one)
* @param {Tandem} tandem
* @constructor
*/
function BlackBoxSceneModel( trueBlackBoxCircuit, tandem ) {
assert && assert( trueBlackBoxCircuit instanceof CircuitStruct, 'circuit should be CircuitStruct' );
var self = this;
// Keep track of what the user has built inside the black box so it may be restored.
var userBlackBoxCircuit = new CircuitStruct( [], [], [], [], [], [] );
// When loading a black box circuit, none of the vertices should be draggable
// TODO: Fix this in the saved/loaded data structures, not here as a post-hoc patch.
for ( i = 0; i < trueBlackBoxCircuit.vertices.length; i++ ) {
trueBlackBoxCircuit.vertices[ i ].draggable = false;
if ( trueBlackBoxCircuit.vertices[ i ].attachable ) {
trueBlackBoxCircuit.vertices[ i ].blackBoxInterface = true;
trueBlackBoxCircuit.vertices[ i ].attachable = false;
}
else {
trueBlackBoxCircuit.vertices[ i ].insideTrueBlackBox = true;
}
}
var exploreStubs = [];
var toRemove = [];
// Add wire stubs outside the black box, see https://github.com/phetsims/circuit-construction-kit-black-box-study/issues/21
for ( var i = 0; i < trueBlackBoxCircuit.vertices.length; i++ ) {
var vertex = trueBlackBoxCircuit.vertices[ i ];
if ( vertex.blackBoxInterface ) {
toRemove.push( vertex );
vertex.blackBoxInterface = false;
console.log( vertex.positionProperty.value );
// the center of the black box is approximately (508, 305). Point the wires away from the box.
var side = vertex.positionProperty.value.x < 400 ? 'left' :
vertex.positionProperty.value.x > 600 ? 'right' :
vertex.positionProperty.value.y < 200 ? 'top' :
'bottom';
var extentLength = 40;
var dx = side === 'left' ? -extentLength :
side === 'right' ? +extentLength :
0;
var dy = side === 'top' ? -extentLength :
side === 'bottom' ? +extentLength :
0;
var outerVertex = new Vertex( vertex.positionProperty.value.x + dx, vertex.positionProperty.value.y + dy );
outerVertex.attachable = true;
outerVertex.blackBoxInterface = true;
outerVertex.draggable = false;
trueBlackBoxCircuit.wires.push( new Wire( vertex, outerVertex, 1E-6, {
wireStub: true
} ) ); // TODO: resistivity
exploreStubs.push( new Wire(
new Vertex( vertex.positionProperty.value.x, vertex.positionProperty.value.y, {
draggable: false,
attachable: true,
blackBoxInterface: true,
insideTrueBlackBox: false
} ),
new Vertex( vertex.positionProperty.value.x + dx, vertex.positionProperty.value.y + dy, {
draggable: false,
attachable: false,
blackBoxInterface: false,
insideTrueBlackBox: true
} ),
1E-6, {
canBeDroppedInToolbox: false, // false in Circuit Construction Kit Intro screen
interactive: false, // false for Black Box elements or wire stub
wireStub: true
}
) );
}
}
for ( var i = 0; i < toRemove.length; i++ ) {
var toRem = toRemove[ i ];
var index = trueBlackBoxCircuit.vertices.indexOf( toRem );
trueBlackBoxCircuit.vertices.splice( index, 1 );
}
// All of the circuit elements should be non-interactive
// TODO: Fix this in the saved/loaded data structures, not here as a post-hoc patch.
for ( i = 0; i < trueBlackBoxCircuit.circuitElements.length; i++ ) {
var circuitElement = trueBlackBoxCircuit.circuitElements[ i ];
circuitElement.interactive = false;
circuitElement.insideTrueBlackBox = true; // TODO
}
// additional Properties that will be added to supertype
var additionalProperties = {
// @public - whether the user is in the 'investigate' or 'build' mode
mode: {
value: 'investigate',
tandem: tandem.createTandem( 'modeProperty' ),
phetioValueType: TString
},
// @public - true when the user is holding down the reveal button and the answer (inside the black box) is showing
revealing: {
value: false,
tandem: tandem.createTandem( 'revealingProperty' ),
phetioValueType: TBoolean
},
// @public - true if the user has created a circuit for comparison with the black box (1+ terminal connected)
isRevealEnabled: {
value: false,
tandem: tandem.createTandem( 'isRevealEnabledProperty' ),
phetioValueType: TBoolean
}
};
CircuitConstructionKitModel.call( this, tandem, additionalProperties );
this.revealingProperty = this.revealingProperty || null;
this.modeProperty = this.modeProperty || null;
this.isRevealEnabledProperty = this.isRevealEnabledProperty || null;
// For syntax highlighting and navigation
this.circuit = this.circuit || null;
// @public {Bounds2} - filled in by the BlackBoxSceneView after the black box node is created and positioned
this.blackBoxBounds = null;
// When reveal is pressed, true black box circuit should be shown instead of the user-created circuit
this.revealingProperty.lazyLink( function( revealing ) {
self.mode = revealing ? 'investigate' : 'build';
} );
var circuit = this.circuit;
/**
* Check whether the user built (at least part of) their own black box circuit.
* @returns {boolean}
*/
var userBuiltSomething = function() {
var count = 0;
var callback = function( element ) {
var isConnectedToBlackBoxInterface = element.startVertex.blackBoxInterface || element.endVertex.blackBoxInterface;
if ( element.interactive && isConnectedToBlackBoxInterface ) {
count++;
}
};
circuit.wires.forEach( callback );
circuit.batteries.forEach( callback );
circuit.lightBulbs.forEach( callback );
circuit.resistors.forEach( callback );
return count > 0;
};
// Enable the reveal button if the user has done something in build mode.
circuit.circuitChangedEmitter.addListener( function() {
var builtSomething = self.mode === 'build' && userBuiltSomething();
self.isRevealEnabled = self.revealing || builtSomething;
} );
// Remove the true black box contents or user-created black box contents
var removeBlackBoxContents = function( blackBoxCircuit ) {
circuit.wires.removeAll( blackBoxCircuit.wires );
circuit.lightBulbs.removeAll( blackBoxCircuit.lightBulbs );
circuit.resistors.removeAll( blackBoxCircuit.resistors );
circuit.batteries.removeAll( blackBoxCircuit.batteries );
// Remove the vertices but not those on the black box interface
for ( var i = 0; i < blackBoxCircuit.vertices.length; i++ ) {
var vertex = blackBoxCircuit.vertices[ i ];
if ( !vertex.blackBoxInterface ) {
circuit.vertices.remove( vertex );
}
}
};
// Add the true black box contents or user-created black box contents
var addBlackBoxContents = function( blackBoxCircuit ) {
// Add the vertices, but only if not already added
for ( var i = 0; i < blackBoxCircuit.vertices.length; i++ ) {
var vertex = blackBoxCircuit.vertices[ i ];
if ( !vertex.blackBoxInterface ) {
circuit.vertices.add( vertex );
}
}
circuit.wires.addAll( blackBoxCircuit.wires );
circuit.resistors.addAll( blackBoxCircuit.resistors );
circuit.batteries.addAll( blackBoxCircuit.batteries );
circuit.lightBulbs.addAll( blackBoxCircuit.lightBulbs );
var updateElectrons = function( circuitElement ) { circuitElement.moveToFrontEmitter.emit(); };
blackBoxCircuit.wires.forEach( updateElectrons );
blackBoxCircuit.resistors.forEach( updateElectrons );
blackBoxCircuit.lightBulbs.forEach( updateElectrons );
blackBoxCircuit.batteries.forEach( updateElectrons );
// TODO: Switches
};
// Logic for changing the contents of the black box when the mode changes
// TODO: All of this logic must be re-read and re-evaluated.
this.modeProperty.link( function( mode ) {
// When switching to build mode, remove all of the black box circuitry and vice-versa
if ( mode === 'build' ) {
removeBlackBoxContents( trueBlackBoxCircuit );
// Any draggable vertices that remain should be made unattachable and undraggable, so the user cannot update the
// circuit outside the box
circuit.vertices.forEach( function( vertex ) {
if ( !vertex.blackBoxInterface ) {
vertex.attachable = false;
vertex.draggable = false;
vertex.interactive = false;
}
} );
circuit.circuitElements.forEach( function( circuitElement ) {
circuitElement.interactive = false;
} );
addBlackBoxContents( userBlackBoxCircuit );
}
else {
// Switched to 'investigate'. Move interior elements to userBlackBoxCircuit
userBlackBoxCircuit.clear();
circuit.vertices.forEach( function( v ) { if ( v.interactive && v.draggable ) {userBlackBoxCircuit.vertices.push( v );}} );
circuit.wires.forEach( function( wire ) { if ( wire.interactive ) { userBlackBoxCircuit.wires.push( wire ); } } );
circuit.batteries.forEach( function( b ) { if ( b.interactive ) { userBlackBoxCircuit.batteries.push( b ); } } );
circuit.lightBulbs.forEach( function( bulb ) { if ( bulb.interactive ) { userBlackBoxCircuit.lightBulbs.push( bulb ); } } );
circuit.resistors.forEach( function( r ) { if ( r.interactive ) { userBlackBoxCircuit.resistors.push( r ); } } );
exploreStubs.forEach(function(exploreStub){
userBlackBoxCircuit.wires.push(exploreStub);
userBlackBoxCircuit.vertices.push(exploreStub.startVertex);
userBlackBoxCircuit.vertices.push(exploreStub.endVertex);
});
removeBlackBoxContents( userBlackBoxCircuit );
// Any attachable vertices outside the box should become attachable and draggable
circuit.vertices.forEach( function( vertex ) {
if ( !vertex.blackBoxInterface ) {
vertex.draggable = true;
vertex.attachable = true;
vertex.interactive = true;
}
} );
circuit.circuitElements.forEach( function( circuitElement ) {
circuitElement.interactive = true;
} );
addBlackBoxContents( trueBlackBoxCircuit );
}
circuit.solve();
} );
// @private - called by reset
this.resetBlackBoxSceneModel = function() {
addBlackBoxContents( trueBlackBoxCircuit );
userBlackBoxCircuit.clear();
};
}
circuitConstructionKitCommon.register( 'BlackBoxSceneModel', BlackBoxSceneModel );
return inherit( CircuitConstructionKitModel, BlackBoxSceneModel, {
/**
* Reset the model.
* @overrides
* @public
*/
reset: function() {
CircuitConstructionKitModel.prototype.reset.call( this );
this.resetBlackBoxSceneModel();
}
} );
} ); |
|
Seeing as I have not been involved in the Stanford study work, I cannot comment on the relative priority here and will defer to @kathy-phet. @samreid my one thought would be, if this work is useful (or potentially useful) for the sim we intend to publish, it is worth doing. It seems unlikely to be helpful for getting DC published, but I know there is talk of eventually publishing a black box CCK to or website. 8 hours plus QA, plus time for fixing QA seems like quite a bit if it is truly a "one off" - but if it will eventually be needed for a published BB version, it seems more reasonable. |
|
When we eventually publish a black box simulation on our own site, we will need to rewrite many parts of this code. Implementing it now doesn't necessarily mean it will survive to the next iteration. The main benefit of working on it now would if it helped us decide how our version should eventually be designed (for instance if we add wire stubs and decide that we like them as a design feature). Many of the currently implemented black box features have been developed with the mind set that they would not need to be maintained in the long run or implemented hastily (hack alert!) for quick turn-around for studies. |
|
Another note: the black box study code is no longer compatible compatible with master. Changes made in the maintenance branch would have adapted to work for master. If we were planning to publish our own Black Box simulation soon I would take the time to get it working with master again and start re-architecting it soon rather than living in an old maintenance branch. However, I got the impression that not many more changes would be necessary for Stanford, so that's why we have been resorting to quick changes in the maintenance branch. If we are going to invest in a significant amount of QA time for the Stanford branches (i.e. if we need to make significant changes like this issue), I would also recommend that we spend time getting back to master so I can move forward with CCK DC sharing the same code. |
|
Let me talk with Amy before you spend any more time on this issue. Thx.
…Sent from my iPhone
On Jan 19, 2017, at 9:41 PM, Sam Reid <[email protected]<mailto:[email protected]>> wrote:
Another note: the black box study code is no longer compatible compatible with master. Changes made in the maintenance branch would have adapted to work for master. If we were planning to publish our own Black Box simulation soon I would take the time to get it working with master again and start re-architecting it soon rather than living in an old maintenance branch. However, I got the impression that not many more changes would be necessary for Stanford, so that's why we have been resorting to quick changes in the maintenance branch.
If we are going to invest in a significant amount of QA time for the Stanford branches (i.e. if we need to make significant changes like this issue), I would also recommend that we spend time getting back to master so I can move forward with CCK DC sharing the same code.
-
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#31 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AE3FZHkdsWqb4taRdnrG6OSFHeNO2_PYks5rUDrugaJpZM4LnNiW>.
|
|
@kathy-phet recommended an hour to investigate and up to 3 hours for implementation if it seems there is a tractable solution. |
|
After a bit less than an hour, I've published this version with the stubs in the user circuit implementation instead of the black box implementation: Known issues: (b) Reset all deletes the stubs. I think it will be straightforward to solve (b). @arouinfar can you test this version and see if there are any other problems? How important do you think it is to address (a) by making it impossible to attach to the black box vertex attached on the box side? |
|
I don't think it is important to address a.
…Sent from my iPhone
On Jan 20, 2017, at 1:40 PM, Sam Reid <[email protected]<mailto:[email protected]>> wrote:
After a bit less than an hour, I've published this version with the stubs in the user circuit implementation instead of the black box implementation:
http://www.colorado.edu/physics/phet/dev/html/circuit-construction-kit-black-box-study/1.0.8-dev.2/circuit-construction-kit-black-box-study_en.html
Known issues:
(a) you can attach to the base of the black box like so:
[image]<https://cloud.githubusercontent.com/assets/679486/22164769/cd31a0c4-df15-11e6-8e36-f88a57a98968.png>
(b) Reset all deletes the stubs.
I think it will be straightforward to solve (b). @arouinfar<https://github.com/arouinfar> can you test this version and see if there are any other problems? How important do you think it is to address (a) by making it impossible to attach to the black box vertex attached on the box side?
-
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#31 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AE3FZPxtLQLjEuM85DxinyE6J0HWu0_Hks5rURuqgaJpZM4LnNiW>.
|
|
This version addresses the wire stub resetting problem: @arouinfar this is ready for you to take a closer look. |
|
super quick look - 5 min - looks good to me. only comment I have is
it would be incrementally improved if the solder joints were layered
on top ... right now they seem to be below the special stubs. But
certainly could leave as it is.
EDIT: removed email metadata
|
|
noticed solder joints do show up nicely on joints after pressing
reset all ... if they could start with that look it seems like it
would be better.
kathy
EDIT: removed email metadata
|
|
@samreid overall, looking good! I haven't found any issues with 1.0.8-dev.3, aside from the solder joint issue that @kathy-phet described. I don't think we need to worry about issue (a) in #31 (comment), either. |
|
@samreid - FYI -- I will likely demo this version at tomorrows talk. |
|
@kathy-phet is |
|
The new wire stubs are working nicely, we'll continue in #36 for next steps, closing. |
#21 (comment)
Shima said:
@kathy-phet @ariel-phet @arouinfar how much time would you recommend I spend looking into this particular issue?
The text was updated successfully, but these errors were encountered: