project-viable / viable-virtual-lab

Simulation framework to provide a virtual lab experience to teach research
https://project-viable.github.io/viable-virtual-lab/export/VirtualLabExport.html
Other
2 stars 0 forks source link

Change Gel Imager collision shape to polygon #151

Closed tylerhand1 closed 3 weeks ago

tylerhand1 commented 4 weeks ago

This makes the collision shape for the GelImager to be a CollisionPolygon2D. This makes it easier to click on the object so that the image appears or disappears.

This lead to an issue I found within LabObject.gd in the function GetIntersectingLabObjects(). It would crash from the line queryOptions.set_shape(child.shape).

The fix I opted for removed the check if the child is CollisionPolygon2D since it does not have the property shape. Instead, it has a polygon property which is of type PackedVector2Array.

As you are more familiar with the base LabObject class, I feel you would have a better understanding of this and will let you decide if I am wrong in this decision or if you know of a way to set the shape within queryOptions to be of this type.

tylerhand1 commented 4 weeks ago

I made this branch based off 139-filling-the-gel-rig-with-the-gel since I've made a lot of changes to GelImager in that branch. So, I felt it would be easier when merging, to do this instead of making a branch from dev.

If it makes it more difficult, I apologize. My intention is for this branch to be merged into 139-filling-the-gel-rig-with-the-gel before merging 139-filling-the-gel-rig-with-the-gel into dev once those issues are resolved.

tylerhand1 commented 3 weeks ago

I like these changes, but it seems like they may not fully fix issue #144. When I put the gel mold in the imager, some parts of it can still be clicked without the popup going away.

1 - Could the Gel Mold be receiving those clicks instead, even though it's in the slot? If so, should the new active property from the 139 branch prevent that problem?

2 - I think it may also be worth considering adding an explicit Close Button to that popup - I know I personally might not have thought to try clicking the object again.

@tylerhand1, that's all the input I have on this at the moment. I'll leave it up to you whether to go ahead and merge this branch into the 139 branch, especially if that would solve what I was talking about above with 1 (if I'm even right about the cause there). I'm not going to explicitly approve or disapprove this, and I'm not going to hit the merge button either, but feel free to do so if you think it should be done, especially since you're just merging into another issue branch.

  1. It technically is, though it does nothing with those clicks. So the active property won't help here. The problem seems to stem from the object being in front of the GelImager, so the GelImager cannot receive those clicks. Given how close we are to the Milestone 1 deadline, I think this should be left to a future group if they want to add the feature of switching which object is in front.
  2. Great idea! I copied the one from the GelMoldSubscene and it fits in well.