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

NavigationServer API is missing some getter functions #84495

Closed
smix8 opened this issue Nov 6, 2023 · 3 comments · Fixed by #84729
Closed

NavigationServer API is missing some getter functions #84495

smix8 opened this issue Nov 6, 2023 · 3 comments · Fixed by #84729

Comments

@smix8
Copy link
Contributor

smix8 commented Nov 6, 2023

Godot version

5ee9831

System information

Windows 10

Issue description

The NavigationServer API has many setter functions for navigation object properties but is often missing the getter function for the property.

For the majority of properties there is no real reason why they should have no getters, they were just never added.

E.g. the setter agent_set_avoidance_layers() has no agent_get_avoidance_layers() getter function.

Without getters some script uses are made more annoying and difficult as users need to track the properties externally somewhere and keep them updated with the server.

Steps to reproduce

TODO List:

  1. Search for missing getter functions on the NavigationServer2D and NavigationServer3D API.
  • Write those missing functions down and post it for a check to avoid adding functions later that are not needed.
  1. Files servers/navigation_server_2d.h and servers/navigation_server_3d.h.
  • Add the missing virtual functions following the schema of other existing getter functions.
  1. Files servers/navigation_server_2d.cpp and servers/navigation_server_3d.cpp
  • Add the bindings for the added getters inside the _bind_methods() function.
  1. Files modules/navigation/godot_navigation_server_2d.h and modules/navigation/godot_navigation_server.h
  • Add the missing virtual function overrides.
  1. Files modules/navigation/godot_navigation_server_2d.cpp and modules/navigation/godot_navigation_server.cpp
  • Add the missing function bodies.
    Note since the NavigationServer2D will have to call the NavigationServer3D some values need to be converted back and forth between 2D and 3D. Do not write manual conversions, use the existing static functions available in that file.
    For the NavigationServer2D it is ok to NOT use the FORWARD_XYZ macros and instead write normal functions that call the NavigationServer3D. We want to get rid of those NavigationServer2D macro uses long-term anyway.
  1. Files doc/classes/NavigationServer2D.xml and doc/classes/NavigationServer3D.xml
  • Add the class documentation for all added getters.

Minimal reproduction project

N/A

@nickyfoo
Copy link
Contributor

nickyfoo commented Nov 6, 2023

Hello, I'd like to work on this!

@nickyfoo
Copy link
Contributor

nickyfoo commented Nov 7, 2023

I've looked around the public class API for NavigationServer2D / NavigationServer3D, and these are the ones that are missing getters

They seem to be generally the same set of methods, except region_set_navigation_polygon (2D)/ region_set_navigation_mesh (3D)
and the inclusion of agent/obstacle_set_height for 3D

Do let me know if there are any here that shouldn't have a getter

navigation_server_2d.h
region_set_transform
region_set_navigation_polygon
agent_set_neighbor_distance
agent_set_max_neighbors
agent_set_time_horizon_agents
agent_set_time_horizon_obstacles
agent_set_radius
agent_set_max_speed
agent_set_velocity_forced
agent_set_velocity
agent_set_position
agent_set_avoidance_callback
agent_set_avoidance_layers
agent_set_avoidance_mask
agent_set_avoidance_priority
obstacle_set_radius
obstacle_set_velocity
obstacle_set_position
obstacle_set_vertices
obstacle_set_avoidance_layers

navigation_server_3d.h
region_set_transform
region_set_navigation_mesh
agent_set_neighbor_distance
agent_set_max_neighbors
agent_set_time_horizon_agents
agent_set_time_horizon_obstacles
agent_set_radius
agent_set_height
agent_set_max_speed
agent_set_velocity_forced
agent_set_velocity
agent_set_position
agent_set_avoidance_callback
agent_set_avoidance_layers
agent_set_avoidance_mask
agent_set_avoidance_priority
obstacle_set_radius
obstacle_set_height
obstacle_set_velocity
obstacle_set_position
obstacle_set_vertices
obstacle_set_avoidance_layers

@smix8
Copy link
Contributor Author

smix8 commented Nov 7, 2023

region_set_navigation_polygon and region_set_navigation_mesh need no getter as both will get deprecated soon with the navmesh instances.

agent_set_velocity_forced does not need a getter, it is an one-update-only override.

agent_set_avoidance_callback should not have a getter to get the Callback object directly but can receive agent_has_avoidance_callback function to check if there is a callback already.

Everything else looks fine to receive a getter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants