Nexus-Mods / Vortex

Vortex Development
GNU General Public License v3.0
894 stars 131 forks source link

Extension upgrade throws errors for duplicate action names #6597

Open agc93 opened 4 years ago

agc93 commented 4 years ago

Describe the bug

Found another TODO bug that I've had a lot of trouble reliably recreating. Essentially, upgrades to a currently enabled extension sometimes results in an error dialog complaining about a duplicate action creation that I think is probably coming from this extension load during installation. Closing the dialog and restarting Vortex manually will sometimes work fine, but sometimes results in two versions of the extension appearing in the extensions list (depending on how the old version was installed).

I think this is partially impacted by how the previous version was installed since just manipulating the info.json of a "properly" installed extension to trigger an update doesn't trigger this bug. However, if you manually install a ZIP version of the extension (even downloaded from Nexus), it doesn't seem to recognise it as the same extension and will allow you to install, rather than upgrade, the latest version from Nexus Mods, triggering this error.

To Reproduce Steps to reproduce the behavior:

  1. Install an extension that creates an action (Beat Saber Support 0.3.1 would do)
  2. Attempt to update that extension
  3. You should see the error message complaining about a duplicate action (BS_ENABLE_OCI if testing with Beat Saber)

Expected behavior

Platform (please complete the following information):

Additional context

Like I said, this is sorta tricky to replicate even in a clean environment, especially without the ability to install older versions through the Extensions pane, and with how Vortex doesn't seem to match manually installed extensions without Nexus-installed versions of the same extension.

TanninOne commented 4 years ago

Yes, Vortex can't correctly determine two extensions as the same if the original one was installed manually. This is because the ID is (usually) assigned during installation, which at least ensures that extensions installed through the integrated mechanism are correctly recognized. The alternative (which is currently optional) is for the extension author to assign the id in info.json but then we rely on the author to never change that id because if they do, even the integrated mechanism will not update it correctly.

The mechanism you refer to, installing extension dependencies automatically, is currently only run when no previous version is installed because of this problem with duplicate actions. Hence I don't think you would be able to reproduce it when the old version is installed through the extensions page, only if you install it manually and then install it again through the extensions page.

agc93 commented 4 years ago

That makes sense, and explains why I was having trouble getting it to reproduce.

That's a good point about a change in id "breaking" automatic updates which dramatically complicates https://github.com/Nexus-Mods/vortex-api/pull/3 as well. I was actually thinking of manually setting the id but definitely not going to do that now 😆

TanninOne commented 2 years ago

Vortex is no longer reporting duplicated actions as errors (hasn't been for a while) so I would say this is mostly "solved" as far as users are concerned.

However: all we're doing is suppress the error. The reason we're loading the extension during installation is so we can identify extension dependencies right away so that if an extension contains "context.requireExtension" and that required extension is missing, Vortex installs it too in the same session before asking the user to restart.

Now this will only happen when initially installing the extension. If an update introduces a new dependency while also declaring actions, Vortex will not be able to evaluate the dependencies and require another restart with the extension failing to load after the first.

This is fairly hypothetical atm, context.requireExtension is barely being used.