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 automatic class_name generation for newly created scripts #91325

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fkeyzuwu
Copy link
Contributor

@fkeyzuwu fkeyzuwu commented Apr 29, 2024

Add automatic class_name generation for newly created scripts when Add Type Hints is enabled.
related to #9491

Feel free to suggest other ways to implement this instead, I just did what was most logical to me.

Add automatic class_name generation when Add Type Hints is enabled
@fkeyzuwu fkeyzuwu requested review from a team as code owners April 29, 2024 18:25
@AThousandShips AThousandShips changed the title Added automatic class_name generation for newly created scripts Add automatic class_name generation for newly created scripts Apr 29, 2024
@AThousandShips
Copy link
Member

I'd say this should always be opt-in, a lot of the time you don't want class names, I'd even say most of the time, nor are you guaranteed to need want it be the file name necessarily

Copy link
Member

@dalexeev dalexeev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion this is not a good idea. Auto-generation may cause collisions for files with the same name in different folders. And the proposal mentions reasons why we don't want to always use class names.

@@ -79,7 +79,8 @@ Ref<Script> GDScriptLanguage::make_template(const String &p_template, const Stri
type_hints = EDITOR_GET("text_editor/completion/add_type_hints");
#endif
if (!type_hints) {
processed_template = processed_template.replace(": int", "")
processed_template = processed_template.replace("class_name _CLASS_\n", "")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will remove the class name for those templates where the name is required/desired.

# Having a class name is required for a custom node.
class_name VisualShaderNode_CLASS_

# Having a class name is handy for picking the effect in the Inspector.
class_name RichText_CLASS_

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually does not remove the class name for these templates, since it is not the same string. Tested it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those specific ones yes, but you can have custom templates, or we might add new ones

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, not here specifically, but in similar cases, if we or the user in a custom template omit/forget the prefix.

@@ -1058,7 +1058,7 @@
If [code]true[/code], uses [StringName] instead of [String] when appropriate for code autocompletion.
</member>
<member name="text_editor/completion/add_type_hints" type="bool" setter="" getter="">
If [code]true[/code], adds [url=$DOCS_URL/tutorials/scripting/gdscript/static_typing.html]GDScript static typing[/url] hints such as [code]-&gt; void[/code] and [code]: int[/code] when using code autocompletion or when creating onready variables by drag and dropping nodes into the script editor while pressing the [kbd]Ctrl[/kbd] key. If [code]true[/code], newly created scripts will also automatically have type hints added to their method parameters and return types.
If [code]true[/code], adds [url=$DOCS_URL/tutorials/scripting/gdscript/static_typing.html]GDScript static typing[/url] hints such as [code]-&gt; void[/code] and [code]: int[/code] when using code autocompletion or when creating onready variables by drag and dropping nodes into the script editor while pressing the [kbd]Ctrl[/kbd] key. If [code]true[/code], newly created scripts will also automatically have type hints added to their method parameters and return types, as well as a generated [code]class_name[/code] based on the script's file name.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the point in adding additional behavior to an existing setting in this case. We could add a separate setting instead.

@fkeyzuwu
Copy link
Contributor Author

I'd say this should always be opt-in, a lot of the time you don't want class names, I'd even say most of the time, nor are you guaranteed to need want it be the file name necessarily

So where would you say this option should be available? In the mentioned issue it seems that people do not want it to be inside the create new script dialog either.

@Spartan322
Copy link
Contributor

Spartan322 commented Apr 29, 2024

I feel like it be more natural for this to be done in the Script creation dialog no?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants