godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.15k stars 97 forks source link

Add Transform helper functions / properties: "Transform.Forward" #5074

Open QueenOfSquiggles opened 2 years ago

QueenOfSquiggles commented 2 years ago

Describe the project you are working on

I am currently working on a tutorial series teaching how to make a first person shooter game in Godot 3.5 using C#. Because we are using C#, and because I used Unity for years before switching to Godot, I make a lot of references to how Unity approaches a problem and how the Godot approach is similar or different.

Specifically in episode 3 of my series I taught how to make a class called UTransform which takes in a Transform in the constructor and has several properties such as Forward, Right, and Up which return the basis vectors, inverted when appropriate. This was made to be similar to Unity's Transform implementation Unity Documentation here.

The code implementation of UTransform I referenced is here unfortunately I haven't added much documentation or comments yet, but hopefully the code is well written enough to be "self-documenting".

I did previously make a PR with this change to the C# struct Godot.Transform but since then I learned I need to start with a proposal. The original PR is here

Describe the problem or limitation you are having in your project

I personally feel that the Unity implementation, specifically Transform.forward provides some helpful values that could be useful to newer users who could have trouble with interacting with the basis property on Transform. Additionally, the helper functions would assist in helping developers to write more clean code, which would be easier to read and manage without excessive comments.

Specifically in my C# tutorial project code, we first used -camera.GlobalTransform.basis.z which could be replaced with camera.GlobalTransform.Forward, and using our helper class UTransform it became utrans.Forward which is even more readable, but required making a local variable.

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

The feature helps for the same reasons I consider it a problem. The helper functions/properties would result in cleaner, more self-documenting code as well as help to make lines less verbose.

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

My C# implementation already exists on [my fork of the Godot repo](). Specifically here in modules/transform/UTransform.cs. For that implementation, I used Properties which basically act as "getter" functions but are interacted with like they are member variables. As per the recommendation on my intial PR, I would be adding this also to the ClassDB so that such functionality would be included in GDScript as well.

My instinct is that GDScript would better handle having this feature as a function such as Transform.forward() but in the C# implementation I would use the same implementation as my previous PR, although I imagine that returning a normalized vector would be better.

I feel pretty confident in my ability to implement this feature, I just need to be sure I'm not the only one who would want this.

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

Essentially, this feature is an alternative approach towards finding the local directional vectors from the Transform.basis member. So any projects that currently use the basis property or would prefer to access the basis property directly would simply continue to do so. But for newer users, especially users who have experience in Unity and want to use Godot's C# capabilities, this would be incredibly helpful for ease of understanding.

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

I believe this should be a core implementation because it would be beneficial to a large number of users. I imagine users that commonly browse the GitHub here are likely to be more advanced (especially more advanced than me), but this fix would have helped me get started with making 3D games so much earlier if it existed back in 2020 when I started using Godot.

I should mention that I did literally make an implementation without access to the core already. But this change would be made so much better through a core implementation.

QueenOfSquiggles commented 2 years ago

I should also mention that my PC cannot run versions than require Vulkan, so unless I can only compile & test edits from the 3.X branch, so if there's some way I could develop on the 3.X branch and have the changes ported to the "master" branch, I would love to know how I should be doing that. Or if there is some way to run the 4.0 version with OpenGL instead of Vulkan that would work too, but as far as I was aware, OpenGL support was dropped for the 4.0 release.

Gnumaru commented 2 years ago

I should also mention that my PC cannot run versions than require Vulkan

When the opengl 3 driver is completed, you should be able to start without vulkan using "--rendering-driver opengl3". But for the time being (alpha 13) it seems that even with --rendering-driver opengl3 vulkan support is needed at startup.

Aside from that, you can use Mesa's vulkan software renderer. It is slow but at least works. I have tested it months ago and it at least worked, both on linux and windows.

raulsntos commented 2 years ago

Related: https://github.com/godotengine/godot/issues/17200 and https://github.com/godotengine/godot-proposals/issues/1710

aaronfranke commented 2 years ago

For working around this in your own C# code, there is a better way than your UTransform class:

Put this code in a file called TransformExtensions.cs ```cs using Godot; // Include this line for Godot 3 projects, remove it for Godot 4+ projects. using Transform3D = Godot.Transform; public static class TransformExtensions { public static Vector3 Right(this Transform3D transform) { return transform.basis.x.Normalized(); } public static Vector3 Left(this Transform3D transform) { return -transform.basis.x.Normalized(); } public static Vector3 Up(this Transform3D transform) { return transform.basis.y.Normalized(); } public static Vector3 Down(this Transform3D transform) { return -transform.basis.y.Normalized(); } public static Vector3 Forward(this Transform3D transform) { return -transform.basis.z.Normalized(); } public static Vector3 Back(this Transform3D transform) { return transform.basis.z.Normalized(); } public static Vector2 Right(this Transform2D transform) { return transform.x.Normalized(); } public static Vector2 Left(this Transform2D transform) { return -transform.x.Normalized(); } public static Vector2 Up(this Transform2D transform) { return transform.y.Normalized(); } public static Vector2 Down(this Transform2D transform) { return -transform.y.Normalized(); } } ```

The above code allows you to do things like this:

    public override void _Ready()
    {
        GD.Print(Transform.Forward());
    }
QueenOfSquiggles commented 2 years ago

@aaronfranke I didn't even know that kind of thing was possible. I will definitely be integrating that into my workflow! I do still like the idea of integrating the helper functions into core so the helpers are available in GDScript as well. I don't think there is such an elegant solution in the current version of GDScript

Diddykonga commented 2 years ago

I think these helper methods should exist as well. So technically they should be a part of Basis API since it is the one that contains the rotation/scale data. Although, I think it would still make sense to forward those helper methods to Transform API as well, since the majority of people interact with Basis through an Transform.

So: Basis.Forward() -> Vector3 Basis.Right() -> Vector3 Basis.Up() -> Vector3 Transform.Forward() -> Vector3 Transform.Right() -> Vector3 Transform.Up() -> Vector3

Not sure if Godot supports inline methods, but if so these could be force-inlined so that they don't have any runtime overhead. (Also for Transform2D, Which direction gets dropped, Up, Forward, or Right?)

aaronfranke commented 2 years ago

@Diddykonga Transform2D has left/right and up/down.

Diddykonga commented 2 years ago

@aaronfranke So I just looked at the Transform2D docs, do you know if there is a reason why we have Basis for 3D, but the Basis for 2D is just directly inside the Transform2D, even has an basis_xform() method. Should this be split out into an Basis2D and Basis3D respectively?

aaronfranke commented 2 years ago

@Diddykonga That's certainly an approach we can take, and there is no specific reason why we can't do this. The need for a separate structure is more important for 3D since it's more complex and worth splitting up, but we could do the same for 2D. Feel free to make a proposal so that this can be discussed in detail.

kiririn39 commented 1 year ago

I'v added calls like:

    inline godot::Vector3 GetLeft() const
    {
        return -basis.get_column(0);
    }

in transform3d.hpp in godot-cpp source in my local project

example GDExtension usage is:

godot::Vector3 forward = parent->get_transform().GetForward();
parent->get_transform().SetForward(forward);

Massively simpler and more usable in my opinion

I really don't understand why we cant have these convenience calls by default. Having to use transform.basis.x is literally unreadable.What is x? I dont know. I open that godot doc page every time to remember. Situation is even worse in GDExtension as of 4.0. We don't even have transform.basis.x to begin with, instead have to use basis.get_column(0); no jokes. What kind of person is supposed to use this in their game code effectively?

Hope it will be added as soon as possible