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 better type info everywhere #93302

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

Conversation

rune-scape
Copy link
Contributor

its slightly common to see 'NativeType(AttachedScriptType)' in an error message
but not common enough
this kinda standardizes it to 1 method get_full_type_name and uses it in many places
+also ContainerTypeValidate::get_contained_type_name()

the exact format of the full type name is of course up for debate but what i came up with:
examples of complex type name results:
Button (CustomButton)
Button (res://ui/CustomButton.gd)
GDScript[PlayerController]
GDScript[res://common/GameData.gd]
Array[GDScript (PlayerController)]

@rune-scape rune-scape requested review from a team as code owners June 18, 2024 11:16
@AThousandShips
Copy link
Member

I'm not sure about adding module dependencies to core

@AThousandShips AThousandShips added this to the 4.x milestone Jun 18, 2024
@rune-scape rune-scape force-pushed the better-type-info branch 2 times, most recently from c18a68d to ab92a42 Compare June 18, 2024 11:34
@rune-scape
Copy link
Contributor Author

theres mono and regex module dependencies in core and i think its properly #ifdefd
but its also not crucial to have the fully qualified typename i supose

@AThousandShips
Copy link
Member

theres mono and regex module dependencies in core

There absolutely are, what I meant was adding more, I'd say we might instead want to achieve this with adding functionality to core that can be implemented in modules, like adding them to Script, that way we maintain separation

@rune-scape
Copy link
Contributor Author

rune-scape commented Jul 3, 2024

that makes sense,, i feel a bit silly for not immediately agreeing with u

removed the dependancy on GDScript
but inner classes and builtin scripts may be displayed wrong, bc i think those aren't represented in Script::get_path()
maybe adding virtual Script::get_debug_name() in another PR is good idea?

@rune-scape rune-scape force-pushed the better-type-info branch 2 times, most recently from 88f2ef3 to e88164e Compare July 22, 2024 11:18
@rune-scape rune-scape requested review from a team as code owners November 8, 2024 08:26
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.

2 participants