GDScript: Add warning if non-@tool class extends @tool class

[dalexeev]

• Closes @tool script extended without @tool does not show all exported vars in inspector #78174.
• Closes Warn if script inheriting tool script isn't tool godot-proposals#754.

[akien-mga]

I support the idea in principle.

One question we might want to ponder is whether there might be a valid use case for extending @tool scripts with a non-@tool script, to disable the tool-specific behavior in that script. E.g. for a script that has both in-game and in-editor logic, where the in-editor logic is only meant for debugging in a specific scene, but inherited scenes with their own extending scripts should not run it.

I guess this use case, if it exists, could be achieved the other way around by making the base script non @tool and extending it via a @tool script only where wanted.

[ajreckof]

Effectively I can see cases where the base script is a tool script and the extended should not. For example the base script can have some exported value that has a setter that needs to run but the current script has a lot of things happening on process which you don't want to run in the editor because of performance reasons.
The workaround proposed by akien does not work because as of now if a script is marked as @tool even if the base class is not marked as tool it will be considered as such.
This is still a fix for the issue but it feels more like a bandaid then a proper fix. I think a proper fix should aim to make it possible to have abas script that run in the editor while the script extending it is not i @tool mode

[dalexeev]

The problem is that we can't make @tool work only for some of the methods and not for others. Especially if the child class overrides the methods of the base class. We cannot guarantee that the methods will never be called. Therefore, the script can either be entirely in @tool mode or not.

If you propose the ability to disable @tool for an inherited class (the base class's @tool code is not executed), then I think that some explicit annotation (@notool or @disable_tool) is needed, not just the absence of @tool. Because the current issue is that users may forget to add @tool or assume that it inherits automatically.

Or we can replace the error with a warning and shift the responsibility to users, but we need to come up with code so that the warning can be ignored.

[Flavelius]

@notool or @disable_tool definitely makes more sense, because the base class explicitly expresses the intent to be a tool, and this explicit requirement shouln't be implicitly violated by inheritors. It's the same reason that not all base class methods are virtual by default. In godot's special case though with explicit annotations users/inheritors can alteast make it their responsibility to deal with broken behaviour.

[adamscott]

As discussed in the GDScript team meeting, instead of adding @disable_tool or @notool, we decided to go ahead with @tool(false) as annotations can take arguments. Technically, the first argument of tool would default to true if none is supplied.

[dalexeev]

we decided to go ahead with @tool(false) as annotations can take arguments. Technically, the first argument of tool would default to true if none is supplied.

Autocompletion inserts empty parentheses when there are optional arguments. We could have added an exception, but I remembered that we already have an annotation for ignoring warnings, which is most likely the first thing the user will use by clicking the "Ignore" button in the warning bar. I don't think we need any other way than @warning_ignore.

[Mickeon]

Documentation for the warning looks fine but I have a feel this PR is abandoned a bit?

[Mickeon]

I personally relate to the issues the warning would address (it used to be extremely painful figuring them out on my own), but it's worth questioning if this should be in 4.3 right now, in feature freeze.

[akien-mga]

Looks good to me.

[akien-mga]

Thanks!