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

Drag bug with electromagnet coil "background layer" #56

Closed
pixelzoom opened this issue Jan 17, 2024 · 2 comments
Closed

Drag bug with electromagnet coil "background layer" #56

pixelzoom opened this issue Jan 17, 2024 · 2 comments
Assignees
Labels
type:bug Something isn't working

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Jan 17, 2024

The background layer of ElectromagnetNode's CoilNode detaches from the coil when dragging it:

screenshot_2986
@pixelzoom pixelzoom added the type:bug Something isn't working label Jan 17, 2024
@pixelzoom pixelzoom self-assigned this Jan 17, 2024
@pixelzoom
Copy link
Contributor Author

pixelzoom commented Jan 17, 2024

This problem exposes some weirdness in the class hierarchy. PickupCoil extends Coil, and has LightBulb and Voltmeter fields. Electromagnet extends CoilMagnet, and has fields SourceCoil, DCPowerSupply, and ACPowerSupply. And SourceCoil has its own positionProperty, which is what is being set when dragging the background layer of the coil. This is also a problem for PhET-iO, because we have both electromagnet.positionProperty and electromagnet.sourceCoil.positionProperty.

I'm thinking that positionProperty should be removed from Coil, and PickupCoil should be structured more like Electromagnet.

Or maybe there should be another class (PickupDevice?) that include the PickupCoil, LightBulb and Voltmeter?

@pixelzoom pixelzoom changed the title Drag bug with electromagnet coil Drag bug with electromagnet coil "background layer" Jan 17, 2024
pixelzoom added a commit that referenced this issue Jan 20, 2024
@pixelzoom
Copy link
Contributor Author

pixelzoom commented Jan 20, 2024

This was addressed in the above commits. Rather than have PickupCoil extends Coil, I made PickupCoil have a Coil as a subcomponent. This allows me to get rid of positionProperty in Coil, which is what is causing the problem with the electromagnet background being draggable separately.

I don't feel like it needs a review, so closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant