phetsims/quadrilateral

Avoid duplicated checks for valid shape in TangibleConnectionModel

samreid opened this issue · 1 comments

Discovered in #398, @jessegreenberg and @matthew-blackman and I were looking at

// No lines intersect
// REVIEW: Could this be simpler?
if ( allowed ) {
for ( let i = 0; i < proposedLines.length; i++ ) {
const firstLine = proposedLines[ i ];
for ( let j = 0; j < proposedLines.length; j++ ) {
const secondLine = proposedLines[ j ];
if ( firstLine !== secondLine ) {
if ( Line.intersectOther( firstLine, secondLine ).length > 0 ) {
allowed = false;
break;
}
}
}
if ( !allowed ) {
break;
}
}
}
and wondering if it would be better to create a scratch QuadrilateralShapeModel then ask if it is a valid shape? We think there may be duplicated logic here.

The code referenced here is totally removed, and its using QuadrilateralShapeModel.isQuadrilateralShapeAllowed( testShapeModel ); instead. So much nicer!

I was able to test it with ?cameraInput=hands, and it is working well, maybe even better - before the change I was seeing what looked like incorrect shapes sometimes. Closing.