SukkoPera / MegaDrivePlusPlus

Universal Region mod, 50/60 Hz switch and In-Game-Reset (IGR) for Sega Mega Drive (AKA Genesis)
GNU General Public License v3.0
121 stars 29 forks source link

Fix broken macro guard in rgb_led_off() #16

Closed okdana closed 4 years ago

okdana commented 4 years ago

I noticed that the #endif for the ENABLE_MODE_LED_RGB guard inside rgb_led_off() is placed inside save_mode(), so if that macro is actually undefined it won't compile. I think this is what you were going for?

okdana commented 4 years ago

Also, a semi-related but minor nit-pick: I found the placement of these functions in the file a little weird. Would it make sense to change the order from this...

void rgb_led_off() { ... }
inline void save_mode () { ... }
inline void change_mode (int increment) { ... }
inline void next_mode () { ... }
void rgb_led_update () { ... }
void flash_single_led () { ... }
void set_mode (VideoMode m, boolean save) { ... }

... to something like this (so that related ones are grouped together)?

inline void save_mode () { ... }
inline void change_mode (int increment) { ... }
inline void next_mode () { ... }
void set_mode (VideoMode m, boolean save) { ... }
void rgb_led_off() { ... }
void rgb_led_update () { ... }
void flash_single_led () { ... }

I can submit another PR if you like that.

SukkoPera commented 4 years ago

You are right! Can you please resubmit this against the experimental branch?

About the order of functions, I'll fix that myself ASAP, thanks.

okdana commented 4 years ago

Thanks, updated the PR to apply to experimental