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

Collider returns an Object. no access to .Name #28318

Open
Tracked by #45334
nathanjwtx opened this issue Apr 23, 2019 · 12 comments
Open
Tracked by #45334

Collider returns an Object. no access to .Name #28318

nathanjwtx opened this issue Apr 23, 2019 · 12 comments

Comments

@nathanjwtx
Copy link

Godot version:
3.1 Mono
Windows 10, i5, GTX1050

Trying to convert the following into C#:
var collision = get_slide_collision(idx) if collision.collider.name == 'Danger':

KinematicCollision2D collision = GetSlideCollision(i);

Doing collison.Collider doesn't expose the name property.

@mysticfall
Copy link
Contributor

mysticfall commented Apr 23, 2019

I had a discussion with @nathanjwtx on Discord about this issue and to clarify, it is about the binding for KinematicCollision2D.Collider having Object as its return type, which better be changed for something more specific for convenience.

@neikeq
Copy link
Contributor

neikeq commented Apr 23, 2019

This property is defined in PhysicsBody and PhysicsBody2D. Looking at the source, it seems the type can't be nothing other than CollisionObject and CollisionObject2D. @reduz, can we update the signature of these methods to reflect this?

@akien-mga
Copy link
Member

I think the collider can be TileMap or GridMap too, no? That's why #24739 and related issues are problematic.

@Reneator
Copy link

I think the collider can be TileMap or GridMap too, no? That's why #24739 and related issues are problematic.

Wouldn't then just clarifying the different return-types in the documentation improve these misconceptions?

@neikeq
Copy link
Contributor

neikeq commented Apr 23, 2019

@akien-mga At least for the collider property in PhysicsBody and PhysicsBody2D I don't see anything other than CollisionObject and CollisionObject2D being set.

@akien-mga
Copy link
Member

@neikeq What property do you mean? I don't see any collider property in PhysicsBody2D.

But I can confirm that KinematicCollision2D.collider can be a TileMap:

extends KinematicBody2D
var vel = Vector2(0, 10)
func _physics_process(delta):
	var col = move_and_collide(vel)
	if col:
		print(col.collider)  # prints [TileMap:1162] on collision

@mysticfall
Copy link
Contributor

Maybe we can introduce a common interface like ICollidable?

@neikeq
Copy link
Contributor

neikeq commented Apr 24, 2019

@akien-mga My mistake, the classes are KinematicCollision and KinematicCollision2D.

@akien-mga
Copy link
Member

akien-mga commented Apr 25, 2019

My mistake, the classes are KinematicCollision and KinematicCollision2D.

@neikeq So yeah, I can confirm that collider can be something different from a CollisionObject2D as per

Object *KinematicCollision2D::get_collider() const {
if (collision.collider) {
return ObjectDB::get_instance(collision.collider);
}
return NULL;
}
ObjectID KinematicCollision2D::get_collider_id() const {
return collision.collider;
}

I guess you thought it's a CollisionObject2D due to the cast for collider_shape?

Object *KinematicCollision2D::get_collider_shape() const {
Object *collider = get_collider();
if (collider) {
CollisionObject2D *obj2d = Object::cast_to<CollisionObject2D>(collider);
if (obj2d) {
uint32_t ownerid = obj2d->shape_find_owner(collision.collider_shape);
return obj2d->shape_owner_get_owner(ownerid);
}
}
return NULL;
}

I can confirm that in the case of TileMap, it can't retrieve the shape:

extends KinematicBody2D
var vel = Vector2(0, 10)
func _physics_process(delta):
	var col = move_and_collide(vel)
	if col:
		print(col.collider)  # prints [TileMap:1162]
		print(col.collider_shape)  # prints [Object:null]

We could probably make collider return Node2D or Spatial instead of Object, which should solve this issue (not the other ones regarding expecting a CollisionObject2D and getting a TileMap, but that's a broader issue).

@neikeq
Copy link
Contributor

neikeq commented Apr 25, 2019

I followed all the code paths that assigned collision.collider until I found something typed. I must have missed something.

Edit: Just checked again. Yes, I missed some usages... 😑

@eka
Copy link

eka commented Sep 24, 2020

I bumped into this when trying to translate some GDScript code to C#.
I there an alternative to get the Name of the collider object right now?

image

Update:

I was able to circumvent this, casting to Node as

if (((Node)collision.Collider).Name == "Name")

@Zylann
Copy link
Contributor

Zylann commented Oct 26, 2021

You should be able to cast Collider to Node, or Spatial or Node2D, since that's the lowest common denominator in classes used for this field, at least in core Godot.
I bind this field to my HTerrain and VoxelTerrain plugin nodes for collisions with them as well. Although, it should be said that the API that allows binding an object to a collider does not require any particular constraint on the type of his object, so in theory, following the current API, Collider could be of any Object type, even if so far these types happen to be nodes: https://docs.godotengine.org/en/stable/classes/class_physicsserver.html#class-physicsserver-method-body-attach-object-instance-id the reason being that the server's API may not know anything about scene tree stuff because it's abstract and should remain that way, so it's all a convention. You could imagine this object being a Tile non-node class for example. Objects don't necessarily have a name so forcing an interface is not possible.

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

No branches or pull requests

9 participants