Refactoring + rework of saving process #32
Conversation
|
@ohol-vitaliy Thanks for your contribution. I request that you use git rebase to remove the garbage commits related to README edits. |
| project(gravity_defied_cpp C CXX) | ||
| set(CMAKE_VERBOSE_MAKEFILE TRUE) |
There was a problem hiding this comment.
There is no need to force this option. You can pass it via -D on the command line.
| @@ -13,14 +14,18 @@ if(WIN32) | |||
| endif() | |||
|
|
|||
| find_package(PkgConfig REQUIRED) | |||
| find_package(OpenSSL REQUIRED) | |||
There was a problem hiding this comment.
OpenSSL, just for the sake of md5 hash, is redundant. Can we use a lighter implementation or another algorithm like crc32. Are there any analogs in the standard c++ library?
| @@ -77,7 +82,7 @@ endif() | |||
| add_executable(GravityDefied ${SOURCES}) | |||
|
|
|||
| target_compile_features(GravityDefied PRIVATE cxx_std_17) | |||
| target_compile_options(GravityDefied PRIVATE -Wall -Wextra) | |||
| target_compile_options(GravityDefied PRIVATE -Wall -Wextra -fsigned-char) | |||
There was a problem hiding this comment.
How is it different from what it was?
| { | ||
| field_232 = var1; | ||
| } | ||
| // void GameCanvas::method_163(int var1) |
There was a problem hiding this comment.
Why is this commented? Please leave a comment why this can't be removed.
| int menuOffsetY; | ||
| int menuSpacing; | ||
| int menuOffsetX; | ||
| // int itemsToShowWindowStart; |
There was a problem hiding this comment.
Why is this commented? Please leave a comment why this can't be removed.
|
@ohol-vitaliy Please check everywhere in your code for extra/missing spaces/new lines.And also justify the commented out pieces of code that should not be removed. |
| { | ||
| int var1; | ||
| for (var1 = 0; var1 < 10; ++var1) { | ||
| activeKeys[var1] = false; |
There was a problem hiding this comment.
Is this no longer required? std::vector<bool> activeKeys = std::vector<bool>(10) still exists.
| if (field_95 > field_106) { | ||
| ++field_105; | ||
| ++field_106; | ||
| if (static_cast<int>(menuItems.size()) <= numberOfItemsToFit) { |
There was a problem hiding this comment.
It seems static_cast is redundant. I think when comparing, there will be an indirect type conversion. Perhaps numberOfItemsToFit should be size_t. It seems that there is similar code in this PR.
| loadedTrack = track; | ||
|
|
||
| const MRGLoader::LevelTracks l = trackHeaders.at(level); | ||
| const int tracksCount = static_cast<int>(l.tracks.size()); |
There was a problem hiding this comment.
sizet_t ? Remove static_cast?
| #include <SDL2/SDL.h> | ||
| #include <SDL2/SDL_image.h> | ||
| #include <SDL2/SDL_ttf.h> | ||
| #include <stdexcept> |
There was a problem hiding this comment.
Group headers (standard, sdl, internal)
| @@ -3,8 +3,10 @@ | |||
| #include <string> | |||
| #include <SDL2/SDL.h> | |||
| #include <SDL2/SDL_ttf.h> | |||
| #include <stdexcept> | |||
|
|
||
| va_list args; | ||
| va_start(args, szFormat); | ||
| vprintf(szFormat, args); |
There was a problem hiding this comment.
I would recommend disabling buffering for stdout. If the application crashes, we may not see the entire log.
| @@ -1,34 +0,0 @@ | |||
| #include "test_rms.h" | |||
There was a problem hiding this comment.
Doesn't the test need to be rewritten?
What was done