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

Offer ContentDB updates for leftover bundled Minetest Game #13906

Merged
merged 4 commits into from Nov 5, 2023

Conversation

grorp
Copy link
Member

@grorp grorp commented Oct 21, 2023

Resolves item 3 from #13818 (comment).

Windows users apparently update Minetest by extracting the new ZIP file over their existing installation. This means that Minetest Game will remain installed for them. Since the bundled Minetest Game has no "release" field in its game.conf, it won't receive updates from ContentDB and Windows users will stay on MTG 5.7.0 forever.

This PR fixes that by considering any installation of MTG that is not versioned (i.e. no "release" field), has not been cloned from Git, and is not system-wide to be updatable.

To do

This PR is a Ready for Review.

How to test

Install Minetest Game from ContentDB. Remove the "release" field from game.conf and verify that you are offered an update in the "Browse online content" dialog.

@erlehmann
Copy link
Contributor

@grorp also consider that Minetest game from Minetest 5.4.1 will crash at startup with Minetest 5.8. Newer versions do not crash at startup, but maybe it may make sense to handle crashes of unversioned minetest_game specially in case they have issues?

@rubenwardy rubenwardy modified the milestones: 5.8.0, 5.9.0 Oct 27, 2023
Copy link
Member

@srifqi srifqi left a comment

Choose a reason for hiding this comment

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

This PR looks good to me.

@grorp
Copy link
Member Author

grorp commented Oct 28, 2023

This will need changes after #13807 is merged.

@grorp grorp added the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Oct 28, 2023
@grorp
Copy link
Member Author

grorp commented Oct 28, 2023

also consider that Minetest game from Minetest 5.4.1 will crash at startup with Minetest 5.8. Newer versions do not crash at startup, but maybe it may make sense to handle crashes of unversioned minetest_game specially in case they have issues?

I think pushing users towards updating their packages is enough for now (see #13807). Old versions of other games probably have similar problems. (Also see #13799.)

@grorp grorp force-pushed the leftover-bundled-mtg-offer-updates branch from 502b81b to fbd58ec Compare October 28, 2023 19:36
Avoids need for special handling of optional "_game" suffix
@grorp
Copy link
Member Author

grorp commented Oct 28, 2023

https://irc.minetest.net/minetest-dev/2023-10-28#i_6126271

I haven't implemented the "backup MTG before updating" thing suggested on IRC, as I'm not sure how to do this best. Where do we want to put the backup? I think it shouldn't show up in the game list. The backup would only be something for experienced users (modders) who have modified their local MTG installation.

Also, I'm not sure if a backup is at all needed. After all, every Minetest update before 5.8.0 also overwrote Minetest Game, wiping any modifications made by the user.

@grorp grorp removed the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Oct 28, 2023
@grorp grorp requested a review from rubenwardy October 28, 2023 20:28
@erlehmann
Copy link
Contributor

Where do we want to put the backup?

In the games folder or in the folder above it I guess.

I think it shouldn't show up in the game list.

Or maybe it should, as “Minetest Game (Legacy)”? This was suggested on IRC and I currently have no opinion on it.

Do all folders in the games folder show up in the game list? If not, what would neet to be done so it does not happen?

Also, I'm not sure if a backup is at all needed. After all, every Minetest update before 5.8.0 also overwrote Minetest Game, wiping any modifications made by the user.

I am pretty sure that extracting a zip file does not overwrite everything, just files that are in a zip archive. Meanwhile, I think the update of a mod deletes the entire folder, or not?

Btw, anecdotally I can say that players who have little idea how this stuff should work actually do save files for texture sets they are making in the minetest_game folder or copy new mods into it. But since much stronger evidence did not pass through the “I'm pretty sure nothing bad will happen” filter in the past, that's not really an argument.

@sfan5
Copy link
Member

sfan5 commented Oct 29, 2023

I haven't implemented the "backup MTG before updating" thing suggested on IRC, as I'm not sure how to do this best.

We don't need this IMO.

@rubenwardy
Copy link
Member

Other than that, code looks good and works as expected

@grorp grorp merged commit adec167 into minetest:master Nov 5, 2023
2 checks passed
grorp added a commit to grorp/minetest that referenced this pull request Nov 8, 2023
grorp added a commit that referenced this pull request Nov 9, 2023
cx384 pushed a commit to cx384/minetest that referenced this pull request Dec 9, 2023
cx384 pushed a commit to cx384/minetest that referenced this pull request Dec 9, 2023
@grorp grorp deleted the leftover-bundled-mtg-offer-updates branch December 18, 2023 16:40
kawogi pushed a commit to kawogi/minetest that referenced this pull request Dec 19, 2023
kawogi pushed a commit to kawogi/minetest that referenced this pull request Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants