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

Bug fixed in test.gd: should pass a string to has_method. #335

Merged
merged 2 commits into from
Jan 15, 2022

Conversation

strank
Copy link
Contributor

@strank strank commented Jan 1, 2022

No description provided.

Copy link
Owner

@bitwes bitwes left a comment

Choose a reason for hiding this comment

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

Rename _new_property to __new_property and I'll merge this in. Good find, I have no idea how this ever made it out the door.

addons/gut/test.gd Outdated Show resolved Hide resolved
@strank
Copy link
Contributor Author

strank commented Jan 14, 2022

I found this while going through the educational exercise of making Gut run on 4.0 / Godot master.
I'll put my changes into the 4.0 branch of this repo clone when I'm done in case you are interested.
Apart from this here, I found two things that might be worth doing even now:

  • mixed indentation with tabs and spaces is considered an error in Godot 4 - used in a few places to align continuation lines.
    I'd suggest just using a standard two-tab indent for continuations.
  • Setting variables of built-in types to null is impossible when adding type hints, and sometimes marked as an error otherwise.
    So maybe it is worth to think of other ways of indicating "no value yet", or lobby for the nullable? types proposal that's on godot-proposals.

@bitwes bitwes merged commit fc416ff into bitwes:master Jan 15, 2022
@bitwes
Copy link
Owner

bitwes commented Jan 15, 2022

@strank Merged. I created two issues and added a new issue tag of Godot 4.0. Can you add some examples to #339 where this is happening in GUT. I kinda followed what you are saying but not completely.

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

Successfully merging this pull request may close these issues.

2 participants