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

GD-579: Minimize warnings in debug mode with any warning is configuration #579

Closed
Tattomoosa opened this issue Sep 24, 2024 · 3 comments · Fixed by #580, #582, #583, #584 or #586
Closed

GD-579: Minimize warnings in debug mode with any warning is configuration #579

Tattomoosa opened this issue Sep 24, 2024 · 3 comments · Fixed by #580, #582, #583, #584 or #586
Assignees
Labels
enhancement New feature or request minor
Milestone

Comments

@Tattomoosa
Copy link

Tattomoosa commented Sep 24, 2024

Is your feature request related to a problem? Please describe.

I'm developing an engine plugin and using GdUnit to test it. Whenever I run my tests in debug mode, I get a bunch of warnings. It's all GDScript warning settings that I've turned on that are not on by default, and because to develop my plugin I have to turn the "exclude addons" setting off.

It does not emit any warnings during normal testing.

Describe the solution you'd like

GdUnit should be developed with all possible warnings on, and written to comply with them either through refactoring or warning_ignore annotations.

Describe alternatives you've considered
Changing my warning settings while debugging tests... not ideal but doable.
Godot adding a feature to selectively ignore addon warnings (Found godotengine/godot#93889 so this might happen soon)

Additional context
Is this a reasonable thing to expect? Is the developer toil here worth the better experience for the very few users it will affect? Refactoring to avoid all warnings would touch a ton of code - I'm not even sure how much since Godot caps at 400 with the "Too many warnings!" error. Some of the warnings don't provide much safety for how annoying they can be to write around (eg return value discarded)

A list of warnings ignored by default which push warnings when running tests in debug mode:

  • Unsafe call argument
  • Return value discarded
  • Unsafe cast
  • Unsafe method access
  • Unsafe property access
  • Inferred declaration

(This is all of the default ignored warnings except Untyped Declaration)

@Tattomoosa Tattomoosa added the enhancement New feature or request label Sep 24, 2024
@MikeSchulze
Copy link
Owner

Hi, I know this problem and I had started to minimize the warnings some time ago, but stopped for now to focus on the release of version 4.4.

There is also a pending task to review and adjust the code style across all classes.
Most of the issues are migrated from the original implementation of GdUnit3 and never touched to apply the current code style guide.

Can you please attach your project setting to inspect the warnings you have enabled?

@MikeSchulze MikeSchulze modified the milestones: v4.4.0, v4.4.1 Sep 25, 2024
@MikeSchulze MikeSchulze changed the title GdUnit should not emit warnings in debug mode with any warning configuration GD-578: Minimize warnings in debug mode with any warning is configuration Sep 25, 2024
@MikeSchulze MikeSchulze changed the title GD-578: Minimize warnings in debug mode with any warning is configuration GD-579: Minimize warnings in debug mode with any warning is configuration Sep 25, 2024
@MikeSchulze MikeSchulze linked a pull request Sep 25, 2024 that will close this issue
@Tattomoosa
Copy link
Author

If I remember correctly the most prevalent one I saw was return value discarded. I listed all of the ignored-by-default settings that push warnings when set to warn in my initial post. Default settings didn't push any warnings that I saw. Funny enough I am thinking about giving up on return value discarded in my project anyway... it's a very annoying one to deal with and I don't think it's helped me catch any logic errors yet.

My settings:

gdscript/warnings/exclude_addons=false
gdscript/warnings/untyped_declaration=1
gdscript/warnings/unsafe_property_access=1
gdscript/warnings/unsafe_method_access=1
gdscript/warnings/return_value_discarded=1

@MikeSchulze
Copy link
Owner

So finally I merged all fixes into master.
image

The two warnings I do not fix because of nonsense to set it to warn or error.

  • debug/gdscript/warnings/inferred_declaration
    To set a variable type save by var foo := "" as String is fine, to change it to var foo :String = "" is no difference
  • debug/gdscript/warnings/return_value_discarded
    It makes no sense to enable this warning because of the fluent style of the asserts. It will generate, for each test, these warnings because of the nature of the assert style I implemented.

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