Add package update detection on Content tab #13807

rubenwardy · 2023-09-14T19:16:18Z

Fixes #7327 and fixes #11799

Minetest now shows available updates for content without opening the ContentDB dialog. This uses the new updates API, which is quite fast to load (35ms + network latency, without cache)

To do

This PR is Ready for Review

How to test

Install some content from the ContentDB dialog
Edit the .conf file to reduce the release number
Change tab to Start Game and Restart Minetest
See a number on the content tab
Change tab to Content and see packages with update icons
Restart Minetest, see content tab. Number and icon should appear shortly

builtin/mainmenu/content/contentdb_api.lua

grorp

A high-level review:

I'm not sure what I think about the "update available" icons. When I see one of these icons in the package list, I think that you can update the package when you select it, or that clicking on the icon will update the package. But actually you have to click on "Browse online content" before you can download any updates.
There is too much text on the "Browse online content" button. With the default scaling settings, it looks like this on my Android device:

The text overflows, but it's still understandable. But if you translate it into German, the text overflows to an extent that it becomes unintelligible:

I suggest reducing the label to Browse online content ($1).
To make it more clear that $1 in Content ($1), Browse online content ($1) and Update All [$1] refers to the same number, I suggest using the same kind of brackets in all three places.
This is a very useful feature btw.

rubenwardy · 2023-09-23T16:57:12Z

I'm not sure what I think about the "update available" icons. When I see one of these icons in the package list, I think that you can update the package when you select it, or that clicking on the icon will update the package. But actually you have to click on "Browse online content" before you can download any updates.

I wondered the same

Options:

Change the icon
Remove the icon (I think it's useful to know which packages before opening the dialog)
Add an [Update] button when selecting a package that opens the ContentDB dialog and starts the update. This actually wouldn't be too hard, you'd pass in an additional argument that is checked after the package list loads. This argument could be used in future for Support minetest:// custom browser protocol #7400

Although, the Content tab will probably be removed in the future - perhaps in 5.9.0 - and replaced by a unified content and ContentDB dialog, so maybe not the best idea to do extra work here

grorp · 2023-09-27T20:54:06Z

Add an [Update] button when selecting a package that opens the ContentDB dialog and starts the update. This actually wouldn't be too hard, you'd pass in an additional argument that is checked after the package list loads. This argument could be used in future for Support minetest:// custom browser protocol #7400

Since #13850 implements the additional argument to create_store_dlg you mentioned, option 3 should be really easy to implement after that PR, so I'm in favor of option 3.

ancientmarinerdev · 2023-09-28T02:48:57Z

A comment from another PR that is probably more relevant here:

"do you have any plans in the future to give notifications if there are games that need updating? Perhaps when you're on the page to play that game "This game has an update available for it", and maybe a notification if you try to play without updating it or mods selected for that world would be good UX so you get that information as you need it. "The world you have chosen has the following updates: ..." and "Update" or "Play anyway" as options."

#13850 (comment)

rubenwardy · 2023-09-28T08:36:18Z

That's something for the future, after this PR is merged

ancientmarinerdev · 2023-09-28T23:45:26Z

That's something for the future, after this PR is merged

Understandable. Thanks for considering it.

Zughy · 2023-10-02T16:42:34Z

@rubenwardy rebase needed

rubenwardy · 2023-10-08T15:57:33Z

There's now an update button. Need a better place to put it, as there won't be enough space for the text / etc on smaller screens and different languages

Perhaps the description could be made shorter, and the button placed there above the existing buttons

grorp

Perhaps the description could be made shorter, and the button placed there above the existing buttons

That's a good idea, but I'd prefer this layout: grorp@9cb47b8

It gives more space to the "Use Texture Pack" button. On the master branch, the text on that button overflows on my Android device.

EDIT: BTW, it looks like you didn't take the modpack "Rename" button into account for the current button arrangement. I'd be open to removing that button altogether, but of course that's not for this PR.

builtin/mainmenu/content/update_detector.lua

builtin/mainmenu/tab_content.lua

builtin/mainmenu/content/pkgmgr.lua

builtin/mainmenu/content/update_detector.lua

builtin/mainmenu/tab_content.lua

builtin/mainmenu/content/pkgmgr.lua

Fixes minetest#7327

srifqi

This does not work for minetest_game (Minetest Game) or any game that ends with _game because the IDs do not match.

minetest/src/content/subgames.cpp

Lines 227 to 233 in ddce858

    
           // Add it to result 
        
           const char *ends[] = {"_game", NULL}; 
        
           std::string shorter = removeStringEnd(dln.name, ends); 
        
           if (!shorter.empty()) 
        
           	gameids.insert(shorter); 
        
           else 
        
           	gameids.insert(dln.name);

rubenwardy · 2023-10-27T20:17:32Z

Fixed

srifqi

This PR looks good to me.

grorp

You can start multiple, simultaneous downloads for the same package by

clicking "Update" (which opens the ContentDB dialog)
leaving the ContentDB dialog
and clicking "Update" again

A related issue is #11799, but I can't reproduce that one.

src/script/lua_api/l_util.cpp

grorp

Very nice!

rubenwardy added Feature ✨ PRs that add or enhance a feature @ Mainmenu @ Content / PkgMgr UI/UX labels Sep 14, 2023

SmallJoker added the Rebase needed The PR needs to be rebased by its author. label Sep 17, 2023

rubenwardy force-pushed the contentdb_update_detection branch from 8241799 to ff3c5dd Compare September 17, 2023 16:41

rubenwardy added Roadmap The change matches an item on the current roadmap. and removed Rebase needed The PR needs to be rebased by its author. labels Sep 17, 2023

sfan5 reviewed Sep 17, 2023

View reviewed changes

builtin/mainmenu/content/contentdb_api.lua Outdated Show resolved Hide resolved

sfan5 reviewed Sep 17, 2023

View reviewed changes

builtin/mainmenu/content/contentdb_api.lua Outdated Show resolved Hide resolved

rubenwardy force-pushed the contentdb_update_detection branch from ff3c5dd to f0a78ac Compare September 18, 2023 11:30

rubenwardy requested a review from sfan5 September 18, 2023 11:41

rubenwardy force-pushed the contentdb_update_detection branch 2 times, most recently from 9dc416f to 9ac023d Compare September 18, 2023 11:54

grorp reviewed Sep 23, 2023

View reviewed changes

rubenwardy force-pushed the contentdb_update_detection branch from 9ac023d to b002959 Compare September 26, 2023 18:52

sfan5 removed their request for review October 2, 2023 12:58

sfan5 added the Rebase needed The PR needs to be rebased by its author. label Oct 2, 2023

rubenwardy force-pushed the contentdb_update_detection branch from 4066c04 to fb1c8a7 Compare October 2, 2023 16:45

rubenwardy removed the Rebase needed The PR needs to be rebased by its author. label Oct 2, 2023

rubenwardy mentioned this pull request Oct 2, 2023

Notify users that they need to reinstall MTG if they still want it #13850

Merged

rubenwardy force-pushed the contentdb_update_detection branch from fb1c8a7 to c2e38bd Compare October 8, 2023 15:56

grorp requested changes Oct 9, 2023

View reviewed changes

rubenwardy and others added 2 commits October 27, 2023 10:44

Add package update detection on Content tab

221a0e2

Fixes minetest#7327

Re-arrange buttons

1a5bf04

rubenwardy added this to the 5.8.0 milestone Oct 27, 2023

Update according to review

8a850b2

rubenwardy force-pushed the contentdb_update_detection branch from c2e38bd to 8a850b2 Compare October 27, 2023 16:56

Zughy removed the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Oct 27, 2023

Use C++ urlencode

f38b68b

srifqi requested changes Oct 27, 2023

View reviewed changes

rubenwardy added 3 commits October 27, 2023 21:00

Fix code style

ce89a0c

Document minetest.urlencode

858be4f

Fix _game

cebba1e

srifqi approved these changes Oct 27, 2023

View reviewed changes

srifqi added the One approval ✅ ◻️ label Oct 27, 2023

rubenwardy requested a review from grorp October 27, 2023 21:53

grorp requested changes Oct 28, 2023

View reviewed changes

src/script/lua_api/l_util.cpp Outdated Show resolved Hide resolved

grorp mentioned this pull request Oct 28, 2023

Offer ContentDB updates for leftover bundled Minetest Game #13906

Merged

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

rubenwardy added 2 commits October 28, 2023 14:46

Fix double update

767cb70

Fix auto install with game and _game

e160806

rubenwardy removed the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Oct 28, 2023

Fix comment

d827858

rubenwardy requested a review from grorp October 28, 2023 14:52

grorp approved these changes Oct 28, 2023

View reviewed changes

grorp added >= Two approvals ✅ ✅ and removed One approval ✅ ◻️ labels Oct 28, 2023

rubenwardy merged commit 4ee32c5 into minetest:master Oct 28, 2023
15 checks passed

rubenwardy deleted the contentdb_update_detection branch October 28, 2023 16:33

cx384 pushed a commit to cx384/minetest that referenced this pull request Dec 9, 2023

Add package update detection on Content tab (minetest#13807)

98fea41

kawogi pushed a commit to kawogi/minetest that referenced this pull request Dec 19, 2023

Add package update detection on Content tab (minetest#13807)

df8624a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add package update detection on Content tab #13807

Add package update detection on Content tab #13807

rubenwardy commented Sep 14, 2023 •

edited

grorp left a comment •

edited

rubenwardy commented Sep 23, 2023 •

edited

grorp commented Sep 27, 2023

ancientmarinerdev commented Sep 28, 2023

rubenwardy commented Sep 28, 2023

ancientmarinerdev commented Sep 28, 2023

Zughy commented Oct 2, 2023

rubenwardy commented Oct 8, 2023 •

edited

grorp left a comment •

edited

srifqi left a comment

rubenwardy commented Oct 27, 2023

srifqi left a comment

grorp left a comment

grorp left a comment

	// Add it to result
	const char *ends[] = {"_game", NULL};
	std::string shorter = removeStringEnd(dln.name, ends);
	if (!shorter.empty())
	gameids.insert(shorter);
	else
	gameids.insert(dln.name);

Add package update detection on Content tab #13807

Add package update detection on Content tab #13807

Conversation

rubenwardy commented Sep 14, 2023 • edited

To do

How to test

grorp left a comment • edited

Choose a reason for hiding this comment

rubenwardy commented Sep 23, 2023 • edited

grorp commented Sep 27, 2023

ancientmarinerdev commented Sep 28, 2023

rubenwardy commented Sep 28, 2023

ancientmarinerdev commented Sep 28, 2023

Zughy commented Oct 2, 2023

rubenwardy commented Oct 8, 2023 • edited

grorp left a comment • edited

Choose a reason for hiding this comment

srifqi left a comment

Choose a reason for hiding this comment

rubenwardy commented Oct 27, 2023

srifqi left a comment

Choose a reason for hiding this comment

grorp left a comment

Choose a reason for hiding this comment

grorp left a comment

Choose a reason for hiding this comment

rubenwardy commented Sep 14, 2023 •

edited

grorp left a comment •

edited

rubenwardy commented Sep 23, 2023 •

edited

rubenwardy commented Oct 8, 2023 •

edited

grorp left a comment •

edited