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

Content download update downloads all versions, not just required/latest #7679

Open
Berbe opened this issue Aug 3, 2019 · 5 comments
Open

Content download update downloads all versions, not just required/latest #7679

Berbe opened this issue Aug 3, 2019 · 5 comments

Comments

@Berbe
Copy link
Contributor

@Berbe Berbe commented Aug 3, 2019

Version of OpenTTD

1.9.2

Expected result

No upgrade offered, as the scripts library import docs states:

The version check is very important. If you expect version 1, but on some users computer the library is in version 2, your AI will refuse to load.

Actual result

Multiple versions (with lower & higher numbers) of a downloaded package are offered as upgrades. Those versions are not required by any downloaded package.

Steps to reproduce

  1. Clean content_download/ai directory & library subdirectory
  2. Through the in-game content downloader:
    1. Download AIAI: SuperLib v39 whill be installed as a dependency (as expected)
    2. Re-enter content downloader, notice upgrades are available and select them all.
      All SuperLib versions will be offered as upgrades (none of them should be)
@LordAro
Copy link
Member

@LordAro LordAro commented Aug 3, 2019

I believe this is actually intended behaviour

if (proc(ci, false)) ci->upgrade = true;

Checks that the ContentInfo ci that's been downloaded matches (inexactly) with anything on the system already by ID/md5sum

This of course makes sense, as version numbers for content are purely informational - there's nothing enforcing any particular version scheme, or any particular order (AIAI uses the Greek alphabet, for instance)

I'm not sure why loads of duplicates for BaseSets don't show up though - they do the same checks as all other content types

@Berbe
Copy link
Contributor Author

@Berbe Berbe commented Aug 3, 2019

I edited the issue to quote & link the docs on the Wiki.

If the version is important, as stated (and as I trust it shall be), then I saw nowhere in the code anything implementing that.
Would that break anything if implemented, apart from scripts having loosely numbered dependencies( version requirements?

Berbe added a commit to Berbe/OpenTTD that referenced this issue Aug 3, 2019
@LordAro LordAro changed the title Content downloader brings unrequired "upgrades" up Content download update downloads all versions, not just required/latest Oct 19, 2019
@TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Sep 26, 2020

So basically what you are saying: libraries should not be upgraded independent. That sounds sane to me. So to me it seems the line @LordAro pointed out should be conditioned with: if not library.

What I am not sure of, what if someone wants to download the latest version of a library. I am not sure if the upgrade flag indicates it is upgradable, or selects it for download. As we only want to influence the latter, in my opinion, not the first.

@Berbe
Copy link
Contributor Author

@Berbe Berbe commented Sep 26, 2020

What I am not sure of, what if someone wants to download the latest version of a library. I am not sure if the upgrade flag indicates it is upgradable, or selects it for download.

I am unsure either. It is a bit out of the scope of this issue, though, in which I am offered version I never asked for (the system tells me I need them).

It is indeed sane to allow to manage multiple versions of the same library independently, as per requirements, you might need to have multiple versions of a same library for different modules.

It all boils down to how your dependencies management operates:
Does it make sense to offer upgrades for items never having been chosen for download? Here is an example covering several cases/decisions related to core mechanisms of dependency/package management:

  1. Download A v1, which depends on B v1 & C v1
  2. A updates to v2, requiring B v2 & C v1 -> A v2 & B v2 are selected for upgrade (what happens to A v1 & B v1: kept, even if nothing else depends on it? deleted?)
  3. C updates to v2 -> Nothing happens on your hand (no upgrade offered)
  4. You decided you want to download C v2: it is downloaded on the side of C v1, and its upgrades are now tracked
  5. C updates to v3 -> upgrade offered
  6. You delete A (v2): what happens to B v2 & C v1: kept, even if nothing else depends on it? deleted?

@TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Dec 27, 2020

I did some more thinking on this topic, and the scenario you describe holds for all our content, not only for AIs. Scenarios can also have dependencies, heck, even NewGRFs can depend on other NewGRFs.

To boil it down, what you suggest, is to track what content was manually downloaded and what came as a dependency. Upgrade the manual ones, and not the ones that came automatically.

Although I get where this question comes from, I do have to wonder if the added complexity of tracking this information adds value to the game. And that brings me to the thought that the interface in general is just a bit poorly designed for the amount of downloads we offer these days. So if we would be to tackle this problem, I suggest we take a bit broader view of the problem, and fix several related issues. For example, it is nearly impossible to see what you will be downloading when you select a single piece of content.

Mostly I wrote this down as this conclusion means that #7680 does not resolve the problem in a sufficient matter. I am unsure yet how we could best proceed .. a revamp of the content service part would be ideal, but I also understand that this would be a massive amount of work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants