Godot: Return RaycastHit instead of Dictionary when using intersect_ray()

Created on 12 Dec 2018  路  2Comments  路  Source: godotengine/godot

Godot version:

37e198c32006d18c9443bf65963ba8182b7d76a6

OS/device including version:

Arch Linux

Issue description:

Physics2DDirectSpaceState.intersect_ray() returns a dictionary with various entries related to how the ray went:

{
   position: Vector2 # point in world space for collision
   normal: Vector2 # normal in world space for collision
   collider: Object # Object collided or null (if unassociated)
   collider_id: ObjectID # Object it collided against
   rid: RID # RID it collided against
   shape: int # shape index of collider
   metadata: Variant() # metadata of collider
}

While this is functional, I think it would be better if a distinct type were returned, as autocomplete could then be used with the result. It seems that this is one of the only functions in the engine that uses a dictionary instead of a type; I'm not sure if there's an explicit reason for this or not.

This is also the case for the 3D variation, PhysicsDirectSpaceState

archived breaks compat enhancement physics

Most helpful comment

Ran a quick check for methods which have return type of Dictionary or Array of Dictionaries, here are some more that might need changing:

  • Intersection: Physics{,2D}DirectSpaceState

    • {collider, metadata (only 2d), shape, collider_id, rid}: intersect_point, intersect_shape

    • {..., normal, position}: intersect_ray

    • {..., linear_velocity}: get_rest_info (has point instead of position)

  • Dates: OS::get_{date,time,datetime,datetime_from_unix_time,time_zone_info}
  • Tuples of a few values:

    • Currently dictionary: Geometry::make_atlas, TreeItem::get_range_config

    • Currently array: @GDScript::rand_seed, Physics{,2D}DirectSpaceState::cast_motion, Mesh::*arrays*, Animation::transform_track_interpolate

  • Property info: Object::{,_}get_property_list, ClassDB::class_get_property_list, VisualServer::shader_get_param_list, EditorImportPlugin::get_import_options, AnimationNode::get_parameter_list
  • Method Info: Object::get_method_list, ClassDB::class_get_method_list, ClassDB::class_get_signal_list, ClassDB::class_get_signal, Object::get_signal_list
  • Misc:

    • Licensing: Engine::get_*_info

    • Used only once: GraphEdit::get_connection_list, ARVRServer::get_interfaces, VisualServer::texture_debug_usage, Object::get_incoming_connections, Object::get_signal_connection_list

In light of that, I think it would be nice to have the following changes:

  • Intersection{,2D} classes, with the following properties:

    • collider, collider_id, shape, rid: as is now

    • metadata: from PhysicsServer{,2D}::shape_get_data, both 2d and 3d

    • position: for all functions, except intersect_shape (unless it is possible without perf. problems)

    • normal: Vector2(0, 0) when unknown (as is with intersect_point)

    • linear_velocity: Vector2(0, 0) when unknown (as is with all except get_rest_info)

  • Date class, which replaces the functions in the OS class: #6463
  • PropertyInfo and MethodInfo as real classes. Not sure how doable this is though.
  • Some slight changes to Dictionary formats used only once, either by adding a small class or by refactoring those cases to not use Dictionaries:

    • Change Geometry::make_atlas to either return one thing or return an array, for consistency.

    • Change TreeItem::get_range_config to return an array, for consistency.

    • Change ARVRServer::get_interfaces (which currently returns {idx, name}) to ARVRServer::get_interface_names or make it return the actual interface objects as done by ::get_interface(idx).

All 2 comments

Ran a quick check for methods which have return type of Dictionary or Array of Dictionaries, here are some more that might need changing:

  • Intersection: Physics{,2D}DirectSpaceState

    • {collider, metadata (only 2d), shape, collider_id, rid}: intersect_point, intersect_shape

    • {..., normal, position}: intersect_ray

    • {..., linear_velocity}: get_rest_info (has point instead of position)

  • Dates: OS::get_{date,time,datetime,datetime_from_unix_time,time_zone_info}
  • Tuples of a few values:

    • Currently dictionary: Geometry::make_atlas, TreeItem::get_range_config

    • Currently array: @GDScript::rand_seed, Physics{,2D}DirectSpaceState::cast_motion, Mesh::*arrays*, Animation::transform_track_interpolate

  • Property info: Object::{,_}get_property_list, ClassDB::class_get_property_list, VisualServer::shader_get_param_list, EditorImportPlugin::get_import_options, AnimationNode::get_parameter_list
  • Method Info: Object::get_method_list, ClassDB::class_get_method_list, ClassDB::class_get_signal_list, ClassDB::class_get_signal, Object::get_signal_list
  • Misc:

    • Licensing: Engine::get_*_info

    • Used only once: GraphEdit::get_connection_list, ARVRServer::get_interfaces, VisualServer::texture_debug_usage, Object::get_incoming_connections, Object::get_signal_connection_list

In light of that, I think it would be nice to have the following changes:

  • Intersection{,2D} classes, with the following properties:

    • collider, collider_id, shape, rid: as is now

    • metadata: from PhysicsServer{,2D}::shape_get_data, both 2d and 3d

    • position: for all functions, except intersect_shape (unless it is possible without perf. problems)

    • normal: Vector2(0, 0) when unknown (as is with intersect_point)

    • linear_velocity: Vector2(0, 0) when unknown (as is with all except get_rest_info)

  • Date class, which replaces the functions in the OS class: #6463
  • PropertyInfo and MethodInfo as real classes. Not sure how doable this is though.
  • Some slight changes to Dictionary formats used only once, either by adding a small class or by refactoring those cases to not use Dictionaries:

    • Change Geometry::make_atlas to either return one thing or return an array, for consistency.

    • Change TreeItem::get_range_config to return an array, for consistency.

    • Change ARVRServer::get_interfaces (which currently returns {idx, name}) to ARVRServer::get_interface_names or make it return the actual interface objects as done by ::get_interface(idx).

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!

Was this page helpful?
0 / 5 - 0 ratings