Skip to content

Conversation

@aecsocket
Copy link
Contributor

Cause:

  • Dynamic Lights has the following versions:
    • Ag9747sk for Forge with file hash f02d2bcb995ee5531555995d6625571bc7f3518fa3f5e45bba230d08cd89a608
    • Yyh6uR59 for Forge + NF with file hash f02d2bcb995ee5531555995d6625571bc7f3518fa3f5e45bba230d08cd89a608
    • Two versions, but the file hashes are the same
  • User downloads Dynamic Lights version Ag9747sk, downloading the f02d… file hash
  • When we scan the instance mods directory to see what mods are installed, we find f02d…
  • We ask the backend to see what version this file hash corresponds to - it tells us Yyuh6uR59
  • We ask the backend to see what updates are available - Ag9747sk is available
  • Therefore, we always update to the "new version", but it's actually the same version, and app gets confused

Solution:

  • When app requests version info for a file hash, return all versions associated with it, not just the last one inserted into the map
  • When setting the update_version_id, iterate through all versions, not just the first one we find()

@aecsocket aecsocket marked this pull request as draft January 16, 2026 12:08
@aecsocket aecsocket marked this pull request as ready for review January 19, 2026 16:14
@fetchfern fetchfern self-requested a review January 19, 2026 17:59
Copy link
Contributor

@fetchfern fetchfern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would maybe emphasize on the fact duplicate files are a rare edge case but otherwise LGTM

@aecsocket
Copy link
Contributor Author

Would maybe emphasize on the fact duplicate files are a rare edge case but otherwise LGTM

IMO rarity of this doesn't really matter, if it's a possible edge case then it should be treated as important as any other path.

But since we are changing the assumption that 1 hash = 1 version, I'm not confident that this is the only place where we have this assumption. There might be more places where we assume this, and will need to change later.

Should fix #4250 though.

@aecsocket aecsocket added this pull request to the merge queue Jan 19, 2026
Merged via the queue into main with commit c94dde9 Jan 19, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants