Compile certain CanvasItem._edit_*() functions with DEBUG_ENABLED

[YeldhamDev]

This fixes the errors while compiling using target=template_debug which were introduced with #97257.

The functions that PR makes use of are:

• _edit_is_selected_on_click()
• _edit_use_rect()
• _edit_get_rect()

But after some talk with other members of the dev team, I chose to make the entire CanvasItem._edit_*() family of functions be compiled with DEBUG_ENABLED instead of TOOLS_ENABLED, as it's cleaner code-wise. If this is considered too much, I can change this so only the stated functions are affected. Only the functions specified now use DEBUG_ENABLED.

[clayjohn]

This should only be exposing the functions that need to be exposed. Its bad enough that we have editor code in our scene functions, we shouldn't make the situation worse by unconditionally exposing editor code.

I would limit this PR to just the functions that are breaking builds currently.

[dsnopek]

Thanks!

This is compiling for me now on Android. I also tested using the game editor feature with "Remote Debug" to my phone, and it worked great!

I chose to make the entire CanvasItem._edit_*() family of functions be compiled with DEBUG_ENABLED instead of TOOLS_ENABLED, as it's cleaner code-wise. If this is considered too much, I can change this so only the stated functions are affected.

I'm personally OK with this. It makes sense to me that more editor-only functionality is needed in game to allow this tighter integration between the game and editor.

But it would be good to get input from folks who work on the editor more.

[dsnopek]

@clayjohn:

we shouldn't make the situation worse by unconditionally exposing editor code.

It not quite unconditionally: this is only for debug builds.

But I don't do too much work in the editor, so I'm not the one to say whether or not including these functions in debug builds is problematic.

[clayjohn]

But I don't do too much work in the editor, so I'm not the one to say whether or not including these functions in debug builds is problematic.

Its not that it will be problematic at run time. The problem is that this reinforces the coupling between the editor and the scene system. Ideally we would have no editor-only code in the scene code. We made compromises for specific functions, but we protected them behind an editor check. Now we are removing that check and exposing them more widely. A new contributor may now unknowingly use these functions elsewhere. Then we may see requests to expose this code to runtime. Then we end up with bloat and technical debt.

Ideally in cases like this we would provide an API through the scene system that allows moving all the editor logic into the editor rather than piling on editor logic into the scene system.

A good illustration of this problem is in our best practices guideline. Prefer Local Solutions:
https://docs.godotengine.org/en/latest/contributing/development/best_practices_for_engine_contributors.html#prefer-local-solutions

[YeldhamDev]

@clayjohn Understood, changes made.

[dsnopek]

I think something needs to be done for this one:

godot/scene/2d/physics/collision_shape_2d.h

Line 62 in acfdfb8

#ifdef TOOLS_ENABLED

Maybe changing to:

#if defined(TOOLS_ENABLED) || defined(DEBUG_ENABLED)

Otherwise, I'm seeing an error about missing override

EDIT: Not for this PR, but CI doesn't appear to be making any target=template_debug builds. If it was, then I think CI would have caught this (as well as the original issue with PR #97257).

[YeldhamDev]

@dsnopek Missed that one, thank you. Strangely, my compiler didn't spit any errors about this while also compiling with target=template_debug.

[dsnopek]

It's building for me again! Just in case, I also re-tested using the "Game" editor with "Remote Debug" and an Android app, and that's still working fine.