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

Update switch statements to prevent build warnings #207

Merged
merged 1 commit into from
Sep 20, 2024

Conversation

devloglogan
Copy link
Collaborator

These switch statements throw a lot of warnings every time I build the plugin because not every enum value is explicitly handled. Changing them to if statements fixes that.

@devloglogan devloglogan added this to the 3.0.1 milestone Sep 19, 2024
@m4gr3d
Copy link
Collaborator

m4gr3d commented Sep 19, 2024

I think the warnings are useful for that scenario; could we instead include all the enum, with a no-op comment for the ones where nothing needs to be done?
Or is the build complaining about *_MAX enum values?

@devloglogan
Copy link
Collaborator Author

Or is the build complaining about *_MAX enum values?

It does complain bout *_MAX enum values, but I see why keeping them switch statements could be valuable. No reason we can't just drop the *_MAX case in there as well I guess? I've updated the code to this.

Copy link
Collaborator

@m4gr3d m4gr3d left a comment

Choose a reason for hiding this comment

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

Looks good to me!

I'll wait until @BastiaanOlij and @dsnopek chime in as well before merging.

Copy link
Collaborator

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Copy link
Member

@BastiaanOlij BastiaanOlij left a comment

Choose a reason for hiding this comment

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

lgtm

@m4gr3d m4gr3d merged commit 9349841 into GodotVR:master Sep 20, 2024
10 checks passed
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.

4 participants