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

Fix translation updater script: Handle nested modpacks #14340

Merged
merged 2 commits into from Feb 5, 2024

Conversation

appgurueu
Copy link
Contributor

Trivial fix for #14281, #14282: update_mod -> update_folder; modpacks can contain other modpacks. Also added a warning if what's supposed to be a mod folder is missing init.lua.

How to test

See the issues.

@appgurueu appgurueu added Trivial The change is a trivial bug fix, documentation or maintenance change, as per the Git Guidelines Bugfix 🐛 PRs that fix a bug labels Feb 3, 2024
@SmallJoker
Copy link
Member

I think it would make sense to also treat directories named "mods" where ../game.conf is present as modpacks. That would allow proper translation updates of games (in respect to my other code comment).

@appgurueu
Copy link
Contributor Author

That would allow proper translation updates of games (in respect to my other code comment).

With the current state of this PR, ./mod_translation_updater.py -r path/to/game/mods works as expected. Would you like me to make ./mod_translation_updater.py -r path/to/game work too?

@SmallJoker
Copy link
Member

Would you like me to make ./mod_translation_updater.py -r path/to/game work too?

Yes, I think for modders it's reasonable to assume that this would work too, hence I'd definitely support such functionality.

@appgurueu
Copy link
Contributor Author

Would you like me to make ./mod_translation_updater.py -r path/to/game work too?

Yes, I think for modders it's reasonable to assume that this would work too, hence I'd definitely support such functionality.

Okay, added support for ./mod_translation_updater.py path/to/game (the -r didn't make sense much sense with our current definition of -r - which I don't want to change - there, my bad).

Copy link
Member

@SmallJoker SmallJoker left a comment

Choose a reason for hiding this comment

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

Works on my machine. Thank you for the changes.

@appgurueu appgurueu merged commit 4859cf4 into minetest:master Feb 5, 2024
@appgurueu appgurueu deleted the fix-mod-trans-upd branch February 5, 2024 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bugfix 🐛 PRs that fix a bug One approval ✅ ◻️ @ Startup / Config / Util Trivial The change is a trivial bug fix, documentation or maintenance change, as per the Git Guidelines
Projects
None yet
3 participants