Skip to content
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.

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

actions.updateCategories/update-categories event clobbers existing categories #6594

Open
agc93 opened this issue Jun 18, 2020 · 2 comments
Open
Labels
improvement 📈 An issue that improves an existing feature

Comments

@agc93
Copy link
Contributor

agc93 commented Jun 18, 2020

Describe the bug

As always, this could be expected behaviour for a very good reason! I'm just stumbling around in the categories code and noticed something weird.

Emitting an update-categories event with an ICategoryDictionary parameter results in the category_management extension dispatching the updateCategories action. The reducer for that action just overwrites any existing categories in the persistent state. As far as I can tell, this means that it's impossible for a specific game to have categories populated from more than one source.

This same behaviour also means that an extension using retrieve-category-list to populate initial categories (I know it's supposed to be internal, but hypothetically) would completely replace the categories from Nexus Mods provided by nexus_integration

To Reproduce
Steps to reproduce the behavior:

  1. Use the Categories dialog to pull Nexus Mods categories
  2. Use a toolbar button/global action/whatever to emit update-categories with valid params
  3. The categories from the event will completely replace the Nexus categories

Expected behavior

So I might be expecting this wrong, but I would have guessed that this should merge the categories into state, rather than replacing them all.

I understand that it's possible to just use the setCategory action to specifically add individual categories, but update-categories/retrieve-category-list seems like it should be the place for adding multiple categories.

Platform (please complete the following information):

  • OS: Windows 10
  • Vortex Version 1.2.16

Additional context

As always, feel free to tell me I'm doing it wrong :)

@TanninOne
Copy link
Contributor

This is indeed the expected behavior atm.
If you want to keep existing categories you can read what's currently there, merge with your new categories and then update-categories the merged dictionary.

retrieve-category-list will indeed replace everything already there but it's supposed to be internal and our UI for triggering it specifically says that it will replace all existing categories.

The problem is that if the api didn't replace but add categories, we would need a more complex api because then we'd have to keep track of where the categories came from so you can then "remove only categories from nexus mods"
Tbh. I'm not sure that's worth it, I wouldn't expect users to constantly sync their categories with multiple sites.

@TanninOne TanninOne added the improvement 📈 An issue that improves an existing feature label Jun 19, 2020
@agc93
Copy link
Contributor Author

agc93 commented Jun 19, 2020

Cool, that does make some sense.

I was hoping to be able to add categories during the initial (essentially one-time) population that retrieve-category-list does, but I think it might be easier to use the the same gamemode-activated logic that the category extension uses. Then I can be sure that the Nexus integration has already run and hopefully merge them. That way I can also be sure that the Nexus implementation won't clobber my extension's categories.

Understandable if this ends up being too hard/complex, was just trying to avoid too much dirty hackery :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement 📈 An issue that improves an existing feature
Projects
None yet
Development

No branches or pull requests

2 participants