Skip to content

Fix GDScript error when using an inherited const as annotation#57510

Closed
trollodel wants to merge 1 commit intogodotengine:masterfrom
trollodel:gdscript_inherited_const_fix
Closed

Fix GDScript error when using an inherited const as annotation#57510
trollodel wants to merge 1 commit intogodotengine:masterfrom
trollodel:gdscript_inherited_const_fix

Conversation

@trollodel
Copy link
Copy Markdown
Contributor

@trollodel trollodel commented Feb 1, 2022 •
edited
Loading

Fix a GDScript analyzer error when using as an annotation a class declared as const which is declared in the parent script (or in a relative one). Regardless the fact is not ideal, declaring again the const class in the child script raises an error because the const is already declared, preventing to workaround the issue.

Example:

# base_class.gd
const external_script = preload("external_script.gd")

func virtual_method(_ext: external_script) -> String:
	return "Base"
extends "base_class.gd"

# Error: "external_script" was not found in the current scope
func virtual_method(_ext: external_script) -> String:
	return "Subclass"

This PR includes an integration test for this issue.

@trollodel trollodel force-pushed the gdscript_inherited_const_fix branch from 2105c4e to d712fe9 Compare February 1, 2022 10:47
@akien-mga akien-mga added this to the 4.0 milestone Feb 1, 2022
@akien-mga akien-mga requested a review from a team February 1, 2022 10:48
@trollodel trollodel marked this pull request as ready for review February 1, 2022 11:28
@vnen
Copy link
Copy Markdown
Member

vnen commented Jun 15, 2022

I believe this doesn't cover enough cases, as it's not checking the inheritance tree of the inner class itself (only for the outermost class). So the proper way would be to check the inherited constants of the current class, then go the outer one and do the same, then the outer one from that and so on. It's complicated but it should solve all cases.

If you want to do it I can help, otherwise I'll tackle it later.

@trollodel
Copy link
Copy Markdown
Contributor Author

trollodel commented Jun 15, 2022 •
edited
Loading

I believe this doesn't cover enough cases, as it's not checking the inheritance tree of the inner class itself (only for the outermost class). So the proper way would be to check the inherited constants of the current class, then go the outer one and do the same, then the outer one from that and so on. It's complicated but it should solve all cases.

Yeah, I suspected that (see the note in the OP).

If you want to do it I can help, otherwise I'll tackle it later.

I can work on it, but not in the near future. The idea I had is to have this completed and then work on the other cases in another PR, but I understand why you may want to fix all the cases here.

@trollodel trollodel force-pushed the gdscript_inherited_const_fix branch 2 times, most recently from 3d2392f to 834cc8e Compare August 5, 2022 10:24
@trollodel trollodel force-pushed the gdscript_inherited_const_fix branch from 834cc8e to 64b70ac Compare November 18, 2022 07:11
@trollodel trollodel force-pushed the gdscript_inherited_const_fix branch from 64b70ac to 818d33c Compare December 2, 2022 15:40
@trollodel trollodel force-pushed the gdscript_inherited_const_fix branch from 818d33c to 30e4394 Compare December 3, 2022 09:18
@trollodel trollodel mentioned this pull request Dec 5, 2022
Merged
@trollodel
Copy link
Copy Markdown
Contributor Author

trollodel commented Dec 5, 2022

Superseded by #69587.

@trollodel trollodel closed this Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

archived bug topic:gdscript

Projects

None yet

Milestone

4.0

Development

Successfully merging this pull request may close these issues.

3 participants

@trollodel @vnen @akien-mga