8000 Clean up codebase and fix bug by jdpatdiscord · Pull Request #1575 · PolyMC/PolyMC · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Clean up codebase and fix bug #1575

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 7, 2023
Merged

Clean up codebase and fix bug #1575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4eb9085
Fix pack updating & Modrinth overrides
jdpatdiscord May 7, 2023
1f4d9cc
Avoid possible conflicts with C++20 `requires` keyword
jdpatdiscord May 7, 2023
9cb6200
Cleanup codebase
jdpatdiscord May 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 21 additions & 35 deletions launcher/FileSystem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ QString getDesktopDir()
// Cross-platform Shortcut creation
bool createShortCut(QString location, QString dest, QStringList args, QString name, QString icon)
{
#if defined(Q_OS_LINUX) || defined(Q_OS_FREEBSD)
#if !defined(Q_OS_WIN) && !defined(Q_OS_OSX)
location = PathCombine(location, name + ".desktop");

QFile f(location);
Expand All @@ -389,49 +389,35 @@ bool createShortCut(QString location, QString dest, QStringList args, QString na
f.setPermissions(f.permissions() | QFileDevice::ExeOwner | QFileDevice::ExeGroup | QFileDevice::ExeOther);

return true;
#elif defined Q_OS_WIN
// TODO: Fix
// QFile file(PathCombine(location, name + ".lnk"));
// WCHAR *file_w;
// WCHAR *dest_w;
// WCHAR *args_w;
// file.fileName().toWCharArray(file_w);
// dest.toWCharArray(dest_w);

// QString argStr;
// for (int i = 0; i < args.count(); i++)
// {
// argStr.append(args[i]);
// argStr.append(" ");
// }
// argStr.toWCharArray(args_w);

// return SUCCEEDED(CreateLink(file_w, dest_w, args_w));
return false;
#else
qWarning("Desktop Shortcuts not supported on your platform!");
return false;
#endif
}

bool overrideFolder(QString overwritten_path, QString override_path)
bool mergeFolders(QString dstpath, QString srcpath)
{
using copy_opts = fs::copy_options;

if (!FS::ensureFolderPathExists(overwritten_path))
return false;

std::error_code err;
fs::copy_options opt = copy_opts::recursive | copy_opts::overwrite_existing;

fs::copy(toStdString(override_path), toStdString(overwritten_path), opt, err);

if (err) {
qCritical() << QString("Failed to apply override from %1 to %2").arg(override_path, overwritten_path);
qCritical() << "Reason:" << QString::fromStdString(err.message());
std::error_code ec;
fs::path fullSrcPath = srcpath.toStdString();
fs::path fullDstPath = dstpath.toStdString();
for (auto& entry : fs::recursive_directory_iterator(fullSrcPath))
{
fs::path relativeChild = fs::relative(entry, fullSrcPath);
if (entry.is_directory())
if (!fs::exists(fullDstPath / relativeChild))
fs::create_directory(fullDstPath / relativeChild);
if (entry.is_regular_file())
{
fs::path childDst = fullDstPath / relativeChild;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this declared here instead of higher up

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's declared inside the if's scope, since it's not used outside of that scope, why declare it anywhere else?

if (fs::exists(childDst))
fs::remove(childDst);
fs::copy(entry, childDst, fs::copy_options::none, ec);
if (ec.value() != 0)
qCritical() << QString("File copy failed with: %1. File was %2 -> %3").arg(QString::fromStdString(ec.message()), entry.path().c_str(), childDst.c_str());
}
}

return err.value() == 0;
return ec.value() == 0;
}

}
2 changes: 1 addition & 1 deletion launcher/FileSystem.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,5 +154,5 @@ QString getDesktopDir();

// Overrides one folder with the contents of another, preserving items exclusive to the first folder
// Equivalent to doing QDir::rename, but allowing for overrides
bool overrideFolder(QString overwritten_path, QString override_path);
bool mergeFolders(QString dstpath, QString srcpath);
}
2 changes: 1 addition & 1 deletion launcher/InstanceList.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -904,7 +904,7 @@ bool InstanceList::commitStagedInstance(const QString& path, InstanceName const&
QString destination = FS::PathCombine(m_instDir, instID);

if (should_override) {
if (!FS::overrideFolder(destination, path)) {
if (!FS::mergeFolders(destination, path)) {
qWarning() << "Failed to override" << path << "to" << destination;
return false;
}
Expand Down
13 changes: 7 additions & 6 deletions launcher/LaunchController.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ void LaunchController::login() {
switch(m_accountToUse->accountState()) {
case AccountState::Offline: {
m_session->wants_online = false;
// NOTE: fallthrough is intentional
[[fallthrough]];
}
case AccountState::Online: {
if(!m_session->wants_online) {
Expand Down Expand Up @@ -223,7 +223,6 @@ void LaunchController::login() {
APPLICATION->settings()->set("LastOfflinePlayerName", usedname);
}
m_session->MakeOffline(usedname);
// offline flavored game from here :3
}
if(m_accountToUse->ownsMinecraft()) {
if(!m_accountToUse->hasProfile()) {
Expand Down Expand Up @@ -270,7 +269,7 @@ void LaunchController::login() {
// This means some sort of soft error that we can fix with a refresh ... so let's refresh.
case AccountState::Unchecked: {
m_accountToUse->refresh();
// NOTE: fallthrough intentional
[[fallthrough]];
}
case AccountState::Working: {
// refresh is in progress, we need to wait for it to finish to proceed.
Expand All @@ -283,12 +282,11 @@ void LaunchController::login() {
progDialog.execWithTask(task.get());
continue;
}
// FIXME: this is missing - the meaning is that the account is queued for refresh and we should wait for that
/*
case AccountState::Queued: {
// FIXME: this is missing - the meaning is that the account is queued for refresh and we should wait for that
qWarning() << "AccountState::Queued is not implemented";
return;
}
*/
case AccountState::Expired: {
auto errorString = tr("The account has expired and needs to be logged into manually again.");
QMessageBox::warning(
Expand Down Expand Up @@ -325,6 +323,9 @@ void LaunchController::login() {
emitFailed(errorString);
return;
}
default: {
qWarning() << "Invalid AccountState enum";
}
}
}
emitFailed(tr("Failed to launch."));
Expand Down
2 changes: 2 additions & 0 deletions launcher/VersionProxyModel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ QVariant VersionProxyModel::data(const QModelIndex &index, int role) const
return tr("Latest");
}
}
[[fallthrough]];
}
default:
{
Expand Down Expand Up @@ -254,6 +255,7 @@ QVariant VersionProxyModel::data(const QModelIndex &index, int role) const
}
return pixmap;
}
[[fallthrough]];
}
default:
{
Expand Down
13 changes: 4 additions & 9 deletions launcher/meta/Index.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,9 @@ QVariant Index::data(const QModelIndex &index, int role) const
switch (role)
{
case Qt::DisplayRole:
switch (index.column())
{
case 0: return list->humanReadable();
default: break;
}
if (index.column() == 0)
return list->humanReadable();
break;
case UidRole: return list->uid();
case NameRole: return list->name();
case ListPtrRole: return QVariant::fromValue(list);
Expand All @@ -70,10 +68,7 @@ QVariant Index::headerData(int section, Qt::Orientation orientation, int role) c
{
return tr("Name");
}
else
{
return QVariant();
}
return QVariant();
}

bool Index::hasUid(const QString &uid) const
Expand Down
7 changes: 3 additions & 4 deletions launcher/meta/JsonFormat.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,10 @@ static VersionPtr parseCommonVersion(const QString &uid, const QJsonObject &obj)
version->setType(ensureString(obj, "type", QString()));
version->setRecommended(ensureBoolean(obj, QString("recommended"), false));
version->setVolatile(ensureBoolean(obj, QString("volatile"), false));
RequireSet requires, conflicts;
parseRequires(obj, &requires, "requires");
RequireSet required, conflicts;
parseRequires(obj, &required, "requires");
parseRequires(obj, &conflicts, "conflicts");
version->setRequires(requires, conflicts);
version->setRequires(required, conflicts);
return version;
}

Expand Down Expand Up @@ -176,7 +176,6 @@ void parseRequires(const QJsonObject& obj, RequireSet* ptr, const char * keyName
{
if(obj.contains(keyName))
{
QSet<QString> requires;
auto reqArray = requireArray(obj, keyName);
auto iter = reqArray.begin();
while(iter != reqArray.end())
Expand Down
4 changes: 2 additions & 2 deletions launcher/meta/Version.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,9 @@ void Meta::Version::setTime(const qint64 time)
emit timeChanged();
}

void Meta::Version::setRequires(const Meta::RequireSet &requires, const Meta::RequireSet &conflicts)
void Meta::Version::setRequires(const Meta::RequireSet &required, const Meta::RequireSet &conflicts)
{
m_requires = requires;
m_requires = required;
m_conflicts = conflicts;
emit requiresChanged();
}
Expand Down
4 changes: 2 additions & 2 deletions launcher/meta/Version.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class Version : public QObject, public BaseVersion, public BaseEntity
{
return m_time;
}
const Meta::RequireSet &requires() const
const Meta::RequireSet &required() const
{
return m_requires;
}
Expand All @@ -87,7 +87,7 @@ class Version : public QObject, public BaseVersion, public BaseEntity
public: // for usage by format parsers only
void setType(const QString &type);
void setTime(const qint64 time);
void setRequires(const Meta::RequireSet &requires, const Meta::RequireSet &conflicts);
void setRequires(const Meta::RequireSet &required, const Meta::RequireSet &conflicts);
void setVolatile(bool volatile_);
void setRecommended(bool recommended);
void setProvidesRecommendations();
Expand Down
4 changes: 2 additions & 2 deletions launcher/meta/VersionList.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ QVariant VersionList::data(const QModelIndex &index, int role) const
case ParentVersionRole:
{
// FIXME: HACK: this should be generic and be replaced by something else. Anything that is a hard 'equals' dep is a 'parent uid'.
auto & reqs = version->requires();
auto & reqs = version->required();
auto iter = std::find_if(reqs.begin(), reqs.end(), [](const Require & req)
{
return req.uid == "net.minecraft";
Expand All @@ -92,7 +92,7 @@ QVariant VersionList::data(const QModelIndex &index, int role) const

case UidRole: return version->uid();
case TimeRole: return version->time();
case RequiresRole: return QVariant::fromValue(version->requires());
case RequiresRole: return QVariant::fromValue(version->required());
case SortRole: return version->rawTime();
case VersionPtrRole: return QVariant::fromValue(version);
case RecommendedRole: return version->isRecommended();
Expand Down
4 changes: 2 additions & 2 deletions launcher/minecraft/Component.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,9 +451,9 @@ void Component::updateCachedData()
m_cachedVolatile = file->m_volatile;
changed = true;
}
if(!deepCompare(m_cachedRequires, file->requires))
if(!deepCompare(m_cachedRequires, file->required))
{
m_cachedRequires = file->requires;
m_cachedRequires = file->required;
changed = true;
}
if(!deepCompare(m_cachedConflicts, file->conflicts))
Expand Down
10 changes: 5 additions & 5 deletions launcher/minecraft/OneSixVersionFormat.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,17 +266,17 @@ VersionFilePtr OneSixVersionFormat::versionFileFromJson(const QJsonDocument &doc

if (root.contains("requires"))
{
Meta::parseRequires(root, &out->requires);
Meta::parseRequires(root, &out->required);
}
QString dependsOnMinecraftVersion = root.value("mcVersion").toString();
if(!dependsOnMinecraftVersion.isEmpty())
{
Meta::Require mcReq;
mcReq.uid = "net.minecraft";
mcReq.equalsVersion = dependsOnMinecraftVersion;
if (out->requires.count(mcReq) == 0)
if (out->required.count(mcReq) == 0)
{
out->requires.insert(mcReq);
out->required.insert(mcReq);
}
}
if (root.contains("conflicts"))
Expand Down Expand Up @@ -368,9 +368,9 @@ QJsonDocument OneSixVersionFormat::versionFileToJson(const VersionFilePtr &patch
}
root.insert("mods", array);
}
if(!patch->requires.empty())
if(!patch->required.empty())
{
Meta::serializeRequires(root, &patch->requires, "requires");
Meta::serializeRequires(root, &patch->required, "requires");
}
if(!patch->conflicts.empty())
{
Expand Down
2 changes: 1 addition & 1 deletion launcher/minecraft/VersionFile.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ class VersionFile : public ProblemContainer
* PolyMC: set of packages this depends on
* NOTE: this is shared with the meta format!!!
*/
Meta::RequireSet requires;
Meta::RequireSet required;

/**
* PolyMC: set of packages this conflicts with
Expand Down
1 change: 1 addition & 0 deletions launcher/minecraft/auth/AccountData.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ enum class AccountState {
Disabled,
Errored,
Expired,
Queued,
Gone
};

Expand Down
11 changes: 9 additions & 2 deletions launcher/minecraft/auth/AccountList.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,12 @@ QVariant AccountList::data(const QModelIndex &index, int role) const
case AccountState::Gone: {
return tr("Gone", "Account status");
}
case AccountState::Queued: {
qWarning() << "Unhandled account state Queued";
[[fallthrough]];
}
default:
return tr("Unknown", "Account status");
}
}

Expand All @@ -341,8 +347,9 @@ QVariant AccountList::data(const QModelIndex &index, int role) const
else {
return tr("No", "Can Migrate?");
}
qWarning() << "Unhandled case in MigrationColumn";
[[fallthrough]];
}

default:
return QVariant();
}
Expand All @@ -359,7 +366,7 @@ QVariant AccountList::data(const QModelIndex &index, int role) const
case ProfileNameColumn:
return account == m_defaultAccount ? Qt::Checked : Qt::Unchecked;
}

[[fallthrough]];
default:
return QVariant();
}
Expand Down
1 change: 1 addition & 0 deletions launcher/minecraft/auth/Yggdrasil.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ void Yggdrasil::processReply() {
AccountTaskState::STATE_FAILED_GONE,
tr("The Mojang account no longer exists. It may have been migrated to a Microsoft account.")
);
break;
}
default:
changeState(
Expand Down
2 changes: 2 additions & 0 deletions launcher/minecraft/mod/Mod.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ std::pair<int, bool> Mod::compare(const Resource& other, SortType type) const
auto res = Resource::compare(other, type);
if (res.first != 0)
return res;
// FIXME: Determine if this is a legitimate fallthrough
[[fallthrough]];
}
case SortType::VERSION: {
auto this_ver = Version(version());
Expand Down
2 changes: 2 additions & 0 deletions launcher/minecraft/mod/Resource.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ std::pair<int, bool> Resource::compare(const Resource& other, SortType type) con
return { 1, type == SortType::ENABLED };
if (!enabled() && other.enabled())
return { -1, type == SortType::ENABLED };
[[fallthrough]];
case SortType::NAME: {
QString this_name{ name() };
QString other_name{ other.name() };
Expand All @@ -76,6 +77,7 @@ std::pair<int, bool> Resource::compare(const Resource& other, SortType type) con
auto compare_result = QString::compare(this_name, other_name, Qt::CaseInsensitive);
if (compare_result != 0)
return { compare_result, type == SortType::NAME };
[[fallthrough]];
}
case SortType::DATE:
if (dateTimeChanged() > other.dateTimeChanged())
Expand Down
Loading
0