WIP: Category Revamp #1880

Silarn · 2023-09-19T04:38:33Z

Nexus category import
Updated category editor pane
Update / init dialogs when loading a new / old instance
Setting to enable or disable Nexus mappings
Unmapped categories will trigger a dialog asking to ignore, map, or disable mappings
Autoassignable via dropdown / context menu

TODO: Add Nexus category ID to mod info so we don't rely on download info

Silarn · 2023-09-21T08:34:41Z

So I'll probably squash these merge commits but I think I'm ready for you to review this. I may put out one more build tomorrow with the recent changes. But I think this is in a pretty good state at this point. At least for a first version.

* No longer cause an error when deleting a category that's being used * Add a dialog when installing a mod with no Nexus mapping (if not disabled) * Allow disabling Nexus category mapping in Settings (Nexus tab) * Add context option to remove nexus mappings in the category editor * Some clang style fixes

- We will check the mod info first then fall back to the download file - Add category field in the info dialog nexus tab

- Bypasses issue where hidden downloads are not accessible - getCategoryID no longer necessary

- Merge or replace strategies - Nexus mappings are optional - If mappings are applied, remapping is optional

Holt59

Looks ok to me from a code point-of-view, I'd have to test it but I'm not sure I'll have time, except for:

I don't see why instance() returns a pointer now (reference before). This makes a lot of changes for not much in this PR since the function always return a non-null value (and the return is never checked anyway).
I don't like the plugin container added to the category dialog because it means passing a big object (in term of features) to a deep dialog. I feel like setting the container should not be needed.
I think the version.rc file is broken, maybe due to clang format.

Holt59 · 2023-09-23T16:19:22Z

src/version.rc


-VS_VERSION_INFO VERSIONINFO


This does not look right.

Holt59 · 2023-09-23T16:26:22Z

src/categoriesdialog.cpp

+void CategoriesDialog::nexusRefresh_clicked()
+{
+  NexusInterface& nexus = NexusInterface::instance();
+  nexus.setPluginContainer(m_PluginContainer);


I don't really understand what this does here and this is the only place where the plugin container is used in the dialog, so this single line mean the plugin container needs to be pass all the way down here for not much.

It was to avoid having to take multiple steps to route the signal to the Nexus API and get useable data back to the Dialog.

But I've done so now anyway.

Holt59 · 2023-09-23T16:29:25Z

src/categories.cpp

 }

-CategoryFactory& CategoryFactory::instance()
+CategoryFactory* CategoryFactory::instance()


This should not return a pointer since this can never be null.

Holt59 · 2023-09-23T16:29:48Z

src/categories.cpp

 }

 void CategoryFactory::reset()
 {
  m_Categories.clear();
+  m_NexusMap.clear();
  m_IDMap.clear();
  // 28 =


Maybe remove the comments since the value are not hardcoded anymore.

I have a feeling these were somehow left behind after a rebase at some point.

Holt59 · 2023-09-23T16:31:11Z

src/categories.h

        : m_SortValue(sortValue), m_ID(id), m_Name(name), m_HasChildren(false),
-          m_NexusIDs(nexusIDs), m_ParentID(parentID)
+          m_ParentID(parentID), m_NexusCats(nexusCats)


nexusCats should be moved here.

I'm assuming you are wanting to simply replace the old 'nexusIDs' but I moved this because it matched the order of the data in the CategoryDialog table.

Therefore I'm not sure I agree with simply replacing the old parameter, as this did have a reason.

No I was simply saying that this should be m_NexusCats(std::move(nexusCats)) since you are taking nexusCats by value, but that's a very minor thing. ;)

Holt59 · 2023-09-23T16:34:37Z

src/downloadmanager.cpp

@@ -1321,6 +1321,21 @@ QString DownloadManager::getFileName(int index) const
  return m_ActiveDownloads.at(index)->m_FileName;
 }

+int DownloadManager::getDownloadIndex(QString filename) const


Feels like this should not be in this PR?

I think I added these because I thought I needed them to get the category ID from the download during the install but I was able to pull it with the post-install nexus API call instead. So this function is not needed.

Holt59 · 2023-09-23T16:34:56Z

src/downloadmanager.h

+   * @param filename the filename of the download
+   * @return the index of the file
+   */
+  int getDownloadIndex(QString filename) const;


Same as above.

Holt59 · 2023-09-23T16:35:16Z

src/filterlist.cpp

@@ -424,7 +425,7 @@ void FilterList::checkCriteria()

 void FilterList::editCategories()
 {
-  CategoriesDialog dialog(qApp->activeWindow());
+  CategoriesDialog dialog(&m_core.pluginContainer(), qApp->activeWindow());


See previous comment, I feel like we should not need to pass the plugin container.

Holt59 · 2023-09-23T16:35:26Z

src/filterlist.h

@@ -9,6 +9,7 @@ namespace Ui
 class MainWindow;
 };
 class CategoryFactory;
+class PluginContainer;


Same as above.

Holt59 · 2023-09-23T16:37:40Z

src/organizer_en.ts

I would not commit this since this main conflict with other PRs.

Silarn · 2023-09-23T17:10:24Z

I started working on this so long ago that I'm not entirely sure why I decided some things were necessary at the time. I'll have to dig into this more later since I'm low on time, but I can probably restructure the nexus call to trigger from the mainwindow or core. That should probably cascade to allow most of these issues to be resolved.

- Remove plugins class - Route signals to run Nexus API call from MainWindow - Pass Dialog instance to route response data - Revert CategoryFactory::instance to return reference

I think VS did this when I tried to use the GUI

- Accidentally added to this branch

Holt59

I made some minor comments, most of which are cleaning of old for loop style, otherwise looks good to me.

Holt59 · 2023-09-24T06:54:46Z

src/categories.cpp

-  // 28 =
-  // 43 = Savegames (makes no sense to install them through MO)
-  // 45 = Videos and trailers
-  // 87 = Miscelanous
  addCategory(0, "None", std::vector<NexusCategory>(), 0);
 }

 void CategoryFactory::setParents()
 {
  for (std::vector<Category>::iterator iter = m_Categories.begin();


Should be changed to a C++11 for-loop.

Holt59 · 2023-09-24T06:55:02Z

src/categories.cpp

  addCategory(0, "None", std::vector<NexusCategory>(), 0);
 }

 void CategoryFactory::setParents()
 {
  for (std::vector<Category>::iterator iter = m_Categories.begin();
       iter != m_Categories.end(); ++iter) {
-    iter->m_HasChildren = false;
+    iter->setHasChildren(false);
  }

  for (std::vector<Category>::const_iterator categoryIter = m_Categories.begin();


Should be changed to a C++11 for-loop.

Holt59 · 2023-09-24T06:55:10Z

src/categories.cpp

@@ -201,15 +197,15 @@ void CategoryFactory::saveCategories()
  categoryFile.resize(0);
  for (std::vector<Category>::const_iterator iter = m_Categories.begin();


Should be changed to a C++11 for-loop.

Holt59 · 2023-09-24T06:55:33Z

src/categories.cpp

@@ -225,9 +221,9 @@ void CategoryFactory::saveCategories()
  nexusMapFile.resize(0);
  for (auto iter = m_NexusMap.begin(); iter != m_NexusMap.end(); ++iter) {


Should be changed to a C++11/17 for-loop.

Holt59 · 2023-09-24T06:56:07Z

src/categories.cpp

@@ -274,8 +270,8 @@ void CategoryFactory::addCategory(int id, const QString& name,
                                  int parentID)
 {
  for (auto nexusCat : nexusCats) {


Should be const auto& nexusCat, otherwise you are making a copy and I don't think that's wanted here.

Holt59 · 2023-09-24T06:56:53Z

src/categories.cpp

@@ -285,14 +281,18 @@ void CategoryFactory::addCategory(int id, const QString& name,
 void CategoryFactory::setNexusCategories(
    std::vector<CategoryFactory::NexusCategory>& nexusCats)
 {
-  m_NexusMap.empty();
  for (auto nexusCat : nexusCats) {


Should be const auto& nexusCat, otherwise you are making a copy and I don't think that's wanted here.

Holt59 · 2023-09-24T06:57:15Z

src/categories.cpp

@@ -285,14 +281,18 @@ void CategoryFactory::addCategory(int id, const QString& name,
 void CategoryFactory::setNexusCategories(
    std::vector<CategoryFactory::NexusCategory>& nexusCats)


This should probably be taken by const-reference I think.

Holt59 · 2023-09-24T06:59:28Z

src/categoriesdialog.cpp

@@ -211,22 +210,22 @@ void CategoriesDialog::fillTable()

  int row = 0;
  for (std::vector<CategoryFactory::Category>::const_iterator iter =


Change to a C++11 for loop and move the ++row, or at least use auto :)

For some reason when I change this loop, it's failing to load the Category data causing the Table to be blank when you initialize the dialog.

I'm probably doing something wrong somewhere, but I'm not sure what.

There is a ++row in this for loop so you need to move it somewhere if you change to a range-loop.

Yes, I did that. For some reason the element seemed to have uninitialized values when using the C++11 loop. I wasn't able to figure why the data wasn't loading properly.

Okay, don't code when it's pushing 4AM. I was putting the ++row at the front of the loop instead of the back.

I'll push the updated version shortly.

Holt59 · 2023-09-24T06:59:42Z

src/categoriesdialog.cpp

@@ -235,17 +234,17 @@ void CategoriesDialog::fillTable()
    table->setItem(row, 3, nexusCatItem.take());
  }

-  for (auto nexusCat : categories->m_NexusMap) {
+  for (auto nexusCat : categories.m_NexusMap) {


Should be const auto&.

Silarn · 2023-09-24T08:19:32Z

Understood. I think I can wrap those changes up before I head to bed.

* row increment needs to be before the logic but after the ID check/continue * avoid running context menu logic on null rows

Silarn force-pushed the qt6-category branch from fdee52b to 0c094a1 Compare September 21, 2023 01:53

Silarn requested review from Holt59 and Al12rs September 21, 2023 08:33

Silarn and others added 26 commits September 21, 2023 16:30

First pass for Qt6 compatibility

83f9af3

Add new API connection to retrieve game info

0b6afd6

WIP: Initial changes to fetch nexus categories

4fd45b9

WIP: Category QObj refactor

127db77

WIP: Refactor cat dialog and implement nex cat updates

f764271

WIP: Fix data storage and fix loading

2f7478f

Implement drag & drop nexus cat assignment

375b8e6

Fix rebase issue

6016b04

Refactoring for upstream merge

69f953a

Add menu item to auto-assign categories based on nexus assignments

5356236

Allow remapping category from context menu

b9f0567

Fix issues with category reset and NX save

3243101

Category setup dialog for new instances

8f31a1c

Don't overwrite existing Nexus category mappings

9a801f6

Add reminder dialog for old instances

eeddc99

C++20 fixes

05edcc8

Preserve old category if Nexus category can't be mapped

fba3d52

Fix merge issues

41c4d09

Fix archive parsing and BSA invalidation

6dfe55f

Clang cleanup

f6bb73d

Add nexus category ID to mod info

c50ad58

- We will check the mod info first then fall back to the download file - Add category field in the info dialog nexus tab

Make the category table sortable

59bc9e3

Cleanup

f8b0152

Dialogs and bugfixes

049a7e2

Category dialog tweaks for clarity

e18452e

Silarn added 3 commits September 21, 2023 17:35

Update new instance / migration dialogs

639fc78

Rework download category parsing

133c072

- Bypasses issue where hidden downloads are not accessible - getCategoryID no longer necessary

Fix rebase issues

d2e48ed

Silarn force-pushed the qt6-category branch from 4b459c3 to d2e48ed Compare September 21, 2023 22:43

Silarn added 3 commits September 21, 2023 21:04

Add new dialog for category import

7c900bb

- Merge or replace strategies - Nexus mappings are optional - If mappings are applied, remapping is optional

Add UI file

aff3e09

Reassign filter group for category classes

31a85a3

Holt59 requested changes Sep 23, 2023

View reviewed changes

Silarn added 5 commits September 23, 2023 19:00

Restructure category refresh action

e22331f

- Remove plugins class - Route signals to run Nexus API call from MainWindow - Pass Dialog instance to route response data - Revert CategoryFactory::instance to return reference

Revert version.rc

a67a919

I think VS did this when I tried to use the GUI

Revert TS

b7d4d20

Make category struct attributes private

efdff42

Remove extraneous function

5d67b8d

Silarn requested a review from Holt59 September 24, 2023 02:25

Silarn added 3 commits September 23, 2023 21:35

Unnecessary class definition

5f14c57

Remove download refresh scoped blocker

4bd860a

- Accidentally added to this branch

Remove unnecessary plugincontainer from settings

11522cf

Silarn force-pushed the qt6-category branch from b7f2c36 to 11522cf Compare September 24, 2023 03:24

Holt59 approved these changes Sep 24, 2023

View reviewed changes

Convert for loops, move nexusCats

c30b518

Silarn force-pushed the qt6-category branch from c97a15e to c30b518 Compare September 24, 2023 10:54

Silarn added 2 commits September 24, 2023 14:36

Use c++11 for loop

cdc16df

Fixes for table

74668b9

* row increment needs to be before the logic but after the ID check/continue * avoid running context menu logic on null rows

Silarn merged commit 79d457f into master Sep 25, 2023
3 checks passed

Silarn deleted the qt6-category branch September 25, 2023 22:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Category Revamp #1880

WIP: Category Revamp #1880

Silarn commented Sep 19, 2023

Silarn commented Sep 21, 2023

Holt59 left a comment

Holt59 Sep 23, 2023

Holt59 Sep 23, 2023

Silarn Sep 24, 2023

Holt59 Sep 23, 2023

Holt59 Sep 23, 2023

Silarn Sep 24, 2023

Holt59 Sep 23, 2023

Silarn Sep 24, 2023

Holt59 Sep 24, 2023

Holt59 Sep 23, 2023

Silarn Sep 24, 2023

Holt59 Sep 23, 2023

Holt59 Sep 23, 2023

Holt59 Sep 23, 2023

Holt59 Sep 23, 2023

Silarn commented Sep 23, 2023

Holt59 left a comment

Holt59 Sep 24, 2023

Holt59 Sep 24, 2023

Holt59 Sep 24, 2023

Holt59 Sep 24, 2023

Holt59 Sep 24, 2023

Holt59 Sep 24, 2023

Holt59 Sep 24, 2023

Holt59 Sep 24, 2023

Silarn Sep 24, 2023

Holt59 Sep 24, 2023

Silarn Sep 24, 2023

Silarn Sep 24, 2023

Holt59 Sep 24, 2023

Silarn commented Sep 24, 2023

		@@ -201,15 +197,15 @@ void CategoryFactory::saveCategories()
		categoryFile.resize(0);
		for (std::vector<Category>::const_iterator iter = m_Categories.begin();

		@@ -225,9 +221,9 @@ void CategoryFactory::saveCategories()
		nexusMapFile.resize(0);
		for (auto iter = m_NexusMap.begin(); iter != m_NexusMap.end(); ++iter) {

		@@ -285,14 +281,18 @@ void CategoryFactory::addCategory(int id, const QString& name,
		void CategoryFactory::setNexusCategories(
		std::vector<CategoryFactory::NexusCategory>& nexusCats)

		@@ -211,22 +210,22 @@ void CategoriesDialog::fillTable()

		int row = 0;
		for (std::vector<CategoryFactory::Category>::const_iterator iter =


		VS_VERSION_INFO VERSIONINFO

WIP: Category Revamp #1880

WIP: Category Revamp #1880

Conversation

Silarn commented Sep 19, 2023

Silarn commented Sep 21, 2023

Holt59 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Silarn commented Sep 23, 2023

Holt59 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Silarn commented Sep 24, 2023