TheDuckCow / godot-road-generator

A godot plugin for creating 3D highways and streets.
MIT License
328 stars 17 forks source link

Initial gizmo handle jumps on cursor press #12

Closed TheDuckCow closed 1 year ago

TheDuckCow commented 1 year ago

See the video. It seems like there must be some viewport perspective issue at play which is causing the calculation. This functionality makes it very hard to finely tune the handle position as on first press and any translate, it shifts outwards noticeably.

This is a pre-existing issue for awhile. The answer will lay somewhere taking another look at func set_handle. A good counter reference would be how this repo does it, although they may not be implementing exactly the same logic of handler control.

An additional detail, this jumping does NOT happen if we are in a top-down view looking at the road point (and it is flat). Further implying it's an issue of the plane intersection not accounting for camera view correctly.

https://user-images.githubusercontent.com/2958461/197378041-f46869af-bb2b-447f-b1d6-7db7ed8e3de2.mp4

TheDuckCow commented 1 year ago

Confirming that indeed this is a good one for you to take a crack at @bdog2112, good luck!

bdog2112 commented 1 year ago

I took a break from Issue #4 to see if I could move this one forward. Owww! My head hurts from looking at this. :-) But, things are improving! Keep your eyes peeled for a pull request, soon.

Here's what needed to happen:

Some of these steps were already being done. The key hurdle was probably the conversion of the magnitude to a point in global space.

TheDuckCow commented 1 year ago

Just confirming @bdog2112, I see we still need that magnitude-zeroing change merged in, is that in a place that's ready to review/merge? Once we close this issue out, we can release the v0.1.1 milestone!

bdog2112 commented 1 year ago

That change is relatively small and easy to stage. For efficiency, I was going to roll it into a later pull request. Although, I don't see any reason it can't be released on its own. Today is pretty booked. But, I should be able to find some time before the end of the day. Yay v0.1.1!

On 11/22/2022 6:46 PM, Patrick W. Crawford wrote:

Just confirming @bdog2112 https://github.com/bdog2112, I see we still need that magnitude-zeroing change merged in, is that in a place that's ready to review/merge? Once we close this issue out, we can release the v0.1.1 milestone!

— Reply to this email directly, view it on GitHub https://github.com/TheDuckCow/godot-road-generator/issues/12#issuecomment-1324479017, or unsubscribe https://github.com/notifications/unsubscribe-auth/A3UUTNYNU3QYGVUQ4WLHEZDWJWAQNANCNFSM6AAAAAARMFDVI4. You are receiving this because you were mentioned.Message ID: @.***>

TheDuckCow commented 1 year ago

I do think it's worth making in its own change, so that any given PR stays focussed. But I also understand it's the week of Thanksgiving, so no stress if you don't get to it right away. Would be just good to do as the next quick step.On Nov 23, 2022 7:35 AM, bdog2112 @.***> wrote:

That change is relatively small and easy to stage. For efficiency, I was

going to roll it into a later pull request. Although, I don't see any

reason it can't be released on its own. Today is pretty booked. But, I

should be able to find some time before the end of the day. Yay v0.1.1!

On 11/22/2022 6:46 PM, Patrick W. Crawford wrote:

Just confirming @bdog2112 https://github.com/bdog2112, I see we

still need that magnitude-zeroing change merged in, is that in a place

that's ready to review/merge? Once we close this issue out, we can

release the v0.1.1 milestone!

Reply to this email directly, view it on GitHub

https://github.com/TheDuckCow/godot-road-generator/issues/12#issuecomment-1324479017,

or unsubscribe

https://github.com/notifications/unsubscribe-auth/A3UUTNYNU3QYGVUQ4WLHEZDWJWAQNANCNFSM6AAAAAARMFDVI4.

You are receiving this because you were mentioned.Message ID:

@.***>

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were assigned.Message ID: @.***>