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.

Sign up for GitHub

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

Jump to bottom

GDScript compiler inner class bugfixes #68374

Merged
merged 1 commit into from
Nov 14, 2022
Merged

GDScript compiler inner class bugfixes #68374

merged 1 commit into from
Nov 14, 2022

Conversation

rune-scape
Copy link
Contributor

@rune-scape rune-scape commented Nov 7, 2022 •
edited by akien-mga
Loading

Replaces some important outdated code about subclasses.
rpc properties correctly apply to subclasses.
subclasses of subclasses now have docs populated.
shallow scripts have their subclass scripts populated and name and id set.
various other fixes.

@rune-scape rune-scape requested a review from a team as a code owner November 7, 2022 13:42
@Chaosus Chaosus added this to the 4.0 milestone Nov 8, 2022
@Chaosus
Copy link
Member

Chaosus commented Nov 8, 2022

Don't forget to squash the commits.

@rune-scape rune-scape closed this Nov 8, 2022
@rune-scape rune-scape force-pushed the rune-gdscript-compiler-bugfixes branch from 972a2ac to f814e15 Compare November 8, 2022 11:35
@rune-scape
Copy link
Contributor Author

collapsed them into nothing lol oops

@rune-scape rune-scape reopened this Nov 8, 2022
@rune-scape rune-scape force-pushed the rune-gdscript-compiler-bugfixes branch from 45bd73c to bc905aa Compare November 8, 2022 11:51
@adamscott adamscott mentioned this pull request Nov 10, 2022
Merged
@adamscott
Copy link
Member

As discussed in #67714, I think this PR should be merged before or after #67714 in order to fix #65953. This PR fix is way more deep that the solution I used in my PR.

@adamscott adamscott mentioned this pull request Nov 10, 2022
Closed
@rune-scape rune-scape force-pushed the rune-gdscript-compiler-bugfixes branch 3 times, most recently from cb3d482 to ca717de Compare November 11, 2022 09:25
adamscott
adamscott approved these changes Nov 12, 2022
View reviewed changes
Copy link
Member

@adamscott adamscott left a comment

Choose a reason for hiding this comment

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

At first glance, it seems fine to me. Can't wait to integrate those changes in my PR.

@adamscott adamscott mentioned this pull request Nov 12, 2022
Closed
@rune-scape rune-scape force-pushed the rune-gdscript-compiler-bugfixes branch 3 times, most recently from dfacfb2 to 700c8b8 Compare November 13, 2022 10:26
@rune-scape rune-scape force-pushed the rune-gdscript-compiler-bugfixes branch from 700c8b8 to bce6f17 Compare November 13, 2022 10:29
@rune-scape
Copy link
Contributor Author

@adamscott sorry for changing it last minute, i added tests and found a bug

This was referenced Nov 14, 2022
Closed
Closed
@rune-scape rune-scape changed the title GDScript compiler subclass bugfixes GDScript compiler inner class bugfixes Nov 14, 2022
@rune-scape rune-scape mentioned this pull request Nov 14, 2022
Closed
akien-mga
akien-mga approved these changes Nov 14, 2022
View reviewed changes
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

I'm not competent to do an in-depth review, but generally speaking this look solid. vnen is not available right now but I put this on his backlog to possibly review post-merge when he has time.

Let's get this merged and tested more broadly to see if any regressions creep up.

@akien-mga akien-mga merged commit 963ffd8 into godotengine:master Nov 14, 2022
@akien-mga
Copy link
Member

Thanks!

This was referenced Nov 16, 2022
Closed
Closed
Closed
Closed
Closed
Closed
@MikeSchulze
Copy link

Thanks for you all hard work!

Rindbee
Rindbee reviewed Nov 20, 2022
View reviewed changes
modules/gdscript/gdscript_cache.cpp
Comment on lines +178 to +180
if (r_error != OK) {
return Ref<GDScript>();
}

This comment was marked as spam.

Sign in to view

@rune-scape rune-scape mentioned this pull request Nov 20, 2022
Merged
@Chaosus Chaosus mentioned this pull request Nov 22, 2022
Closed
@rune-scape rune-scape mentioned this pull request Nov 22, 2022
Closed
@rune-scape rune-scape mentioned this pull request Dec 4, 2022
Closed
@YuriSizov YuriSizov mentioned this pull request Mar 8, 2023
Closed
@mattbork mattbork mentioned this pull request Oct 18, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Rindbee Rindbee Rindbee left review comments

@adamscott adamscott adamscott approved these changes

@akien-mga akien-mga akien-mga approved these changes

Assignees
No one assigned
Labels
bug topic:gdscript
Projects
None yet
Milestone
4.0
Development

Successfully merging this pull request may close these issues.

GDScript 2.0: Incorrect base class when using extends Class.Subclass Typehints on inner classes do not recognize function argument types GDScript 2.0: "Cannot convert argument 1 from Object to Object" when signal parameter is typed with an inner class. Subclass cannot be inherited from other subclass in autoload using its naming alias GDScript 2.0: Subclass member with type of another script's subclass does not get assigned correctly
Random errors related to inner classes GDScript 2.0: Typed assigning an inner class leads to a runtime error GDScript 2.0: Extending an inner class results in invalid constructor resolution at runtime GDScript 2.0: Calling an overwritten function of an external inner class results in a runtime error GDScript: Subclass overwrites rpc info
6 participants
@rune-scape @Chaosus @adamscott @akien-mga @MikeSchulze @Rindbee