From 44f4d67c80385d78459fda1cf22914926c0d55d1 Mon Sep 17 00:00:00 2001 From: Gary Talent Date: Fri, 26 Feb 2021 08:55:40 -0600 Subject: [PATCH] [nostalgia] Address CLion recommendations --- developer-handbook.md | 14 ++++---- scripts/setup-build.py | 32 ++++++++++++------- src/nostalgia/common/bounds.hpp | 12 +++---- src/nostalgia/core/CMakeLists.txt | 1 - src/nostalgia/core/gba/gfx.cpp | 2 +- src/nostalgia/core/gfx.hpp | 2 +- src/nostalgia/core/mem.hpp | 13 -------- src/nostalgia/core/sdl/core.cpp | 2 +- src/nostalgia/core/sdl/gfx.cpp | 4 +-- src/nostalgia/core/studio/imgconv.hpp | 14 +------- .../core/studio/newpalettewizard.cpp | 1 - src/nostalgia/core/studio/paletteeditor.cpp | 28 ++++++++-------- src/nostalgia/core/studio/tilesheeteditor.cpp | 4 --- src/nostalgia/player/main.cpp | 2 -- 14 files changed, 53 insertions(+), 78 deletions(-) delete mode 100644 src/nostalgia/core/mem.hpp diff --git a/developer-handbook.md b/developer-handbook.md index 5f76d4be..01392666 100644 --- a/developer-handbook.md +++ b/developer-handbook.md @@ -112,13 +112,13 @@ int main() { ``` The code base where this was observed actually got away with this for the most -part, as the std::vector implementation used evidentally waited until the +part, as the std::vector implementation used evidently waited until the internal array was needed before initializing and the memory was zeroed out because the allocation occurred early in the program's execution. While the -std::vector implementation in queston worked with this code and the memory leak -is not noticable because the std::vector was meant to exist for the entire life +std::vector implementation in question worked with this code and the memory leak +is not noticeable because the std::vector was meant to exist for the entire life of the process, other classes likely will not get away with it due to more -substantial constructors and more frequent instatiations of the classes in +substantial constructors and more frequent instantiations of the classes in question. ### Pointers vs References @@ -140,7 +140,7 @@ one of the main reasons why many embedded developers prefer C to C++. Instead throwing exceptions, all engine code must return error codes. Nostalgia and Ox both use ```ox::Error``` to report errors. ```ox::Error``` is a struct that has overloaded operators to behave like an integer error code, plus some -extra fields to enhance debugability. If instantiated through the ```OxError(x)``` +extra fields to enhance debuggability. If instantiated through the ```OxError(x)``` macro, it will also include the file and line of the error. The ```OxError(x)``` macro should only be used for the initial instantiation of an ```ox::Error```. @@ -186,7 +186,7 @@ back up the call stack, ```oxReturnError``` and ```oxThrowError```. will return an ```ox::Error``` if it is not 0 and ```oxThrowError``` will throw an ```ox::Error``` if it is not 0. Because exceptions are disabled for GBA builds and thus cannot be used in the engine, ```oxThrowError``` is only really -useful at the boundry between engine libraries and Nostalgia Studio. +useful at the boundary between engine libraries and Nostalgia Studio. ```cpp void studioCode() { @@ -223,7 +223,7 @@ ox::Error engineCode() { ### File I/O All engine file I/O should go through nostalgia::core::Context, which should go -through ox::FileSystem. Similarly, all studio file I/O should go throuh +through ox::FileSystem. Similarly, all studio file I/O should go thorough nostalgia::studio::Project, which should go through ox::FileSystem. ox::FileSystem abstracts away differences between conventional storage devices diff --git a/scripts/setup-build.py b/scripts/setup-build.py index 15bd23a0..2d0a4cd4 100755 --- a/scripts/setup-build.py +++ b/scripts/setup-build.py @@ -9,9 +9,11 @@ import sys from pybb import mkdir, rm + def main(): parser = argparse.ArgumentParser() - parser.add_argument('--target', help='Platform target ({OS}-{Arch},gba)', default='{:s}-{:s}'.format(sys.platform, platform.machine())) + parser.add_argument('--target', help='Platform target ({OS}-{Arch},gba)', + default='{:s}-{:s}'.format(sys.platform, platform.machine())) parser.add_argument('--build_type', help='Build type (asan,debug,release)', default='release') parser.add_argument('--build_tool', help='Build tool (default,xcode)', default='') parser.add_argument('--vcpkg_dir', help='Path to VCPKG') @@ -33,6 +35,9 @@ def main(): elif args.build_type == 'release': build_type_arg = 'Release' sanitizer_status = 'OFF' + else: + print('Error: Invalid build tool') + sys.exit(1) if args.build_tool == 'xcode': build_config = '{:s}-{:s}'.format(args.target, args.build_tool) @@ -45,27 +50,30 @@ def main(): qt_path = '' if args.build_tool == '' or args.build_tool == 'default': - if shutil.which('ninja') == None: + if shutil.which('ninja') is None: build_tool = '' else: build_tool = '-GNinja' elif args.build_tool == 'xcode': build_tool = '-GXcode' + else: + print('Error: Invalid build tool') + sys.exit(1) project_dir = os.getcwd() build_dir = '{:s}/build/{:s}'.format(project_dir, build_config) rm(build_dir) mkdir(build_dir) subprocess.run(['cmake', '-S', project_dir, '-B', build_dir, build_tool, - '-DCMAKE_EXPORT_COMPILE_COMMANDS=ON', - '-DCMAKE_BUILD_TYPE={:s}'.format(build_type_arg), - '-DUSE_ASAN={:s}'.format(sanitizer_status), - '-DNOSTALGIA_IDE_BUILD=OFF', - '-DNOSTALGIA_BUILD_CONFIG={:s}'.format(build_config), - '-DNOSTALGIA_BUILD_TYPE={:s}'.format(nostalgia_build_type), - qt_path, - toolchain, - ]) + '-DCMAKE_EXPORT_COMPILE_COMMANDS=ON', + '-DCMAKE_BUILD_TYPE={:s}'.format(build_type_arg), + '-DUSE_ASAN={:s}'.format(sanitizer_status), + '-DNOSTALGIA_IDE_BUILD=OFF', + '-DNOSTALGIA_BUILD_CONFIG={:s}'.format(build_config), + '-DNOSTALGIA_BUILD_TYPE={:s}'.format(nostalgia_build_type), + qt_path, + toolchain, + ]) mkdir('dist') if args.target != 'gba': @@ -77,9 +85,9 @@ def main(): if platform.system() != 'Windows': os.symlink('build/{:s}/compile_commands.json'.format(build_config), 'compile_commands.json') + if __name__ == '__main__': try: main() except KeyboardInterrupt: sys.exit(1) - diff --git a/src/nostalgia/common/bounds.hpp b/src/nostalgia/common/bounds.hpp index 90bc6104..48de1908 100644 --- a/src/nostalgia/common/bounds.hpp +++ b/src/nostalgia/common/bounds.hpp @@ -22,17 +22,17 @@ class Bounds { Bounds(int x, int y, int w, int h); - bool intersects(Bounds other) const; + [[nodiscard]] bool intersects(Bounds other) const; - bool contains(int x, int y) const; + [[nodiscard]] bool contains(int x, int y) const; - int x2() const; + [[nodiscard]] int x2() const; - int y2() const; + [[nodiscard]] int y2() const; - Point pt1(); + [[nodiscard]] Point pt1(); - Point pt2(); + [[nodiscard]] Point pt2(); }; template diff --git a/src/nostalgia/core/CMakeLists.txt b/src/nostalgia/core/CMakeLists.txt index 66ba3817..b8db6a0e 100644 --- a/src/nostalgia/core/CMakeLists.txt +++ b/src/nostalgia/core/CMakeLists.txt @@ -32,7 +32,6 @@ install( gfx.hpp input.hpp media.hpp - mem.hpp DESTINATION include/nostalgia/core ) diff --git a/src/nostalgia/core/gba/gfx.cpp b/src/nostalgia/core/gba/gfx.cpp index e2dc76c5..0fe8a270 100644 --- a/src/nostalgia/core/gba/gfx.cpp +++ b/src/nostalgia/core/gba/gfx.cpp @@ -219,7 +219,7 @@ void clearTileLayer(Context*, int layer) { memset(&MEM_BG_MAP[layer], 0, GbaTileRows * GbaTileColumns); } -void hideSprite(Context*, unsigned idx) { + [[maybe_unused]] void hideSprite(Context*, unsigned idx) { oxAssert(g_spriteUpdates < config::GbaSpriteBufferLen, "Sprite update buffer overflow"); GbaSpriteAttrUpdate oa; oa.attr0 = 2 << 8; diff --git a/src/nostalgia/core/gfx.hpp b/src/nostalgia/core/gfx.hpp index 9d2a1801..9fe714e8 100644 --- a/src/nostalgia/core/gfx.hpp +++ b/src/nostalgia/core/gfx.hpp @@ -139,7 +139,7 @@ void setTile(Context *ctx, int layer, int column, int row, uint8_t tile); void clearTileLayer(Context*, int layer); -void hideSprite(Context*, unsigned); + [[maybe_unused]] void hideSprite(Context*, unsigned); void setSprite(Context*, unsigned idx, unsigned x, unsigned y, unsigned tileIdx, unsigned spriteShape = 0, unsigned spriteSize = 0, unsigned flipX = 0); diff --git a/src/nostalgia/core/mem.hpp b/src/nostalgia/core/mem.hpp deleted file mode 100644 index 078ed3b7..00000000 --- a/src/nostalgia/core/mem.hpp +++ /dev/null @@ -1,13 +0,0 @@ -/* - * Copyright 2016 - 2021 gary@drinkingtea.net - * - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. - */ - -namespace nostalgia::core { - -void initHeap(); - -} diff --git a/src/nostalgia/core/sdl/core.cpp b/src/nostalgia/core/sdl/core.cpp index 99622060..24be503e 100644 --- a/src/nostalgia/core/sdl/core.cpp +++ b/src/nostalgia/core/sdl/core.cpp @@ -50,7 +50,7 @@ void setEventHandler(event_handler h) { } uint64_t ticksMs() { - return SDL_GetTicks();; + return SDL_GetTicks(); } bool buttonDown(Key) { diff --git a/src/nostalgia/core/sdl/gfx.cpp b/src/nostalgia/core/sdl/gfx.cpp index 7cbc658a..c95fe2b6 100644 --- a/src/nostalgia/core/sdl/gfx.cpp +++ b/src/nostalgia/core/sdl/gfx.cpp @@ -7,7 +7,6 @@ */ #include -#include #ifdef NOST_FPS_PRINT #include #endif @@ -196,7 +195,7 @@ void draw(Context *ctx) { void puts(Context *ctx, int column, int row, const char *str) { for (int i = 0; str[i]; i++) { - setTile(ctx, 0, column + i, row, static_cast(charMap[static_cast(str[i])])); + setTile(ctx, 0, column + i, row, static_cast(charMap[static_cast(str[i])])); } } @@ -208,6 +207,7 @@ void setTile(Context *ctx, int layer, int column, int row, uint8_t tile) { id->bgTileMaps[z][y][x] = tile; } +[[maybe_unused]] void hideSprite(Context*, unsigned) { } diff --git a/src/nostalgia/core/studio/imgconv.hpp b/src/nostalgia/core/studio/imgconv.hpp index 4c5f136f..65754fa2 100644 --- a/src/nostalgia/core/studio/imgconv.hpp +++ b/src/nostalgia/core/studio/imgconv.hpp @@ -29,18 +29,6 @@ namespace nostalgia::core { return colStart + colOffset + rowStart + rowOffset; } -template -ox::Result> toBuffer(T *data, std::size_t buffSize = ox::units::MB) { - std::vector buff(buffSize); - std::size_t sz = 0; - oxReturnError(ox::writeMC(buff.data(), buff.size(), data, &sz)); - if (sz > buffSize) { - return OxError(1); - } - buff.resize(sz); - return buff; -} - -[[nodiscard]] std::unique_ptr imgToNg(QString argInPath, int argBpp = -1); + [[nodiscard]] std::unique_ptr imgToNg(QString argInPath, int argBpp = -1); } diff --git a/src/nostalgia/core/studio/newpalettewizard.cpp b/src/nostalgia/core/studio/newpalettewizard.cpp index 10a30564..55dad2ff 100644 --- a/src/nostalgia/core/studio/newpalettewizard.cpp +++ b/src/nostalgia/core/studio/newpalettewizard.cpp @@ -8,7 +8,6 @@ #include #include -#include #include #include diff --git a/src/nostalgia/core/studio/paletteeditor.cpp b/src/nostalgia/core/studio/paletteeditor.cpp index d877ed38..5f1acd6c 100644 --- a/src/nostalgia/core/studio/paletteeditor.cpp +++ b/src/nostalgia/core/studio/paletteeditor.cpp @@ -31,22 +31,22 @@ enum class PaletteEditorCommandId { class ColorChannelValidator: public QValidator { public: - ColorChannelValidator(QLineEdit *parent); + explicit ColorChannelValidator(QLineEdit *parent); QValidator::State validate(QString &input, int&) const override; private: - QString convert(const QString &input) const; + [[nodiscard]] static QString convert(const QString &input); }; ColorChannelValidator::ColorChannelValidator(QLineEdit *parent): QValidator(parent) { - connect(parent, &QLineEdit::editingFinished, [this, parent] { + connect(parent, &QLineEdit::editingFinished, [parent] { parent->setText(convert(parent->text())); }); } -QString ColorChannelValidator::convert(const QString &input) const { +QString ColorChannelValidator::convert(const QString &input) { int num = 0; if (input[0] == '_') { num = input.mid(1).toInt() >> 3; @@ -86,9 +86,9 @@ class AddColorCommand: public QUndoCommand { m_idx = idx; } - virtual ~AddColorCommand() = default; + ~AddColorCommand() override = default; - int id() const override { + [[nodiscard]] int id() const override { return static_cast(PaletteEditorCommandId::AddColor); } @@ -115,9 +115,9 @@ class RemoveColorCommand: public QUndoCommand { m_idx = idx; } - virtual ~RemoveColorCommand() = default; + ~RemoveColorCommand() override = default; - int id() const override { + [[nodiscard]] int id() const override { return static_cast(PaletteEditorCommandId::RemoveColor); } @@ -147,9 +147,9 @@ class UpdateColorCommand: public QUndoCommand { setObsolete(m_oldColor == m_newColor); } - virtual ~UpdateColorCommand() = default; + ~UpdateColorCommand() override = default; - int id() const override { + [[nodiscard]] int id() const override { return static_cast(PaletteEditorCommandId::UpdateColor); } @@ -176,9 +176,9 @@ class MoveColorCommand: public QUndoCommand { m_offset = offset; } - virtual ~MoveColorCommand() = default; + ~MoveColorCommand() override = default; - int id() const override { + [[nodiscard]] int id() const override { return static_cast(PaletteEditorCommandId::MoveColor); } @@ -212,7 +212,7 @@ void PaletteEditorColorTableDelegate::paint(QPainter *painter, const QStyleOptio } -static QTableWidgetItem *mkCell(QString v, bool editable = true) { +static QTableWidgetItem *mkCell(const QString& v, bool editable = true) { auto c = new QTableWidgetItem; c->setText(v); c->setFont(QFont("monospace")); @@ -345,7 +345,7 @@ Color16 PaletteEditor::rowColor(int row) const { void PaletteEditor::colorSelected() { auto selIdxs = m_table->selectionModel()->selectedIndexes(); - auto row = selIdxs.size() ? selIdxs[0].row() : -1; + auto row = !selIdxs.empty() ? selIdxs[0].row() : -1; if (row > -1) { m_rmBtn->setEnabled(true); m_moveUpBtn->setEnabled(row > 0); diff --git a/src/nostalgia/core/studio/tilesheeteditor.cpp b/src/nostalgia/core/studio/tilesheeteditor.cpp index fd3dada0..6edffbc5 100644 --- a/src/nostalgia/core/studio/tilesheeteditor.cpp +++ b/src/nostalgia/core/studio/tilesheeteditor.cpp @@ -10,7 +10,6 @@ #include #include -#include #include #include #include @@ -18,15 +17,12 @@ #include #include #include -#include #include #include #include #include #include #include -#include -#include #include #include diff --git a/src/nostalgia/player/main.cpp b/src/nostalgia/player/main.cpp index ad5f2fae..1e45c5e5 100644 --- a/src/nostalgia/player/main.cpp +++ b/src/nostalgia/player/main.cpp @@ -7,10 +7,8 @@ */ #include -#include #include #include -#include using namespace nostalgia;