Get rid of platform subfolder #772

Merged
merged 4 commits into from Feb 21, 2016

Projects

None yet

3 participants

@fdelapena
Member

There are already platform specific files in the src folder, e.g. input_buttons_*.
The purpose of this pull request is normalize these cases and use proper preprocessor guards instead of using dummy files just for including them.

This PR has nothing to do with a proper subfolder redesign, if considered in the future to make source code navigation cleaner could be OK, this should not be considered as a step back.

fdelapena added some commits Feb 20, 2016
@fdelapena fdelapena Move registry files from platform to src 18602f1
@fdelapena fdelapena Move audio files from platform to src ac4a067
@fdelapena fdelapena Move emscripten files from platform to resources and cleanup
0d1d774
@carstene1ns carstene1ns commented on an outdated diff Feb 21, 2016
src/sdl_ui.cpp
audio_.reset(new SdlAudio());
-#elif defined(HAVE_OPENAL)
+#elif HAVE_OPENAL
@carstene1ns
carstene1ns Feb 21, 2016 Member

These checks are not the same. You check if HAVE_SDL_MIXER is defined at all (may be set 0 => disabled or even some random string "foobar") and then check if HAVE_OPENAL is defined and set to true (i.e. 1).
I do not consider the way it was done before really better, but it should be consistent.

@carstene1ns carstene1ns commented on the diff Feb 21, 2016
src/sdl_ui.cpp
#ifdef _WIN32
- #define WIN32_LEAN_AND_MEAN
- #ifndef NOMINMAX
- #define NOMINMAX
- #endif
- #include <windows.h>
- #include "SDL_syswm.h"
+# define WIN32_LEAN_AND_MEAN
+# ifndef NOMINMAX
+# define NOMINMAX
+# endif
+# include <windows.h>
+# include "SDL_syswm.h"
@carstene1ns
carstene1ns Feb 21, 2016 Member

We really need a tab vs. spaces policy somehow.

@fdelapena
fdelapena Feb 21, 2016 Member

Some years ago we changed these to the way I did because of http://stackoverflow.com/a/789156/1244033

@carstene1ns carstene1ns commented on the diff Feb 21, 2016
src/audio_al.cpp
@@ -605,3 +607,5 @@ void ALAudio::SE_Stop() {
}
se_src_.clear();
}
+
+#endif
@carstene1ns
carstene1ns Feb 21, 2016 Member

Also a policy to include or not include comments about closing preprocessor conditionals // HAVE_OPENAL

@carstene1ns carstene1ns commented on the diff Feb 21, 2016
src/sdl_ui.cpp
@@ -1211,4 +1218,4 @@ void GekkoResetCallback() {
}
#endif
-#endif
+#endif // USE_SDL
@carstene1ns
carstene1ns Feb 21, 2016 Member

(added here for example)

@carstene1ns
Member

The good thing about this pull request is that the build systems like autotools now detect if things in the formerly hidden (i.e. not timestamped) platform/* files changed 👍.

@fdelapena fdelapena Add defined() to elif
0c6a6ab
@Ghabry Ghabry merged commit 4a25f54 into EasyRPG:master Feb 21, 2016

4 of 5 checks passed

web Build finished. No test results found.
Details
Android Build finished. No test results found.
Details
Linux Build finished. No test results found.
Details
OSX Build finished. No test results found.
Details
Windows Build finished. No test results found.
Details
@fdelapena fdelapena deleted the fdelapena:platform branch Feb 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment