From e90706461caadcaf3ae5913a27555dda13d8d6a6 Mon Sep 17 00:00:00 2001 From: Tayou Date: Fri, 19 May 2023 21:21:15 +0200 Subject: [PATCH] some further cleanup, following some Clang-Tidy suggestions and CodeQL Signed-off-by: Tayou --- .../minecraft/gameoptions/GameOptions.cpp | 56 ++++++++----------- .../minecraft/gameoptions/GameOptionsSchema.h | 12 ++-- .../GameOptions/GameOptionWidgetKeyBind.cpp | 2 + 3 files changed, 32 insertions(+), 38 deletions(-) diff --git a/launcher/minecraft/gameoptions/GameOptions.cpp b/launcher/minecraft/gameoptions/GameOptions.cpp index c98a964f6..ac2344f1e 100644 --- a/launcher/minecraft/gameoptions/GameOptions.cpp +++ b/launcher/minecraft/gameoptions/GameOptions.cpp @@ -41,11 +41,6 @@ #include "FileSystem.h" #include "GameOptions.h" -static Qt::CheckState boolToState(bool b) -{ - return b ? Qt::Checked : Qt::Unchecked; -}; - namespace { bool load(const QString& path, std::vector& contents, @@ -91,32 +86,26 @@ bool load(const QString& path, bool isFloat = false; item.intValue = item.value.toInt(&isInt); item.floatValue = item.value.toFloat(&isFloat); - if (isInt) { - item.type = OptionType::Int; - // qDebug() << "The value" << value << "is a int"; - } else if (isFloat) { - item.type = OptionType::Float; - // qDebug() << "The value" << value << "is a float"; - } else if (item.value == "true" || item.value == "false") { - item.boolValue = item.value == "true"; - item.type = OptionType::Bool; - qDebug() << "The value" << item.value << "is a bool"; - } else if (item.value.endsWith("]") && item.value.startsWith("[")) { + item.boolValue = item.value == "true"; + item.type = isInt ? OptionType::Int : isFloat ? OptionType::Float : OptionType::Bool; + if (item.value.startsWith("[") && item.value.endsWith("]")) { qDebug() << "The value" << item.value << "is an array"; for (const QString& part : item.value.mid(1, item.value.size() - 2).split(",")) { GameOptionChildItem child{ part, static_cast(contents.size()) }; qDebug() << "Array has entry" << part; item.children.append(child); } + item.type = OptionType::Array; } else if (item.key.startsWith("key_")) { item.type = OptionType::KeyBind; } else { - // this is really ugly, please suggest how to truncate the start and end - item.value = item.value.remove(item.value.length()-1, 1).remove(0, 1); + // This removes the leading and ending " from the string to display it more cleanly. + item.value = item.value.mid(1, item.value.length()-2); + item.type = OptionType::String; } - // adds reference to known option from gameOptionsSchema if avaiable to get display name and other metadata - item.knownOption = knownOptions->find(item.key) != knownOptions->end() ? knownOptions->find(item.key).value() : nullptr; + // adds reference to known option from gameOptionsSchema if available to get display name and other metadata + item.knownOption = knownOptions->value(item.key, nullptr); contents.emplace_back(item); } qDebug() << "Loaded" << path << "with version:" << version; @@ -171,6 +160,7 @@ bool GameOptions::setData(const QModelIndex& index, const QVariant& value, int r auto column = (Column)index.column(); if (column == Column::Value) { switch (contents[row].type) { + case OptionType::Array: case OptionType::String: { contents[row].value = value.toString(); return true; @@ -203,13 +193,13 @@ Qt::ItemFlags GameOptions::flags(const QModelIndex& index) const QVariant GameOptions::data(const QModelIndex& index, int role) const { if (!index.isValid()) - return QVariant(); + return {}; int row = index.row(); Column column = (Column)index.column(); if (row < 0 || row >= int(contents.size())) - return QVariant(); + return {}; if (index.parent().isValid()) { switch (role) { @@ -218,11 +208,11 @@ QVariant GameOptions::data(const QModelIndex& index, int role) const GameOptionChildItem* item = static_cast(index.internalPointer()); return item->value; } else { - return QVariant(); + return {}; } } default: { - return QVariant(); + return {}; } } } @@ -237,29 +227,29 @@ QVariant GameOptions::data(const QModelIndex& index, int role) const if (contents[row].knownOption != nullptr) { return contents[row].knownOption->description; } else { - return QVariant(); + return {}; } } default: { - return QVariant(); + return {}; } } } default: { - return QVariant(); + return {}; } } - return QVariant(); + return {}; } QModelIndex GameOptions::index(int row, int column, const QModelIndex& parent) const { if (!hasIndex(row, column, parent)) - return QModelIndex(); + return {}; if (parent.isValid()) { if (parent.parent().isValid()) - return QModelIndex(); + return {}; GameOptionItem* item = static_cast(parent.internalPointer()); return createIndex(row, column, &item->children[row]); @@ -271,14 +261,14 @@ QModelIndex GameOptions::index(int row, int column, const QModelIndex& parent) c QModelIndex GameOptions::parent(const QModelIndex& index) const { if (!index.isValid()) - return QModelIndex(); + return {}; const void* childItem = index.internalPointer(); // Determine where childItem points to if (childItem >= &contents[0] && childItem <= &contents.back()) { // Parent is root/contents - return QModelIndex(); + return {}; } else { GameOptionChildItem* child = static_cast(index.internalPointer()); return createIndex(child->parentRow, 0, reinterpret_cast(&contents[child->parentRow])); @@ -298,7 +288,7 @@ int GameOptions::rowCount(const QModelIndex& parent) const if (parent.parent().isValid()) return 0; - GameOptionItem* item = static_cast(parent.internalPointer()); + auto* item = static_cast(parent.internalPointer()); return item->children.count(); } } diff --git a/launcher/minecraft/gameoptions/GameOptionsSchema.h b/launcher/minecraft/gameoptions/GameOptionsSchema.h index ab8e9aee7..cf5437ea4 100644 --- a/launcher/minecraft/gameoptions/GameOptionsSchema.h +++ b/launcher/minecraft/gameoptions/GameOptionsSchema.h @@ -23,7 +23,7 @@ #include #include -enum class OptionType { String, Int, Float, Bool, KeyBind }; +enum class OptionType { String, Int, Float, Bool, KeyBind, Array }; template struct Range { T min, max; @@ -38,7 +38,6 @@ union OptionValue { float floatValue; int intValue; bool boolValue; - // QString stringValue; }; class GameOption { @@ -91,13 +90,16 @@ class GameOption { OptionType type; bool readOnly = false; QList validValues; // if empty, treat as text input - //int introducedVersion; - //int removedVersion; + + // Not sure if versioning these makes sense, it would be a lot of effort to keep track of as new minecraft versions are made. + // not even considering the large bulk of old minecraft versions, of which some data may not even be documented yet. + // int introducedVersion; // format version number, where this options key got introduced + // int removedVersion; // format version number, where this options key got removed - -1 if it is still present in the latest version int getDefaultInt() const { return defaultValue.intValue; }; bool getDefaultBool() const { return defaultValue.boolValue; }; float getDefaultFloat() const { return defaultValue.floatValue; }; - QString getDefaultString();; + QString getDefaultString(); Range getIntRange() const { return range.intRange; }; Range getFloatRange() const { return range.floatRange; }; diff --git a/launcher/ui/widgets/GameOptions/GameOptionWidgetKeyBind.cpp b/launcher/ui/widgets/GameOptions/GameOptionWidgetKeyBind.cpp index 2879b8b39..639de01b2 100644 --- a/launcher/ui/widgets/GameOptions/GameOptionWidgetKeyBind.cpp +++ b/launcher/ui/widgets/GameOptions/GameOptionWidgetKeyBind.cpp @@ -38,6 +38,7 @@ GameOptionWidgetKeyBind::~GameOptionWidgetKeyBind() } void GameOptionWidgetKeyBind::setEditorData(GameOptionItem optionItem) { + // TODO implement this, *keybindingOptions is needed /*for (auto& keyBinding : *keybindingOptions) { // this could become a std::find_if eventually, if someone wants to bother making it that. if (keyBinding->minecraftKeyCode == contents[row].knownOption->getDefaultString()) { @@ -50,6 +51,7 @@ void GameOptionWidgetKeyBind::setEditorData(GameOptionItem optionItem) { } void GameOptionWidgetKeyBind::saveEditorData(GameOptionItem optionItem) { QString minecraftKeyCode; + // TODO implement this, *keybindingOptions is needed /*for (auto& keyBinding : *keybindingOptions) { // this could become a std::find_if eventually, if someone wants to bother making it that. if (keyBinding->qtKeyCode.keyboardKey == ui->keySequenceEdit->keySequence()[0].key() ||