VisualScriptEditor Fix in graph position calculation (do not skip zoom)

[kleonc]

Calculations for converting in-editor position to in-graph position was made manually in many separate places, some of them didn't take zoom into account. Extracted that into seperate method.

Tested it briefly in 3.x, seems to work. But more testing is welcomed before merging.

Fixes #49840 (and probably more zoom-related issues, e.g. before this PR it was possible to start dragging a node by clicking outside of it, with this PR it seems to no longer be the case).
Cherry-pickable 3.x, 3.3.

[Gallilus]

Oh seeing this makes me happy.
I did a very similar thing in #49749 I have to admit this version is cleaner. Better readability etc.
I will let @fire merge this PR first then I will do a last rebase on my PR. That will provide the cleanest result.

Code seems good.
Tested The windows build its OK.
Only one request for @kleonc, do you have any id where the position is decided when you drag out from the sequence port?

It seems to set position on window zero or last position.

[kleonc]

Only one request for @kleonc, do you have any id where the position is decided when you drag out from the sequence port?

After a little digging I think here's the relevant part (linked code includes this PR). As you can see it uses port_action_pos to determine the actual in-graph position. In the case you've mentioned port_action_pos seems to be set here. And indeed this condition doesn't check for output sequence port count so port_action_pos might not being set properly. In 3.x it's being set unconditionally. Will try to fix this in a moment.

@DavidCambre And big thanks for the review/feedback!

[fire]

Looks good.

Thanks for reviewing @DavidCambre and @kleonc for working on it.

[Gallilus]

@fire I do not now ho to ask. Please merge? So I can work on the rebase for PR49749

[fire]

Thanks.

[akien-mga]

Cherry-pickable 3.x, 3.3.

This would need a dedicated PR for 3.x, there are significant conflicts to handle.

[kleonc]

This would need a dedicated PR for 3.x, there are significant conflicts to handle.

It's because of this change since that changed condition is non-existant in 3.x (as I've mentioned in the comment above). Not sure if that condition is neccessary at all but I'll add it in 3.x too to sync that part with master (code involving port_action_pos (which is being changed conditionally in there) seems the same in both master and 3.x so it shouldn't break anything).

[veryprofessionaldodo]

I think that this is still a problem. Currently building a plugin that uses a Visual Node system for dialogues, and I get this behaviour on 3.3.3.rc1 and 3.4.beta3. Here's a gif of it in action.

Sorry to resurrect an old issue, I'll open a new one if it is more appropriate.

[kleonc]

@veryprofessionaldodo As I've just mentioned in the Reddit thread where you're describing your issue: this PR is actually not related to your issue. So yes, please open a new issue. Also provided gif is a nice addition but it's much easier to investigate the issue having minimal reproduction project provided (please provide one when opening new issue).

[veryprofessionaldodo]

Hi there! It was actually my bad, and not related to this at all indeed. The problem was that I had a hidden AcceptDialog inside of the GraphEdit that was catching inputs, as it can be seen here. Sorry for adding entropy, it's already fixed!