-
Notifications
You must be signed in to change notification settings - Fork 74
[GEN][ZH] Annotate fallthrough between various switch case labels #1085
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
base: main
Are you sure you want to change the base?
Conversation
812d386
to
d88e895
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall, only a few things that seem odd.
case CHINA_FRIEND: | ||
side.set("China"); | ||
break; | ||
case GLA_ENEMY: | ||
isFriend = FALSE; | ||
isFriend = FALSE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems odd that it thinks there was a change here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems odd that it thinks there was a change here.
There's a tab after it in the original line.
case SCORESCALEUPTRANSITION_2: | ||
case SCORESCALEUPTRANSITION_3: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am surprised it does not warn about these cases falling through.
FALLTHROUGH; | ||
case 2: | ||
break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
State is not used outside of this function so this seems a bit redundant, case 2 could be removed and a break added at the end of case 1.
FALLTHROUGH; | ||
|
||
case FLASHTRANSITION_FADE_IN_2: | ||
case FLASHTRANSITION_FADE_IN_3: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as the score scaleup, im surprised it does not complain here, unless it just optimises away empty cases?
This change annotates fallthroughs in GameEngine to suppress warnings and make the intent clear.