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

Add 'disabled' property to Area2D/3D #9701

Closed
reimgrab opened this issue May 8, 2024 · 6 comments
Closed

Add 'disabled' property to Area2D/3D #9701

reimgrab opened this issue May 8, 2024 · 6 comments

Comments

@reimgrab
Copy link

reimgrab commented May 8, 2024

Describe the project you are working on

SciFi pixel art 2D sidescroller

Describe the problem or limitation you are having in your project

Setting Area2Ds 'monitorable' property to false lets them still be detected by ray- and shapecasts. See this open issue
In my game some enemies have destructible parts. So one enemy can have multiple Area2Ds for his parts. These areas may have multiple collision shapes. Damage dealers (projectiles or other hazards) may be area or ray based. So just setting 'monitorable' to false only disables collisions with some of these damage dealers.
The workaround mentioned in the issue above, namely "checking if the result contains a disabled area, and casting again if it does" would result in unneeded code and performance cost (for example hitting multiple collision shapes of multiple 'disabled' areas). Also I am not sure how this is meant to be done with ray/shapecast nodes (force_ray/shapecast_update until there is a 'valid' or no collision?). Effectively this is a call for writing a custom ray/shapecast class to not duplicate the code everywhere casts are used.
Of course you could also just free the entire area but this leads to problems if they or their collision shapes are referenced in animationplayer tracks.
So currently IMHO the best way to deal with this is to disable the corresponding collision shapes. This means that you have to have references to all collision shapes of all areas in the code or in animation player tracks just to disable them.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Having a 'disabled' property for Area2D/3D would simplify disabling all interactions with this area tremendously and bugs where you forgot to disable one of multiple collision shapes could not occur.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Areas get a bool 'disabled' property which when set would set monitoring and monitorable to false and also disable interactions with ray- and shapecasts.

If this enhancement will not be used often, can it be worked around with a few lines of script?

As described above it can be worked around by disabling the collision shapes of the areas but that is more work, uglier code and more error prone than setting a single property on the area.

Is there a reason why this should be core and not an add-on in the asset library?

Area2D/3D are native classes

@theraot
Copy link

theraot commented May 8, 2024

You can disable the shapes that make up an area (or body). Since they must be direct children, you can iterate over the children and disable them. Know that you can't do this during a physics response, and so you might have to use a deferred operation (an error message will tell you).

However, if shapes might not be direct children in the future (see: godotengine/godot#77937) then we would have to do this recursively... In that case, it would be good to have a better way to do it.

@reimgrab
Copy link
Author

reimgrab commented May 8, 2024

I know that I can do that in code but not in an animationplayer. So you have to make a function that you can call in the animationplayer do this and put it in all scripts that need that feature. Where we are at a point where it is better to just make an area subclass that has this functionality.
And I think that this feature may be used often enough (even with only a single collision shape) as it will disable not only the collision shapes but set the monitoring/monitorable as well, that it warrants to be in the base class.
It is not a show stopper but still it would be nice to have.

@reimgrab reimgrab closed this as completed May 8, 2024
@reimgrab
Copy link
Author

reimgrab commented May 8, 2024

Sorry, missed click

@reimgrab reimgrab reopened this May 8, 2024
@theraot
Copy link

theraot commented May 8, 2024

I can see that, disabling and enabling shapes with animation tracks would be a pain.

Doing the subclass you suggest would work. Which suggest that we can partially solve this with an addon library, with some drawbacks: you need to change your scrips to extend the new class instead of area directly. And that might be futher hassle if you need your scripts to extend another class already.

Here is an alternative if you need this working today: You can make an script where you export a reference to the area (or even an array of them), and a variable with a setter that will perform the enabling or disabling. This way it is not a subclass, so you do not need to update your existing code. Instead you would add a new node with the mentioned script, set the area reference, which you can animate.

@dalexeev
Copy link
Member

dalexeev commented May 8, 2024

There is CollisionObject2D.disable_mode property, by default it is DISABLE_MODE_REMOVE:

When Node.process_mode is set to Node.PROCESS_MODE_DISABLED, remove from the physics simulation to stop all physics interactions with this CollisionObject2D.

Automatically re-added to the physics simulation when the Node is processed again.

So you can process_mode = PROCESS_MODE_DISABLED to disable an area/body and process_mode = PROCESS_MODE_INHERIT to enable.

@reimgrab
Copy link
Author

reimgrab commented May 9, 2024

Currently porting my game from 3 to 4, so I have not realized that pause mode changed to process mode, thanks.

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

3 participants