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

Tilemap collisions not showing with 'visible collision shapes' in 3.3 #48631

Closed
chucklepie opened this issue May 11, 2021 · 20 comments
Closed

Tilemap collisions not showing with 'visible collision shapes' in 3.3 #48631

chucklepie opened this issue May 11, 2021 · 20 comments

Comments

@chucklepie
Copy link

chucklepie commented May 11, 2021

Godot version:
3.3 stable

OS/device including version:
Linux mint mate 19 64 bit

Issue description:
This is Godot 3.2.3 with debug/visible collisions showing all the tiles that have collision shapes, plus the player kinematic body spaceship hiding at the bottom right
image

This is 3.3. It has forgotten to include tilemap collision shapes. The only collision shape you see is the kinematic body.
image

It is the same project.

Steps to reproduce:

  1. create a tilemap
  2. add a tileset with collisions
  3. enable visible collisions
  4. profit

Minimal reproduction project:
See my comment below with code.

@jusw85
Copy link

jusw85 commented May 11, 2021

I cannot reproduce.

3.3, Arch Linux

The tile comes from a tileset with a single tile with collision

file

@chucklepie
Copy link
Author

chucklepie commented May 11, 2021

I'll add as new comment. I didn't before in case it is a known fault.

@SirLich
Copy link
Contributor

SirLich commented May 11, 2021

I am not experienced this bug, on 3.3. I am on windows.

You could create a demo project for the bug. I think it would be quite fast to setup and would allow others to try and repro.

@chucklepie
Copy link
Author

chucklepie commented May 11, 2021

I am not experienced this bug, on 3.3. I am on windows.

You could create a demo project for the bug. I think it would be quite fast to setup and would allow others to try and repro.

I'm doing it now. Be there in a few minutes. I was waiting to know if it was a valid and unknown fault first.

@chucklepie
Copy link
Author

chucklepie commented May 11, 2021

Code is attached or clone:
no_shapes.zip

git@gitlab.com:chucklepie-productions/raised_bugs/no_collision_visible_33.git
https://gitlab.com/chucklepie-productions/raised_bugs/no_collision_visible_33.git

For me on 3.3 stable linux mint this is the screen:

image

On 3.2.3 stable, this is the screen
image

@Calinou
Copy link
Member

Calinou commented May 11, 2021

You need to enable the property in TileMap to display collision shapes. It's disabled by default because of its performance impact with large TileMaps.

@chucklepie
Copy link
Author

chucklepie commented May 11, 2021 via email

@Calinou
Copy link
Member

Calinou commented May 11, 2021

And why is it working on 323 and not 33?

The behavior has changed; tilemaps no longer display collision shapes by default to improve performance. This was initially prompted by #34923 because drawing outlines has a performance cost (but it does improve the debug experience a fair bit).

Is this a change in the changelog wr never noticed?

It's at the top of the Added > Core section: https://github.com/godotengine/godot/blob/3.x/CHANGELOG.md#core

Remember to read the changelog on the 3.x branch instead of master, as master does not cover minor releases anymore.

@chucklepie
Copy link
Author

chucklepie commented May 11, 2021 via email

@SirLich
Copy link
Contributor

SirLich commented May 11, 2021

Thanks, good to know. So how did it work for the others commenting here if its turned off by default? I'm no designer, but maybe even if others didn't know either, a better way would be to incorporate it somehow into the debug menu? Otherwise it'll have to be done on a per Tilemap basis, especially if there's no default project setting to do this either and should debug flags be part of a node?

On Tue, 11 May 2021, 15:53 Hugo Locurcio, @.***> wrote: And why is it working on 323 and not 33? The behavior has changed; tilemaps no longer display collision shapes by default to improve performance. This was initially prompted by #34923 <#34923> because drawing outlines has a performance cost (but it does improve the debug experience a fair bit). Is this a change in the changelog wr never noticed? It's at the top of the Added > Core section: https://github.com/godotengine/godot/blob/3.x/CHANGELOG.md#core Remember to read the changelog on the 3.x branch instead of master, as master does not cover minor releases anymore. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#48631 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADCERRLRX7AJTOW6AFQS4E3TNFAGZANCNFSM44UWWRWQ .

I am new to Godot. I simply turned on the option in the Tilemap.

I did not realize this was something "special", so I ended up accidentally doing it correctly.

If anything, that's evidence that its not confusing for beginners, at least :)

@chucklepie
Copy link
Author

chucklepie commented May 11, 2021

There you go then, us old people don't notice these minor changes ;)

But jokes aside, my point really is it doesn't quite make sense that a debug specific functionality should be part of the node and given this is debug, the 'performance' hit it gives should be understood and default to on not off especially given it has to be done for each tilemap (I have over 20 in my game spread across various levels). However, given tilemaps are being completely redone these flags (things like 'compatibility mode' and this) may just be temporary until 4.

@chucklepie
Copy link
Author

chucklepie commented May 12, 2021

@Calinou

Hello, I've just realised why this collision functionality was changed and I'm not sure it's a good thing :(

It now makes collision shapes on tilemaps consistent with all other nodes by making collision shapes visible, which I get. But I think tilemaps are a bit of an exception. You do not want to see collision shapes normally in a map as it makes editing a map impossible.

I say this because the way it's done you have to turn on and off every single tilemap in your game, which there may be dozens and spread across various scene instances, and it goes against how the debug/view collision objects menu work because unless you have it set for tilemaps in the editor you do not see collision boxes at runtime, which is a bit mad and opposite to how every other node's collision visibility works! i.e. you still cannot see tilemap collision shapes unless you have them visible in the editor.

Is this just a temporary thing and won't be around in 4?

Surely a better way is to make this visibility just an editor thing, i.e. it does not affect debug mode....

@groud
Copy link
Member

groud commented May 24, 2021

Well, this behavior was implemented in #46623. I mentioned another solution that would make things simpler to setup (with a default being no collision shown in the editor, and visible in game is "debug collision shape" is on) See the discussion in #47204 too, that's related.

IMHO, we should likely go for 3 options instead of a bool. Ping @Janglee123 @pouleyKetchoupp and @akien-mga, as we you were in the discussion there.

@akien-mga
Copy link
Member

I'm confused as I thought what we had agreed upon was that it should behave this way:

  1. Collision shapes always visible in game if debug collision shape is on. Same behavior as previous releases.
  2. Collision shapes visible in editor if and only if the property is enabled.

Seems like it's not what we implemented in the end.

@chucklepie
Copy link
Author

chucklepie commented May 24, 2021 via email

@groud
Copy link
Member

groud commented May 24, 2021

This is the condition on master, where show_collision is the Tilemap property:

bool debug_shapes = show_collision && (Engine::get_singleton()->is_editor_hint() || (st && st->is_debugging_collisions_hint()));

So indeed, that's not what @akien-mga describes.

But yeah, if we keep the boolean option, I agree we should go for that.

@pouleyKetchoupp
Copy link
Contributor

My bad if it's working this way. The idea was:
-show_collision=false: always disabled in both game and editor
-show_collision=true: always enabled in editor, controlled by "Visible Collision Shapes" in game

It's working this way to make it possible to disable collision debug for tilemaps individually in game.

So since it appears to cause confusion, we can either:

  1. Pick the simple solution @akien-mga mentioned:

Collision shapes always visible in game if debug collision shape is on. Same behavior as previous releases.
Collision shapes visible in editor if and only if the property is enabled.

Which is very clear but doesn't allow disabling individual tile maps in game.

  1. Go for @groud's proposal with 3 options. In this case it could be something like:
    Default: shows in editor, shows in game only if debug collision shape is on.
    Force show: always shows in game and editor
    Force hide: always hidden in game and editor

That allows all possible combinations, but doesn't fully address the issue from #46623 (comment) (although it seems there's a workaround for it).

For 4.0, it would be great to also have gizmos for 2D so we can enable/disable all tilemaps in editor at once. That would help with this last use case.

So maybe we can go for option 1 in 3.x (best compatibility) and option 2 with gizmo for 4.0 (best usability)?

@akien-mga
Copy link
Member

So maybe we can go for option 1 in 3.x (best compatibility) and option 2 with gizmo for 4.0 (best usability)?

Seems good to me.

@groud
Copy link
Member

groud commented May 24, 2021

Which is very clear but doesn't allow disabling individual tile maps in game.

Well, to be fair, I am not sure the use case of having a lot of TileMap in same scene, with all having collision is very common. I think the issue in #46623 might be when you have a lot of different scene, each with a TileMap in it.

For 4.0, it would be great to also have gizmos for 2D so we can enable/disable all tilemaps in editor at once. That would help with this last use case.

The more I think of it, the more I believe implementing TileMap layers would be a good idea in the end.
I wasn't sure it was worth the work, as It would not be a big change for the setup, but it seems like it would simplify a lot the editing process when you edit several TileMap at once.

@akien-mga
Copy link
Member

Fixed in upcoming 3.4 and 3.3.3 by #49075.

Fixed in upcoming 4.0 by #49024.

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

7 participants