Create separate groups for different resistor types

[arouinfar]

Related to #917

model.circuit.resistorGroup and view.circuitLayerNode.resistorNodeGroup contains different resistor types: standard resistors, grab-bag objects (pencil, coin, etc.), and high-resistance resistors, which is inconsistent with how we structure the groups for batteries and light bulbs.

Collapsing all of these resistors into one group makes it harder to distinguish between them. It also means that resistors will have properties that are not always applicable. For example, there isn't any reason for the pencil to have isEditiableProperty or isColorCodeVisibleProperty.

In discussions with @matthew-blackman and @samreid we decided to split the resistor group into three separate groups:

• resistorGroup for the standard resistors that support color coding.
• extremeResistorGroup for the resistors with a larger range of resistances that do not support color coding.
• objectGroup for the items that were originally in the "grab bag" in Java (pencil, coin, etc.) and have a fixed resistance.

[samreid]

I'd like to be involved in this implementation, but I also assigned @matthew-blackman in case we collaborate.

[samreid]

@matthew-blackman and I do not feel objectGroup is the most precise and appropriate name. We are wondering--how about something like miscellaneousGroup or miscellaneousItemGroup or miscellaneousObjectGroup? @arouinfar what do you think?

[arouinfar]

I don't love "miscellaneous" here. It's long and prone to typos. Maybe householdObjects or everydayObjects? These both have their own baggage, but I think it makes up for it with better interpretability.

[arouinfar]

Discussed with @matthew-blackman and @samreid and we decided to proceed with householdObjectGroup. @matthew-blackman will take the lead on implementation.

[arouinfar]

Note that householdObjectGroup should have the same phetioFeatured behavior as the resistors.

[matthew-blackman]

@samreid I think this is in a good place for you to review before I move on to adjusting the appropriate resistor-type-specific properties like isEditableProperty and isColorCodeVisibleProperty.

[samreid]

I'm seeing some duplicated code between createExtremeResistorToolNode and createHouseholdObjectToolNode and createExtremeResistorToolNode that would be good to factor out even more, let me know if we should collaborate on this.

[samreid]

@matthew-blackman and I took an initial look:

```diff Subject: [PATCH] hello --- Index: js/view/CircuitElementToolFactory.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/view/CircuitElementToolFactory.ts b/js/view/CircuitElementToolFactory.ts --- a/js/view/CircuitElementToolFactory.ts (revision 03c4a310af6f7289c31bb467fe69c2735dec4f31) +++ b/js/view/CircuitElementToolFactory.ts (date 1675460314229) @@ -321,7 +321,7 @@ } ); } - public createExtremeResistorToolNode( providedOptions?: CreateResistorToolNodeProvidedOptions ): CircuitElementToolNode { + public createAbstractResistorToolNode( providedOptions?: CreateResistorToolNodeProvidedOptions ): CircuitElementToolNode { const options = optionize()( { count: 4, lifelikeIconHeight: 15, @@ -355,6 +355,20 @@ } ); } + + public createExtremeResistorToolNode( providedOptions?: CreateResistorToolNodeProvidedOptions ): CircuitElementToolNode { + return this.createAbstractResistorToolNode( providedOptions, { + count: 4, + lifelikeIconHeight: 15, + schematicIconHeight: 14, + tandemName: 'extremeResistorToolNode', + group: this.circuit.extremeResistorGroup, + resistorType: ResistorType.HIGH_RESISTANCE_RESISTOR // todo required parameter? + // lifelikeIconHeight: options.lifelikeIconHeight, + // schematicIconHeight: 14 + } ); + } + public createHouseholdObjectToolNode( providedOptions?: CreateHouseholdObjectToolNodeProvidedOptions ): CircuitElementToolNode { const options = optionize()( { count: 10, ```

[samreid]

Using https://github.com/phetsims/circuit-construction-kit-common/blob/3d68e37f5304a7120a9d255871faf7fdfa0c9e4b/js/view/CircuitElementToolFactory.ts as a model, I unified the resistor type methods. It seems to be working OK but could use a review by @matthew-blackman.

[matthew-blackman]

This is looking great, thanks @samreid for reviewing and refactoring. It looks like there is one remaining issue in studio:

• isColorCodeVisibleProperty should be uninstrumented for extremeResistors and householdObjects.
• isEditableProperty should be uninstrumented for householdObjects

@samreid I have a fix for the circuit elements, but these properies are still showing in the archetype of the householdObjects group. Let's find a time to work on finalizing this together.

[samreid]

Fixed, thanks. @arouinfar ready for review. Please close if all is well.

[arouinfar]

Looks great, thanks!