grrttedwards / wow-addon-updater

Python utility for updating World of Warcraft addons
GNU General Public License v3.0
71 stars 14 forks source link

Classic only addons returned with `GameVersion.retail` #151

Closed amas0 closed 4 years ago

amas0 commented 4 years ago

When specifying a classic-only addon, that version will be returned regardless of the specified game version. See below code:

from updater.site.curse import Curse
from updater.site.enum import GameVersion

c = Curse('https://www.curseforge.com/wow/addons/classiccodex', GameVersion.retail)
print(c.get_latest_version())  # prints v1.3.9-classic

c = Curse('https://www.curseforge.com/wow/addons/classiccodex', GameVersion.classic)
print(c.get_latest_version())  # prints v1.3.9-classic

This is caused by the logic in get_latest_version (and find_zip_url) looking for multiple versions and assuming the first one is the retail release. There is fallback logic that falls back on the "retail" version when it cannot find a "classic" version. So what is happening in the above example is:

1) The updater finds the addon page and searches for recent releases 2) It finds 1 release, which it assumes to be the retail version but is actually for classic 3) When asking for the GameVersion.retail version this gets returned immediately since it was the first version found 4) When asking for the GameVersion.classic version, it finds no "classic" version and instead falls back on the "retail" release, which is the real classic release.

I'd expect some sort of error to be thrown when this occurs. Additionally, this specific example that I used is contained within the testing code of test_curse.py.

I would like to propose modifying this behavior so that errors are thrown when trying to retrieve an addon for a version of the game that doesn't exist.

I stumbled upon this while working on https://github.com/grrttedwards/wow-addon-updater/issues/149#issuecomment-686656728 and would be happy to address this as part of that work.

grrttedwards commented 4 years ago

Nice catch 😅 You’ve found one of the “unintentional features“

There’s a bit of activity in this repo which I’ve put on my personal backlog to review, so I’m happy to review and merge any improvements if you’re willing to post some PRs!

grrttedwards commented 4 years ago

Fixed in #153