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

move_and_slide() takes in velocity, while move_and_collide() takes in position offset #30029

Closed
Tracked by #45334 ...
EeroMutka opened this issue Jun 24, 2019 · 4 comments · Fixed by #53174
Closed
Tracked by #45334 ...

Comments

@EeroMutka
Copy link

EeroMutka commented Jun 24, 2019

Title sums it up, I just found the inconsistency annoying when using those functions. It might be problematic to change it due to backwards compatibility though

@EeroMutka EeroMutka changed the title move_and_slide takes in velocity, while move_and_collide takes in position offset move_and_slide() takes in velocity, while move_and_collide() takes in position offset Jun 24, 2019
@KoBeWi
Copy link
Member

KoBeWi commented Jun 24, 2019

Is this only about argument name? I don't see a problem. They do two different things, so it makes sense that they aren't named the same.

@akien-mga
Copy link
Member

I agree that it's confusing that the two move_and_* methods have a completely different API: move_and_slide expects velocity and returns the modified velocity, move_and_collide expects an offset and returns a KinematicCollision2D.

In Godot 2 there was only move, which behaved like move_and_collide. move_and_slide was then added, which uses _move internally (which is what move_and_collide binds to).

I find that the KinematicBody[,2D] move API is pretty cumbersome overall, and I hope we can refactor it for 4.0 to be more intuitive and convenient to use (especially regarding the many parameters passed to move_* methods). Currently it's quite inconsistent due to seeing several iterations of improvements while trying to preserve compatibility.

@nathanfranke
Copy link
Contributor

I think it should be move, take an optional parameter slide, and return a collision

@Calinou
Copy link
Member

Calinou commented Jul 9, 2020

@nathanfranke Boolean parameters aren't very readable when you skim over the code, so I'm not sure if it's a good idea.

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.

5 participants