godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.02k stars 21.09k forks source link

look_at() with Tween #17577

Closed brylie closed 4 years ago

brylie commented 6 years ago

Godot version: 3.0.2

OS/device including version: Ubuntu 17.10

Description

When using look_at(), the transformation is very fast, e.g. the object immediately rotates. This seems a bit unnatural, and would be better if the rotation could be smooth.

Feature request

Allow look_at() to take a Tween argument, so that:

Rationale

Using the phrase look at is fairly common and imbued with meaning that even children understand. That is likely why look_at was included in the Godot API. Whereas, concepts like interpolate_property are more abstract. Using interpolate_propertybrings the thought process away from the immediate goal of looking at something.

The idea here is to use something that already exists in Godot (Tween) and enhance an expressive, high-level function (look_at) for more 'natural' output. The ideal is to provide a high-level, declarative API that keeps people as close to their thoughts and goals.

groud commented 6 years ago

You have a tween node to do that. That's the purpose of this node, so there's no reason to add this possibility in a per-function fashion

brylie commented 6 years ago

It just seems that the look_at method is fairly expressive, but just slightly unnatural in it's immediacy. Adding a tween argument might keep the expressive quality of look_at while enhancing it with a natural transition animation. I.e. we would just be using common building blocks to reach a bit higher abstraction/expressiveness.

groud commented 6 years ago

No, in any case that's impossible to code without dirty hacks into the engine code. I have no clue why you would belive that look_at() is less immediate that any other function in the engine. I mean, even set_position could be accept that kind if argument too, but that's not the point of such function. Tweening can be made with yield functions or Tween nodes, no need to add this everywhere in the code. And adding this only for look_at would be a confusing inconsistency

brylie commented 6 years ago

Gilles, consider taking some time for reflection before immediately dismissing an idea.

brylie commented 6 years ago

It might be constructive to approach this discussion from a why and how, before we get to "can't".

brylie commented 6 years ago

I have no clue why you would belive that look_at() is less immediate that any other function in the engine

The issue description mentions that the look_at is too immediate.

even set_position could be accept that kind if argument too

That seems like a reasonable idea too.

Tweening can be made with yield functions or Tween nodes

I have added a rationale section to the description, explaining about the importance of high-level tools for thought (with a link to Bret Victor's talk: The Future of Programming).

And adding this only for look_at would be a confusing inconsistency

Good point. We agree that consistency is important in an API.

groud commented 6 years ago

I already told you this is not how the engine works. The animation of properties is not meant to be done in a per-function basis. That's a design decision that was made before and there's no reason to change this for a single function.

Anyway, I'll yet other contributors tell what they think about this proposal.

ghost commented 6 years ago

I'm against this proposal.

Godot would be internally consistent with this added feature

How exactly? There are no functions in the API that I know of that accepts this tween argument. This is as consistent as it gets.

We have Tween node that can do that for all properties. Why is look_at so special that it needs its own tweening argument?

Also, tweening is not yes or no. If I want to Tween, I would like to be able to tune the parameters until it looks good. I don't think look_at(position, tween_duration, tween_transition, tween_easing) is okay.

I can say this for all properties:

If you don't like interpolate_property, just use _process(delta). It is called every frame and you can make it however smooth you like.

brylie commented 6 years ago

I'm against this proposal.

One of the problems we have in conversations like this is that we cast it in binary terms: 'for' or 'against', 'yes' or 'no'. There may be some degrees of grey here that are worth exploring.

At the very least, I am willing to use the approaches people have recommended to make 'looking around' feel more smooth and natural.

How exactly? There are no functions in the API that I know of that accepts this tween argument. This is as consistent as it gets.

Well, perhaps my words are not 100% accurate, as words tend to have inherent ambiguity. What I meant by 'internally consistent' was that Godot would build higher level abstractions on lower level components. I have added quotes and a clarification to the description.

Why is look_at so special that it needs its own tweening argument?

I am new to Godot, so have not used many other APIs. look_at was just one of the first methods I have used, and seems natural that looking at something would have some smoothness. E.g. our neck doesn't instantly jerk when we turn to look at something.

Also, tweening is not yes or no. If I want to Tween, I would like to be able to tune the parameters until it looks good. I don't think look_at(position, tween_duration, tween_transition, tween_easing) is okay.

Could the tween argument perhaps take a Tween node? e.g. look_at might get the Tween node, and then abstract the following from the user:

tween.interpolate_property(get_node(target), "transform/rotation", 30), 90, 1, Tween.TRANS_LINEAR, Tween.EASE_IN_OUT)
groud commented 6 years ago

I think what @Noshyaar wants to point out is that any property of the engine might need to be interpolated at some point. So to have a consistent way of dealing with tweening in general, we could, as you propose, add a "tween" argument to any function of the engine that sets a property. However, that would need a complete refactor of the engine, and would completely bloat the API while the current way of dealing with tweening works fine.

I am new to Godot, so have not used many other APIs. look_at was just one of the first methods I have used, and seems natural that looking at something would have some smoothness. E.g. our neck doesn't instantly jerk when we turn to look at something.

That's where you're wrong, smoothing is usually never handled directly by the setters themselves, typically besause that would bloat the API. That's just a wrong assumption.

akien-mga commented 6 years ago

I'm against this proposal.

One of the problems we have in conversations like this is that we cast it in binary terms: 'for' or 'against', 'yes' or 'no'. There may be some degrees of grey here that are worth exploring.

Well, more nuanced words could be used, but in the end it's the same: contributors with an intimate knowledge of Godot's architecture and philosophy are stating that this proposal doesn't match their (but given their experience and the arguments they raised, also Godot's) design expectations.

I am new to Godot, so have not used many other APIs. look_at was just one of the first methods I have used, and seems natural that looking at something would have some smoothness. E.g. our neck doesn't instantly jerk when we turn to look at something.

As a new Godot user, you're probably mislead by the name look_at. This method strictly means: set the transform so that this node faces this direction. That's all. It doesn't imply that a physical object in your game with at least 3 degrees of freedom (the 3 rotation ones) will gradually change their yaw, pitch and roll to face the given direction. It's instantaneous, like every other method in the Godot API.

Godot does not decide your game's logic for you, it gives you the tools to implement it yourself. look_at is one such tool, Tween is another. As others stated, if we start adding logic-specific parameters to purely functional APIs like look_at, we'll need to add them to set_position, set_rotation, and anything that might need to be interpolated in a given game.

And it's not possible to do this in a way that would match all game genres that can be done with Godot. That's why there's a whole Tween class full of options to fine tune this for your game.

It's your job as the game programmer to define your own look_at or turn_neck_towards method using the instantaneous methods that Godot provides, to make it "feel right" in the context of your game. If it's a human turning their head, you'd likely want to use tweening with ease-in and ease-out to simulate starting a rotation with caution in the first milleseconds, then increase speed quickly and then decrease as a PID controller would do to finish at the proper location. And you might even want to add some error margin because it's a human after all.

ghost commented 6 years ago

look_at is nothing more than rotate an object so that it points toward a location. It has no way of knowing the physical restrictions like turning rate limit.

Edit: also apologize for using the word "against", but it conveyed what I meant perfectly. Usually I see possibilities in feature requests. Unfortunately this is not one of them. Even something close to what you propose (InterpolatedCamera) is considered a "forgot to remove" feature by the core developer.

eon-s commented 6 years ago

Look at can be tweened with:

tween_node.interpolate_property(node_to_rotate, "transform",
  transform,
  transform.looking_at(target.translation,Vector3(0,1,0)), #if up
  time,trans_type,ease_type
)

or

tween_node.interpolate_property(node_to_rotate, "rotation",
  rotation,
  transform.looking_at(target.translation,Vector3(0,1,0)).basis.y, #if up
  time,trans_type,ease_type
)

Depending the needs on Spatial nodes.

For Node2D is more simple, just:

tween_node.interpolate_property(node_to_rotate, "rotation",
  rotation,  
  position.angle_to_point(target_node.position),
  time,trans_type,ease_type
)
ghost commented 6 years ago

Giving this some more thoughts, I think it's quite challenging. look_at can't be inverted. There's no is_currently_looking_at because the number of points is infinite. Suppose that I want to interpolate look_at from a point in blue ray to a point in green ray, I sample three points from each, which give the same look_at result. There are nine paths that are valid interpolations.

image

I estimate the mid point of those paths, and as you can see, they're not the same point, or even give the same look_at result. So which one is valid when we want to look_at halfway?

akien-mga commented 6 years ago

I estimate the mid point of those paths, and as you can see, they're not the same point, or even give the same look_at result. So which one is valid when we want to look_at halfway?

look_at only cares about the direction, and thus the angle, so you should interpolate on an arc of a circle centered on the looker.

ghost commented 6 years ago

Well, the point is (sorry), since look_at takes Vector2 (or Vector3 in 3D), both start and end points must be known to interpolate properly. Unless the start point is kept somewhere, there's no way to know as it can't be inferred from the current transformation state. So I'm not sure how tweening look_at would even work without: keeping the start point in a variable, or tweening rotation instead.

Zylann commented 6 years ago

Yep, I see two ways of doing it quite simply: either tween the target's position, or tween a quaternion. Both behave a little differently though in some cases. The latter seems more correct and would be a piece of cake with a tween going to Quat.look_rotation(), however is a bit harder to setup because the API doesn't expose rotation as quaternions, so you have to write intermediary code to use this representation (at least, from what comes to my mind right now^^). But depending on the effect you want to achieve, you could want quaternion or target interpolation, they are both valid solutions (and is one more reason why we would hardly add built-in tweening in look_at)

brylie commented 6 years ago

tweening rotation instead.

Ah, that is how I thought tweening was working in look_at. E.g. take the current rotation in 2D/3D, get the angle to the second object, update the rotation to face the second object (which might be tween-able).

PLyczkowski commented 5 years ago

Such a long winded discussion for such a simple thing. Just have an invisible dummy object with same position as main object, run look_at on the dummy_object, and then Tween main object rotation to the rotation of the dummy object. Full control this way.

hodgej commented 5 years ago

I found a even more simple way to do this:

In godot 3.1, you can simply use a timer node.

Set the timer node to have a delay, like 0.01 seconds, set it on autostart, not one_shot! then use this code in whatever is using look_at().

func _process(): #Must be a process or some kind of loop to make it repeat! if get_node("look_at delay").time_left < 0.001: look_at(player.position)

Hopefully this solves the problem! :)

EDIT: Just realized this thread is a year old, sorry. lol.

Calinou commented 5 years ago

@hodgej This will likely result in a "stuttery" appearance if the timer duration is longer than 0.01 seconds.

hodgej commented 5 years ago

Yes, but at least it provides some sort of delay.

GammaGames commented 5 years ago

I was able to "tween" rotation in 3d by using look_at and keeping track of the previous position:

export var turning_speed = 10
var target = null
var last_origin = null

func _process(delta):
    if target:
        var offset = (target.global_transform.origin - last_origin) * turning_speed * delta
        last_origin = last_origin + offset
        look_at(last_origin, Vector3.FORWARD)

func hold(by):
    target = by
    last_origin = target.global_transform.origin

func drop():
    target = null
hodgej commented 5 years ago

So this would not work in 2D?

Calinou commented 4 years ago

There doesn't seem to be much support for this feature, so this will probably have to be rethought if someone wants to open a proposal.


Feature and improvement proposals for the Godot Engine are now being discussed and reviewed in a dedicated Godot Improvement Proposals (GIP) (godotengine/godot-proposals) issue tracker. The GIP tracker has a detailed issue template designed so that proposals include all the relevant information to start a productive discussion and help the community assess the validity of the proposal for the engine.

The main (godotengine/godot) tracker is now solely dedicated to bug reports and Pull Requests, enabling contributors to have a better focus on bug fixing work. Therefore, we are now closing all older feature proposals on the main issue tracker.

If you are interested in this feature proposal, please open a new proposal on the GIP tracker following the given issue template (after checking that it doesn't exist already). Be sure to reference this closed issue if it includes any relevant discussion (which you are also encouraged to summarize in the new proposal). Thanks in advance!