marlitas
commented
2 weeks ago Here is a patch where I have some debugging code to indicate when cubes reach an "out of order" state. This patch also changes how cubes are initially placed to match the "free drag" aesthetic and remove cube separator reliance on "snap to positions"
```diff
Subject: [PATCH] Update separator logic
---
Index: js/common/view/CubesOnWireNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/CubesOnWireNode.ts b/js/common/view/CubesOnWireNode.ts
--- a/js/common/view/CubesOnWireNode.ts (revision 14251b4b3a2feb359183e76734922309ee187248)
+++ b/js/common/view/CubesOnWireNode.ts (date 1730845762146)
@@ -28,6 +28,7 @@
import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js';
import { Shape } from '../../../../kite/js/imports.js';
import NumberProperty from '../../../../axon/js/NumberProperty.js';
+import dotRandom from '../../../../dot/js/dotRandom.js';
type SelfOptions = {
sceneRange: Range;
@@ -37,6 +38,8 @@
const LEFT_MOST_CUBE_X = 1;
const END_CAP_RADIUS = 10;
+const SEPARATOR_RANGE = new Range( 2, 24 ); // empirically determined
+
type CubesOnWireNodeOptions = StrictOmit & SelfOptions & PickRequired;
export default class CubesOnWireNode extends Node {
@@ -60,7 +63,7 @@
) {
const modelViewTransform = ModelViewTransform2.createSinglePointScaleMapping(
new Vector2( 0, 0 ),
- new Vector2( CUBE_WIDTH, 0 ),
+ new Vector2( CUBE_WIDTH / 2, 0 ), // center of the cube
CUBE_WIDTH - CUBE_OVERLAP
);
const wire = new Line( 0, 0, countingAreaBounds.width, 0, {
@@ -100,6 +103,7 @@
const cubeNode = new CubeNode(
countingObject,
{
+ opacity: 0.7,
tandem: providedOptions.tandem.createTandem( `cubeNode${i}` ),
onDrop: () => {
this.cubeDragging = false;
@@ -112,15 +116,23 @@
this.cubeModelToNodeMap.set( countingObject, cubeNode );
} );
- // TODO: When we add cubes to the wire they should not snap. They should be placed in random spots
Multilink.multilink( [
model.leftAddendProperty,
- model.rightAddendProperty,
- model.totalProperty,
- model.leftAddendCountingObjectsProperty,
- model.rightAddendCountingObjectsProperty
- ], () => {
- !this.cubeDragging && this.snapCubesToPositions();
+ model.rightAddendProperty
+ ], ( leftAddend, rightAddend ) => {
+ if ( !this.cubeDragging ) {
+ const cubeSeparatorCenterX = calculateCubeSeparatorPlacement( leftAddend );
+ this.cubeSeparatorCenterXProperty.value = this.modelViewTransform.modelToViewX( cubeSeparatorCenterX );
+
+ const leftAddendPositionRange = new Range( LEFT_MOST_CUBE_X, cubeSeparatorCenterX - 1 );
+ this.setRandomPositions( model.leftAddendCountingObjectsProperty.value, leftAddendPositionRange );
+
+ const rightAddendPositionRange = new Range(
+ cubeSeparatorCenterX + 1,
+ this.modelViewTransform.viewToModelDeltaX( countingAreaBounds.width ) - 1 );
+ this.setRandomPositions( model.rightAddendCountingObjectsProperty.value, rightAddendPositionRange );
+
+ }
} );
this.cubeModelToNodeMap.forEach( cube => {
@@ -128,6 +140,28 @@
} );
}
+ private setRandomPositions( countingObjects: CountingObject[], positionRange: Range ): void {
+ const availablePositions = [];
+ for ( let i = positionRange.min; i < positionRange.max; i++ ) {
+ availablePositions.push( i );
+ }
+ assert && assert( availablePositions.length >= countingObjects.length, 'There are not enough available positions for counting objects' );
+
+ const leftAddendPositions: number[] = [];
+ for ( let i = 0; i < countingObjects.length; i++ ) {
+ const randomPosition = dotRandom.sample( availablePositions );
+ availablePositions.splice( availablePositions.indexOf( randomPosition ), 1 );
+ leftAddendPositions.push( randomPosition );
+ }
+ assert && assert( leftAddendPositions.length === countingObjects.length, 'leftAddendPositions.length should match leftAddendCountingObjects length' );
+ leftAddendPositions.sort( ( a, b ) => a - b );
+ countingObjects.forEach( ( countingObject, i ) => {
+ const cube = this.cubeModelToNodeMap.get( countingObject )!;
+ cube.center = new Vector2( this.modelViewTransform.modelToViewX( leftAddendPositions[ i ] ), 0 );
+ } );
+
+ }
+
/**
* Snap the cubes to their positions on the wire based on the addend values. By default, the cubes are arranged in
* groups of 5 with a separator between the two addends.
@@ -189,7 +223,9 @@
// Calculate the distance the grabbed cube has moved and constrain the movement so that it does not exit
// the drag bounds.
const oldCenterX = grabbedCube.centerX;
- const dragBoundsWithMovingCubes = this.cubeDragBounds.dilatedX( -( CUBE_WIDTH - CUBE_OVERLAP ) * ( cubesToMove.length - 1 ) );
+ const minXOffset = draggingRight ? 0 : -( CUBE_WIDTH - CUBE_OVERLAP ) * ( slicedCubes.length - 1 );
+ const maxXOffset = draggingRight ? -( CUBE_WIDTH - CUBE_OVERLAP ) * ( slicedCubes.length - 1 ) : 0;
+ const dragBoundsWithMovingCubes = this.cubeDragBounds.withOffsets( minXOffset, 0, maxXOffset, 0 );
const newCenterX = dragBoundsWithMovingCubes.closestPointTo( grabbedCube.globalToParentPoint( newPosition ) ).x;
const deltaX = newCenterX - oldCenterX;
grabbedCube.centerX = newCenterX;
@@ -203,7 +239,8 @@
if ( !this.rightAddendCountingObjectsProperty.value.includes( cube.model ) ) {
// Since a cube is moving to the right, the separator should adjust one position to the left.
- this.cubeSeparatorCenterXProperty.value = this.modelViewTransform.modelToViewX( calculateCubeSeparatorPlacement( this.model.leftAddendProperty.value - 1 ) );
+ this.cubeSeparatorCenterXProperty.value = this.modelViewTransform.modelToViewX(
+ calculateCubeSeparatorPlacement( this.model.leftAddendProperty.value - 1 ) );
// Add the cube to the right addend first to avoid duplicate work being done when the left addend value is
// updated in the ObservableArray.lengthProperty listener.
@@ -211,12 +248,17 @@
this.rightAddendCountingObjectsProperty.value.add( cube.model );
this.leftAddendCountingObjectsProperty.value.remove( cube.model );
cube.model.traverseInactiveObjects = true;
+
+ // Immediately move the cubes past the separator
+ cube.centerX = Math.max( cube.centerX, this.cubeSeparatorCenterXProperty.value + CUBE_WIDTH - CUBE_OVERLAP );
}
}
if ( cube.centerX < this.cubeSeparatorCenterXProperty.value ) {
if ( !this.leftAddendCountingObjectsProperty.value.includes( cube.model ) ) {
+
// Since a cube is moving to the left, the separator should adjust one position to the right.
- this.cubeSeparatorCenterXProperty.value = this.modelViewTransform.modelToViewX( calculateCubeSeparatorPlacement( this.model.leftAddendProperty.value + 1 ) );
+ this.cubeSeparatorCenterXProperty.value = this.modelViewTransform.modelToViewX(
+ calculateCubeSeparatorPlacement( this.model.leftAddendProperty.value + 1 ) );
// Remove the cube from the right addend first to avoid duplicate work being done when the left addend value is
// updated in the ObservableArray.lengthProperty listener.
@@ -224,6 +266,9 @@
this.rightAddendCountingObjectsProperty.value.remove( cube.model );
this.leftAddendCountingObjectsProperty.value.add( cube.model );
cube.model.traverseInactiveObjects = true;
+
+ // Immediately move the cubes past the separator
+ cube.centerX = Math.min( cube.centerX, this.cubeSeparatorCenterXProperty.value - CUBE_WIDTH + CUBE_OVERLAP );
}
}
} );
@@ -243,14 +288,23 @@
assert && assert( this.leftAddendCountingObjectsProperty.value.length === this.model.leftAddendProperty.value, 'leftAddendObjects.length should match leftAddendNumberProperty' );
assert && assert( this.rightAddendCountingObjectsProperty.value.length === this.model.rightAddendProperty.value, 'rightAddendObjects.length should match rightAddendNumberProperty' );
+
+ if ( assert ) {
+ const activeCubes = cubeNodes.filter( cube => cube.model.addendTypeProperty.value !== AddendType.INACTIVE )
+ .sort( ( a, b ) => a.centerX - b.centerX );
+ let previousIndex = 0;
+ assert( activeCubes.every( cube => {
+ const idCorrectSequence = cube.model.id >= previousIndex;
+ previousIndex = cube.model.id;
+ return idCorrectSequence;
+ } ), 'cubes in wrong order' );
+ }
}
}
-function calculateCubeSeparatorPlacement( leftAddendValue: number ): number {
-
- // The cube separator should not be grouped as part of the groups of 5.
- const separatorAdjustment = leftAddendValue % 5 === 0 ? 1 : 0;
- return Math.floor( leftAddendValue / 5 ) + leftAddendValue - separatorAdjustment + LEFT_MOST_CUBE_X;
+function calculateCubeSeparatorPlacement( leftAddend: number ): number {
+ const separatorMargin = leftAddend / 5 + 1;
+ return SEPARATOR_RANGE.constrainValue( leftAddend + separatorMargin );
}
class WireEndCap extends Path {
```
pixelzoom
catherinecarter
Cubes should default to "free dragging". There will be an organize button that groups them in 5's.