Closed pixelzoom closed 2 years ago
The easiest thing to do here would be to reimplement SoluteIO like this:
public static readonly SoluteIO = new IOType( 'SoluteIO', {
valueType: Solute,
supertype: ReferenceIO( IOType.ObjectIO ),
} );
I'm not really convinced that showing Solute name and pH in Studio has much value. The instructional designer can always consult the running sim. @arouinfar your thoughts?
I'll self-unassign until we hear from @arouinfar, but I agree if information is in the sim, it does not need to be restated in studio.
The problem is that the name field is now dynamic (changes with localeProperty) and it does not update where SoluteIO is displayed in Studio.
I'm not really convinced that showing Solute name and pH in Studio has much value.
Can we drop the name field and instead just include the phetioID and pH? We include the pH so users know the undiluted value.
I can certainly drop the 'name' field for SoluteIO.
But what's the general plan for when a dynamic string needs to appear in an IOType in Studio. @zepumph @samreid? Should I create a general issue, and in which repo?
https://github.com/phetsims/ph-scale/issues/243#issuecomment-1239765179 seems fine to me. I see 2 other problems though:
phScale.global.model.solutes.soda
and see its initialState
. It has serialization potential, but since it is phetioState:false
, we don't get serialization data in the wrapper.phScale.global.model.solutes.soda
says its IOType is ObjectIO instead of SoluteIO.If (1) was not the case, then https://github.com/phetsims/ph-scale/issues/243#issuecomment-1239765179 would be particularly ideal because we would be able to go to that in studio and see the state of it. @samreid, yet another time we wished we differentiated between allowing to have a serialized value VS. being included in state setting.
- It would be ideal if we could go to
phScale.global.model.solutes.soda
and see itsinitialState
. It has serialization potential, but since it isphetioState:false
, we don't get serialization data in the wrapper.
How/why does it have any serialization potential other than "reference"? It has no Properties that it owns, and nameProperty
is a references to a localized string Property.
- It seems like a weird bug in studio that I cannot explain that phScale.global.model.solutes.soda says its IOType is ObjectIO instead of SoluteIO.
That does seem buggy.
I see the same problem in BLL with static solutes under beersLawLab.global.model.solutes
. Static solutions under beersLawLab.beersLawScreen.model.solutions
look OK.
@arouinfar please review. SoluteIO 'name' field was removed in the above commit. Here's how the value of a soluteProperty
looks in Studio:
It's unclear if I need to comment on any parts of this, I'll self-unassign for now but please reinvite me if necessary.
Yes @samreid -- I would still like you or @zepumph to reply to https://github.com/phetsims/ph-scale/issues/243#issuecomment-1242544779:
But what's the general plan for when a dynamic string needs to appear in an IOType in Studio. @zepumph @samreid? Should I create a general issue, and in which repo?
But what's the general plan for when a dynamic string needs to appear in an IOType in Studio.
Dynamic, localized strings should not appear in IOType documentation or method names. If we need to link one PhetioObject to a LocalizedString, perhaps that could be done via a linked element?
Dynamic, localized strings should not appear in IOType documentation or method names.
It's not appearing in IOType documentation or method names. It's in the state object.
If we need to link one PhetioObject to a LocalizedString, perhaps that could be done via a linked element?
That is the problem that resulted in https://github.com/phetsims/axon/issues/414. LocalizedStrings are TReadOnlyProperty<string>
in Strings.ts files, which is not linkable.
In https://github.com/phetsims/axon/issues/414 we will make the localized strings linkable.
Copying the current value of one instrumented property so it will show up in the state of another PhetioObject doesn't sound like a good idea to me.
But what's the general plan for when a dynamic string needs to appear in an IOType in Studio. @zepumph @samreid? Should I create a general issue, and in which repo?
This sounds like a job for linked elements, if I understand correctly.
This sounds like a job for linked elements, if I understand correctly.
You understand correctly, and I agree. So I'll unassign @samreid and @zepumph.
I'll create a new ph-scale issue to link Solute to its nameProperty
when https://github.com/phetsims/axon/issues/414 has been addressed.
Remaining work here is review by @arouinfar, see https://github.com/phetsims/ph-scale/issues/243#issuecomment-1242563941.
Looks good, thanks @pixelzoom.
After adding support for dynamic locale in #239, we have a problem with SoluteIO. It is currently defined like this:
And the value shown in Studio displays 'phetioID', 'name', and 'pH' fields, for example:
The problem is that the name field is now dynamic (changes with localeProperty) and it does not update where SoluteIO is displayed in Studio.
@zepumph @samreid thoughts?