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

another fix for accessing mod details struct + querying tags count #103

Merged
merged 8 commits into from
Dec 1, 2024

Conversation

otavepto
Copy link
Contributor

  • fix mod details struct access again! (I think this will be a recurring problem until we catch every access, sorry :/ )
  • fix querying mod tags count
  • fix mod preview file URI
  • copy missing mod min/max branches (genuinely have not the slightest intuition what this is until now)
  • use different timing for mod update/create/add dates
  • respect mod owner when copying mod details

partial fix for #98

* fix querying mod tags count
* fix mod preview file URI
* copy missing mod min/max branches
* use different timing for mod update/create/add dates
@otavepto
Copy link
Contributor Author

otavepto commented Nov 26, 2024

ping @Edremon I'd appreciate if you test your patch on this PR before merging it.

@Edremon
Copy link
Contributor

Edremon commented Nov 26, 2024

If tags are not specified in mods.json this crash with assertion failed (on tmp.back()).

@Detanup01
Copy link
Owner

Will wait until you says it working

dll/settings_parser.cpp Outdated Show resolved Hide resolved
Comment on lines 500 to 499

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you drop that change? Would make a useless conflict.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

on my end I don't see the changes nor git detects it since I use LF for everything, can't fix, don't even know what the original line ending was
inside this mess I removed 2 duplicate lines though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh wait I'm an idiot, one moment

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a bit annoying because at least locally, it causes a merge conflict when applying #104. It's not a line ending, but some trailing whitespace that got removed. Most files are full of them, and editors will remove them automatically.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah fix already, check above comment

@otavepto otavepto mentioned this pull request Nov 27, 2024
@Edremon
Copy link
Contributor

Edremon commented Nov 28, 2024

I don't think this preview url change anything for Linux, you can have file:///////home and that will still resolves to home.

@otavepto
Copy link
Contributor Author

even if that worked it's better to stick to the standard RFC 8089/3986 in case this is needed later.

@Detanup01 Detanup01 merged commit 5ec0435 into Detanup01:dev Dec 1, 2024
62 checks passed
@otavepto otavepto deleted the patch-mods-access-tags-count-fix branch December 1, 2024 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants