Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrapper Unit Tests: createTandem cannot accept dots, Property already deferred #68

Closed
jessegreenberg opened this issue Apr 26, 2019 · 6 comments

Comments

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Apr 26, 2019

While debugging #67 I am encountering these when running wrapper unit tests.

ncaught Error: Assertion failed: createTandem cannot accept dots: energySkatePark.labScreen.labModel.controlPoint.element~0.sourcePositionProperty

And

Assertion failed: Property already deferred

I am curious about the first error. "element" is not defined as a tandem name in the sim, so is "controlPoint.element~0" getting generated internally?

I ran ?fuzz with studio and I am not sure how else I can try to reproduce outside of unit tests. @samreid @chrisklus or @zepumph would you mind helping with these?

@zepumph zepumph changed the title Unit Tests: createTandem cannot accept dots, Property already deferred Wrapper Unit Tests: createTandem cannot accept dots, Property already deferred May 9, 2019
@zepumph
Copy link
Member

zepumph commented May 9, 2019

I can reproduce this by loading the state wrapper.

@zepumph
Copy link
Member

zepumph commented May 9, 2019

Bugs I've cleaned up in this issue (sorry they all sorta mixed together):

  • A few instances of phetioEngine.hasInstance weren't renamed to phetioEnging.hasPhetioObject
  • many NodeIO subproperties were added to the state when we worked on (https://github.com/phetsims/phet-io/issues/1420). In general this was planned, but for dynamic Nodes, we didn't want the sub Properties in the state at all, (most of the time the nodes were sub elements of a dynamic node that was dependent on a dynamic model element). Commits coming in for multiple sims here.
  • createTandem cannot accept dots assertion was fixed by adding createFromPhetioID, see https://github.com/phetsims/phet-io/issues/1442 for more info (because we overhauled Tandem as part of that issue) and I will link those commits over there too. I marked it as deprecated because I don't want people just using it before we know that it is a good pattern. I am treating it as a holdover right now.
  • I also changed the GroupTandem structure back from before we prototyped with it for new dynamic elements (Group.js stuff).
  • In CAF there was a function trying to be cloned over the frame, it was causing an error in the state wrapper.

@samreid I'm going to assign you to review, but ideally we should just have a conversation about this stuff. I think it will be much easier than trying to parse each thing above over comment chat.

zepumph added a commit to phetsims/balloons-and-static-electricity that referenced this issue May 9, 2019
zepumph added a commit to phetsims/circuit-construction-kit-common that referenced this issue May 9, 2019
zepumph added a commit to phetsims/energy-skate-park-basics that referenced this issue May 9, 2019
@samreid
Copy link
Member

samreid commented May 30, 2019

Reducing priority relative to other high priority tasks.

@samreid
Copy link
Member

samreid commented Apr 25, 2020

Unassigning until we turn our attention to phet-io for this sim.

@samreid samreid removed their assignment Apr 25, 2020
@pixelzoom
Copy link
Contributor

I'm not clear on what the status is for this issue. It's ready-for-review but unassigned. Looks like @zepumph did the bulk of the work as described in #68 (comment). So back to him to adjust labels and describe what remains to be done.

@zepumph
Copy link
Member

zepumph commented Feb 26, 2021

This older pattern of dynamic elements, and therefore need to create a hacky Tandem, was removed in phetsims/tandem#87.

There are no more usages of createFromPhetioID

CLosing

@zepumph zepumph closed this as completed Feb 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants