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

Rename some properties of PathFollower3D and PathFollower2D #5104

Closed
xiongyaohua opened this issue Aug 8, 2022 · 0 comments · Fixed by godotengine/godot#64804
Closed

Rename some properties of PathFollower3D and PathFollower2D #5104

xiongyaohua opened this issue Aug 8, 2022 · 0 comments · Fixed by godotengine/godot#64804
Labels
breaks compat Proposal will inevitably break compatibility topic:core
Milestone

Comments

@xiongyaohua
Copy link

xiongyaohua commented Aug 8, 2022

Describe the project you are working on

A road traffic game, where roads are described by 2D or 3D curves. Curves are constantly sampled in order to convert the position of cars from linear coordinate to descartes coordinate.

Describe the problem or limitation you are having in your project

In the process of moving rotation interpolation code from PathFollower to Curve(see PR #63956, PR #64043), I feel some naming of properties are not clear.

  1. On PathFollower3D:

    • h_offset: the doc says "The node's offset along the curve". But it is incorrect, it actually represents the offset along the "Sideway"(right hand) direction in the code.
    • v_offset: the doc says "The node's offset perpendicular to the curve". The concepts of "vertical" or "perpendicular" make sense for 2D curve but are ambiguous for 3D curve. It actually represents the offset along the "Up" direction in the code.
    • for completeness, there should be an offset along the tangent direction(or "Forward" direction in the code), but it is missing.
  2. On both PathFollower2D/3D: the offset property indicate the distance moving from the beginning along the curve, which have different quality as h_offset or v_offset, as they move along straight vector. Use the same word "offset" could create confusion for new user.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

for PathFollower3D
combine v_offset ,h_offset, f_offset into a Vector3 offset

for PathFollower2D
combine v_offset, h_offset into a vector3 offset,

for both PathFollower3D and PathFollower2D
- change offset to progress,
- change unit_offset to progress_ratio

The new names feel more consistent and clear.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

I can make this change in a separate PR.

If this enhancement will not be used often, can it be worked around with a few lines of script?

no

Is there a reason why this should be core and not an add-on in the asset library?

they are part of the engine API

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaks compat Proposal will inevitably break compatibility topic:core
Projects
None yet
2 participants