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

Execute _get_configuration_warning() even in non-tool classes #3004

Open
SeleDreams opened this issue Jul 17, 2021 · 11 comments
Open

Execute _get_configuration_warning() even in non-tool classes #3004

SeleDreams opened this issue Jul 17, 2021 · 11 comments

Comments

@SeleDreams
Copy link

SeleDreams commented Jul 17, 2021

Describe the project you are working on

I am working on a 3D action rpg with godot engine

Describe the problem or limitation you are having in your project

In order to avoid bugs, I like to display warnings when things aren't properly setup as it's always likely to forget stuff when working on a big project causing big issues down the road.
This is why I rely on _get_configuration_warning in order to display warnings whenever a child is missing or some exported value isn't correctly set

The main issue is that due to how it currently works, _get_configuration_warning forces me to set my entire class to a tool class for it to be called, it forces me to add editor checks to all my functions just to ensure my code is only called in game, this can cause real issues down the road to my scenes if i ever forget to add a check at some point

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

allowing _get_configuration_warning to execute in normal classes would avoid having to manually check the editor everytime in every other function of the class reducing risks of scene breaking bugs.

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

I'd imagine that writing

func _get_configuration_warning():
return "Warning"
would display a yellow sign with warning on the node even if the class isn't a tool class

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

this enhancement would be very crucial to guarantee code doesn't risk breaking the scene because the user didn't add checks for the editor on every single callback of their class

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

This is something related to the core functionality of godot

@YuriSizov
Copy link
Contributor

YuriSizov commented Jul 17, 2021

This is a bit too specific. I think that what this proposal should be about is allowing to mark individual methods and properties as tool methods and properties (with an annotation perhaps, in GDScript), while the class itself is not a tool script.

This would require changes to the way scripts are loaded in the editor, but at least it would be done in a generalized way instead of a special case for one particular method.

@SeleDreams
Copy link
Author

This is a bit too specific. I think that what this proposal should be about is allowing to mark individual methods and properties as tool methods and properties (with an annotation perhaps, in GDScript), while the class itself is not a tool script.

This would require changes to the way scripts are loaded in the editor, but at least it would be done in a generalized way instead of a special case for one particular method.

this is another idea i had, the reason i talked about _get_configuration_warning is because this callback makes no sense in game as it's meant for the editor specifically

@Zylann
Copy link

Zylann commented Jul 17, 2021

Note: I realize now my comment is about a general concept of tool functions that might be broader than this proposal. Although it was mentionned on Discord so I thought I'll leave it.


This would require scripts containing such directives to get compiled entirely in the editor (or at least in a special way if we want to just have the function and what it uses) just so the function can be run. Because then that also allows the function to call other functions of the script (which you'd have to also tag as tool somehow), which in turn could depend on having run other logic, and... you can sometimes get into a mix of it. Although in this case you would have to fallback again on a global tool on top of the script.

Also everywhere the editor calls script functions it would have to verify if it was made fully tool or not, cuz that would become different concepts depending on which function is being called.

Compilation-wise it also sounds like something that would be specific to GDScript, as I can't see any way to do this with C# without having compiled and instanced an entire class (which would end up calling the constructor, even if not the whole class is marked tool).
So it's mostly a problem of making sure the editor calls or not calls the correct functions on a case by case basis. Because right now the editor just checks if it's compiled or not (if it's not compiled it will be a placeholder instance that does nothing), so making a class compiled just so _get_configuration_warning can be called, means the editor will see it can call everything else.

_get_configuration_warning is because this callback makes no sense in game as it's meant for the editor specifically

Alternatively, I wonder if it's possible for an editor plugin (or an editor script of some sort) to produce warnings, such that it decouples the game files from the editor files, but atm that sounds impractical to people who want to mix everything in the same file.

@timothyqiu
Copy link
Member

timothyqiu commented Jul 17, 2021

I think an alternative way of thinking this proposal, although impractical, might be implicitly making the script a tool if _get_configuration_warning exists, and add wrapping if not Engine.is_editor_hint() to other methods when compiling the script.

@YuriSizov
Copy link
Contributor

@Zylann I think that we could introduce a @tool(partial) annotation of sorts that would suggest to the compiler that the script needs to be compiled for the editor use, but that parts not marked by @tool_method or @tool_property should be omitted.

I imagine ignoring parts of the script and making them no-op/non-existent is not a hard task for the compilation step, and then for the rest of the system it would just be a tool script that is missing parts of its code (the excluded lines would literally be empty if it was presented as a text, so that the line references align). From the point of that scripts validity it would be no different if the user was to remove the lines themselves.

@Calinou Calinou changed the title Execute _get_configuration_warning even in non-tool classes Execute _get_configuration_warning() even in non-tool classes Jul 17, 2021
@dalexeev
Copy link
Member

Related: #729

@ChrisAbra
Copy link

The lack of this causes some real problems with C# use.

If you want to use _GetConfigurationWarnings you have to make your class a Tool script but then if you want to check or set any Nodes those nodes have to be a Tool as well or else you get a cast exception.

The docs warn against using Tool unless needed but here it's forcing us to for what should be relatively simple validation.

@Calinou
Copy link
Member

Calinou commented Sep 16, 2023

To (partially) resolve the core problem, we could call _get_configuration_warnings() in non-tool scripts when running a project (in the node's NOTIFICATION_READY notification) and print a warning message to console with the node path and a list of warnings. This will not be used within the editor, but it also addresses situations where you procedurally create nodes during gameplay.

@neth392
Copy link

neth392 commented Apr 2, 2024

I ran into this issue where I wanted configuration warnings to work but code in _process etc was being ran causing issues.

My workaround was to put this in my _ready() function and keep the @tool annotation

	if Engine.is_editor_hint():		
		set_process(false)
		set_physics_process(false)
		set_process_input(false)
		set_process_unhandled_input(false)

@aduernberger
Copy link

@neth392 Can you confirm that _physics_process is not called if you call set_physics_process(false)?
If I copy the code above and add a print("test") to _physics_process I get permanent output in the editor.

@neth392
Copy link

neth392 commented Jun 19, 2024

@aduernberger I can't, I honestly forgot what I was working on involving this. What I would do if I were you is put if Engine.is_editor_hint(): return in your _physics_process. If you need code to run in that function in the editor just put it before the statement. Code that shouldn't run in the editor would go below

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

9 participants