godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.07k stars 69 forks source link

NavigationAgent3D emit velocity_computed signal even if not using collision avoidance #5013

Open maximkulkin opened 1 year ago

maximkulkin commented 1 year ago

Describe the project you are working on

A 3D RTS-like game

Describe the problem or limitation you are having in your project

Current tutorials on using NavigationAgent3D recommend doing NavigationAgent3D.set_velocity(), then waiting for velocity_computed signal, passing "safe_velocity", which is then should be passed to RigidDynamicBody3D.set_linear_velocity():

extends RigidDynamicBody3D

const SPEED = 10

@onready var agent: NavigationAgent3D = get_node("NavigationAgent3D")

func move_to(target_position: Vector3) -> void:
    agent.set_target_location(target_position)

func _physics_process(_delta: float) -> void:
    if agent.is_navigation_finished():
        return

    var direction: Vector3 = (agent.get_next_location() - global_position).normalized()
    agent.set_velocity(direction * SPEED)

func _on_navigation_agent_3d_velocity_computed(safe_velocity: Vector3) -> void:
    look_at(global_position + safe_velocity)
    set_linear_velocity(safe_velocity)

The problem with this setup that it only works if collision avoidance is enabled, otherwise signal is not emitted and the only hint is the description of velocity_computed signal saying "Notifies when the collision avoidance velocity is calculated".

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

Either rename signal to something that explicitly says it's for collision avoidance ("avoidance_velocity_calculated") or change it to be called even if avoidance is not enabled merely passing velocity that was set with set_velocity(). The later will allow using same setup regardless of whether collision avoidance is on or off and would allow easily switch back and forth.

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

NavigationAgent3D.set_velocity() will immediately emit "velocity_computed" signal in case avoidance_enabled == false. Otherwise it will work as currently is.

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

If case collision avoidance is not used, one can directly call RigidDynamicBody3D.set_linear_velocity() instead of doing NavigationAgent3D.set_velocity()

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

This is a small quality of life improvement making core modules more easy to use and beginner friendly.

Calinou commented 1 year ago

This appears to be done by design: https://github.com/godotengine/godot/issues/61378#issuecomment-1136332600

cc @smix8

smix8 commented 1 year ago

All the tutorials with just the signal connection are outdated by now or assume that users would always want to use avoidance in every situation which is not the case for both performance and quality reasons.

The actor movement script should have a branch that checks if avoidance is enabled and process the movement accordingly e.g. something like this:

func _physics_process(_delta: float) -> void:

    # calculate your velocity for the actor

    if agent.avoidance_enabled:
        # feed the velocity to the NavigationServer for avoidance calculation and signal callback
        agent.set_velocity(calculated_velocity)
    else:
        # do normal movement code

func _on_navigation_agent_3d_velocity_computed(safe_velocity: Vector3) -> void:
    if agent.avoidance_enabled:
        # do movement code with avoidance safe velocity
maximkulkin commented 1 year ago

Why actor script should have a branch instead of that branch being inside NavigationAgent3D? It's like "you feed your desired velocity to NavigationAgent3D and it gives you what velocity you should use". In case of not using avoidance, it will just give you your velocity back. Why it should be harder than that?

Also, why in your example velocity computed callback checks avoidance_enabled again?