godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.83k stars 21.14k forks source link

Autotile editor collisions dont work sometimes - using raycast2d #13319

Closed blurymind closed 6 years ago

blurymind commented 6 years ago

Probably due to not having a fill. Related to: https://github.com/godotengine/godot/issues/13287 https://github.com/godotengine/godot/issues/10110

The collisions are pretty flaky- especially for grid type movement. See gif: flakycolisions

@MarianoGnu @reduz suggestions?

How do we do a solid grid movement with these new autotiles- I cant get collisions to work properly

On a related note- The current collisions are a problem in another usecase- when you want to implement pathfinding movement for the player that checks if the user has clicked on an empty tile or not before placing the pathfind marker. With the current autotile collisions its not possible to check,unless the user clicks the tiny pixel outline of the collision square

MarianoGnu commented 6 years ago

raycast fails when you change the direction and enter an invalid tile at the same time:

  1. looking left, raycast doesn't detect collision
  2. you pressed down, you rotated the raycast, or changed the cast direction and checked if it was colliding, but it will return the last result instead (when you where looking left) this causes to return false
  3. you got inside tile, then you are surounded by shapes, this means no matter where you look at, you raycast.is_colliding will return true

SOLUTION: instead of changing the raycast have 4 raycast and check the one you need

blurymind commented 6 years ago

@MarianoGnu I cast and check raycast prior to moving by increment- sometimes raycast doesnt register the collision and returns false. Even with 4 raycasts- sometimes I get a false when there is a wall.

Please consider that this type of collisions causes other problems apart of raycast not always registering. Intersect_point doesnt work with them either. both @damarindra and @eon-s seem to have problems with it https://github.com/godotengine/godot/issues/13047

@damarindra seems to also get stuck inside a tile with his platformer - is that the same issue as me?

MarianoGnu commented 6 years ago

Hum, do you mind sharing a minimal example?

blurymind commented 6 years ago

@MarianoGnu give me some time to cleanup the code and minimize it as an example.I will try some workarounds

DoctorAlpaca commented 6 years ago

move_and_slide can also push through the autotile colliders (will provide an example tomorrow).

DoctorAlpaca commented 6 years ago

Alright, the problem in my specific case seems to be that move_and_slide doesn't recognize the tiles as ground, making my gravity routine accelerate the player downwards until the speed is high enough that he phases through the collider.

bug Minimal example: fallthrough.zip

blurymind commented 6 years ago

I think that because the collision is so damn thin - it's really easy to miss the pixel and get stuck inside the shape. Is there a way to check if a tile contains a collision shape instead? It might help my case

MarianoGnu commented 6 years ago

@DoctorAlpaca just in case, your script runs on _process or _physic_process ? this getting really buggy, but it's more related to segment collision shape than Autotiles themselves

blurymind commented 6 years ago

@MarianoGnu I am also attaching my test scene collisionBug.zip

It has a raycast2d for each direction- as you suggested, however the player still can get stuck inside a tile easily.

Please note that the same code just works without any of this getting stuck in a tile problem - when the collision shape is not a segment collision shape

Unfortunately that is the collision shape type that autotiles are using

reduz commented 6 years ago

seems there may be a bug in segment collision shape?

On Tue, Nov 28, 2017 at 10:25 AM, Todor Imreorov notifications@github.com wrote:

@MarianoGnu https://github.com/marianognu I am also attaching my test scene collisionBug.zip https://github.com/godotengine/godot/files/1510011/collisionBug.zip

Please note that the same code just works without any of this getting stuck in a tile problem - when the collision shape is not a segment collision shape

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/godotengine/godot/issues/13319#issuecomment-347522404, or mute the thread https://github.com/notifications/unsubscribe-auth/AF-Z2zp8fe8R2RHyalexE8bLUaxHG4l9ks5s7AnMgaJpZM4Qq43l .

blurymind commented 6 years ago

@reduz it seems that way Segment collision shapes have a number of problems:

  1. Character may get stuck in them when using move_and_slide (@DoctorAlpaca)
  2. They always return empty when using world.intersect_point(tilePositionHere) - so they can not be used for gdquest's video tutorials on grid movement that rely on that method to check if a tile has a collision
  3. With top down grid movement using raycast2d instead,you can still get the player stuck inside a tile- if you try long enough (see my attached example in previous post)

Because autotile uses this collision shape type, autotiles are affected by all of the problems above

eon-s commented 6 years ago

@reduz yes, I think #10110 is affecting here.

MarianoGnu commented 6 years ago

@MarianoGnu I am also attaching my test scene collisionBug.zip

@blurymind i have tested this and it doesn't look like the issue is caused by the raycast, it's more caused by script execution order, but i'm not sure since i don't fully understand it... (i have the impression that moving direction can be changed in middle of a movement, when the raycast is not touching the shape yet) i may be mistaken thought...

blurymind commented 6 years ago

@MarianoGnu the way the script works is that the direction check happens on input and you have 4 raycasts. As noted the collision problem did not happen when using the same script with world_intersect - so i must insist that the problem is with the collision shapes again. Also you can get the player stuck inside a tile even if you dont try to change direction in the middle of movement. You cant really change direction until a grid movement has been completed. If you could -it would be obvious as the player will no longer be snapped to the grid. I wrote this script using a tutorial by gdquest

I can add another tileset that is not using autotiles if you want to. It would help testing

On 29 Nov 2017 00:23, "Mariano Javier Suligoy" notifications@github.com wrote:

@MarianoGnu https://github.com/marianognu I am also attaching my test scene collisionBug.zip

@blurymind https://github.com/blurymind i have tested this and it doesn't look like the issue is caused by the raycast, it's more caused by script execution order, but i'm not sure since i don't fully understand it... (i have the impression that moving direction can be changed in middle of a movement, when the raycast is not touching the shape yet) i may be mistaken thought...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/godotengine/godot/issues/13319#issuecomment-347710300, or mute the thread https://github.com/notifications/unsubscribe-auth/AGMbVVFK0guO74c3py1G8XK35ef7Hp9Kks5s7KQYgaJpZM4Qq43l .

blurymind commented 6 years ago

@MarianoGnu ok my case might be a problem with using raycast2d to do the collision check - I get stuck in filled shapes too. How come does this not happen when I am checking with world.intersect_point instead of raycast2d?

damarindra commented 6 years ago

This issue becomes more complicated. For me, I can't reproduce this using raycast and also tested using your demo is fine too (I dunno how to reproduce it, I just move towards collision and its fine). This problem is out of autotiles itself (only because autotiles using segment collision shape), maybe open another issue with specific problem like "raycast can't detect segment collision"?

I know the problem between autotiles collision between kinematicbody, but I search what the actual problem is, and then found out, because of the collider using concave. Then do simple project with specific problem, and gotcha, the problem is described here https://github.com/godotengine/godot/issues/13047

blurymind commented 6 years ago

I dont think I can use autotiles to do grid based movemen at all, because autotiles use collision shape type that does not support world.intersect_point - always returning empty. Replacing the exact same code I had with raycast2d instead of intersect_point for doing the checking results in getting stuck in tiles. @damarindra if you try long enough I guarantee you that you will get stuck

Here is the project using raycast2d: collisionBug.zip You can get the player stuck

Here is the exact same grid movement code, using world intersect and filled collision shapes instead of raycast2d and concave collision shapes: collision-working.zip The execution order is the same in both, the only difference is the input line checks raycast2d for collision in one and world_intersect in the other

I opened a bug about world_intersect not working with concave shapes https://github.com/godotengine/godot/issues/13287 , but @MarianoGnu closed it (is it intended design to not work with concave shapes?). So as far as getting autotile concave collisions to work in a grid based movement- this is a dead end for me. If its not the execution order, then there must be a problem in raycast2d or the way it is being used in my code?

damarindra commented 6 years ago

Yes, as I said, this issue is more related to the collision itself, instead of changing the collision type of autotiles (but it depends on devs too, if @MarianoGnu want to change it, I'm also happy tbh), I think better to fix the collision itself (in this case segment based). IMHO, opening new issue with the "real" problem is better.

Also, I give it try more time, and yes, it can pass the collision (I've tested with my simple project too). So that means, sometimes raycast can't detect segment collision.

blurymind commented 6 years ago

@damarindra yes you are right. The truth is that we uncovered a number of problems with the concave collision type that autotiles are using at the moment. In my example file raycast2d sometimes fails to detect concave collisions

@MarianoGnu noted in another thread that changing the collision type would break all tilesets done up until now. Would you be ok with that? I mean godot is in an alpha stage still anyway. If this needs to be done, wouldn't this stage be the best time to do it? Here is @MarianoGnu 's post:

related but not the same. yes, the cquse of the issue is because i used concave polygon shapes wich where easier to use, sadly if i change it now it will break alltilesets done up until now, i could prevent this but would meqn an extra work for just a bunch of tilesets. an opinion here? @reduz shouldn't segments work with tilemap on static mode? i heard from damar indra that this doesnt work well with kinematic bodies...

MarianoGnu commented 6 years ago

@blurymind segment shapes have no problem detecting the collision with raycasts (only problem is one way collision), this is because of execution order, in particular i have my doubts about yielding the idle frame, during that shielding the _physics_process will run again and probably change the values of the variables. Also you will hve to change the implementation of your movement anyways, because this might work to detect static objects but consider what would happen if 2 characters starts to move at the same position at the same time. That problem is considered and solved in the video i sent earlier, it's a pity that it's on spanish :/

blurymind commented 6 years ago

@MarianoGnu I added that yield earlier when trying to do it with a single raycast that is rotated and forgot to remove it. Does commenting it out fix this? I will try to see if I can fix the code. The yielding didnt seem to bring the collision problem when it was used in the version of the code with intersect_world though. That version has the exact same execution order- its just not using raycast2d + autotiles at all

I technically dont need the yield at all, just forgot it in there by mistake, after trying different things

blurymind commented 6 years ago

@MarianoGnu removing the yield seems to have fixed it, but I need to test more extensively at home just to be sure! The first time it didnt work because I was using one raycast and rotating it. The second time it didnt work because I forgot to rempve the yield from my previous tests.

But we still have two or more definite bugs that have to do with concave polygon shapes:

Would you be inclined to change autotiles to use the other collision type shape? What are the advantages of using concave polygon shapes? The filled shapes seem to be harder to introduce a bug in the game code- at least you wont get the getting stuck in a tile problem as easily.

Would you like me to rename this issue to reflect @damarindra 's issue?

MarianoGnu commented 6 years ago

i'll take care of point 1, but point 2 can (and should) be done eighter using navigation, AStar or TileMap.world_to_map() function, instead of intersect point imo

reduz commented 6 years ago

@MarianoGnu what's the status of this?

MarianoGnu commented 6 years ago

Haven't done anything, considering if swiching to concave shapes is worth... It depends on if the segment shape's bugs are fixed or not. I have never messed up with physics yet, i will when i have time if you want me to

DoctorAlpaca commented 6 years ago

Considering that filled collision shapes seem to be the default for most use cases in Godot, I'd like to see it used here as well; principle of least surprise and all.

blurymind commented 6 years ago

I also prefer the filled shape type (concave). Are there any advantages in using segment shapes here? it looks like they only have disadvantages for this use case, but might be useful for other use cases (bigger collision shapes perhaps?) But for small tiles - it would be hard to come up with a reason to justify getting the player stuck inside a tile as a part of the game design

Do concave shapes also support directional collisions?

MarianoGnu commented 6 years ago

Maybe not usefull for tiles, but supose you have a house and want to make collisions on the walls, using a concave shape lets you to set all walls in a single polygon and bodyes will be trapped inside that shape and cannot scape. Main reason to do this was because it was easier to do since i was having problem with invalid polygons when using Convex Shapes and i'm not sure how to manage this, maybe @reduz could give me a hint on how to determinate if a polygon shape is valid and what to do if it's not?

eon-s commented 6 years ago

The method used by CollisionPolygon2D to triangulate polygons do not work here? May worth to fix that, exposing a function to triangulate polygons on the core perhaps.

CyanBlob commented 6 years ago

I also have a problem with the autotile editor collisions, but without using raycasts. My player is a KinematicBody2D and has a capsule-shaped collider, and the tilemap colliders are all set to fill the entire tile, but my player "catches" on the egdes between the tiles (from what I can tell) when using move_and_collde(). One of my attacks (also a KinematicBody2D, but with a circle collider and still using move_and_collide()) doesn't detect collisions at all at these points. ezgif com-optimize 1 The tornado attack should disappear as soon as it hits the wall, but it doesn't always do that. The player should move cleanly against the wall, but his movement is very jagged. Here are my colliders on the tilemap: tilemapcolliders

CyanBlob commented 6 years ago

@MarianoGnu Have you had a chance to look into this at all? I could look into it myself with some guidance if you're too busy.

blurymind commented 6 years ago

@damarindra just got a fix merged - Collision now using ConvexPolygonShape2D https://github.com/godotengine/godot/pull/15117 everyone please test, so i can close this

CyanBlob commented 6 years ago

I can confirm that the PR fixed my issue, as long as the collision polygons are defined properly (add each point in a clockwise manner). Thanks a lot, @damarindra!

blurymind commented 6 years ago

@DoctorAlpaca , @eon-s has this fixed it for you too?

MarianoGnu commented 6 years ago

I'm Closing this issue since new shapes will always be detected by raycasts, Open a new issue referensing this one if you find any corner case.

eon-s commented 6 years ago

@blurymind I have not tested the fix but if is using convex shapes it should work, the issue with concave and one side shapes still needs to be solved (already are other issues about them).

MarianoGnu commented 6 years ago

btw @blurymind, closed the other issue, you you should test intersect_point since it should work now

blurymind commented 6 years ago

thank you @MarianoGnu and especially @damarindra :)