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

Allow games (just MTG?) to enforce lock-step upgrades with versions #13799

Closed
sfan5 opened this issue Sep 12, 2023 · 10 comments
Closed

Allow games (just MTG?) to enforce lock-step upgrades with versions #13799

sfan5 opened this issue Sep 12, 2023 · 10 comments
Labels
@ Content / PkgMgr Feature request Issues that request the addition or enhancement of a feature @ Mainmenu

Comments

@sfan5
Copy link
Member

sfan5 commented Sep 12, 2023

Situation: The user now has MTG installed locally. What happens in the future when they update the engine and MTG is still at a version that is not compatible?
There needs to be an ideally offline way of detecting this (game.conf?) and another very obvious flow that leads the user to update MTG before they open their worlds.
(Identically applies to any other game with strict version compat)

Originally posted by @sfan5 in #13550 (comment)

@sfan5 sfan5 added Feature request Issues that request the addition or enhancement of a feature @ Mainmenu labels Sep 12, 2023
@erlehmann
Copy link
Contributor

The user now has MTG installed locally. What happens in the future when they update the engine and MTG is still at a version that is not compatible?

In what ways could a game like MTG be incompatible with a newer engine?

@rubenwardy
Copy link
Member

rubenwardy commented Sep 12, 2023

Adapted from https://irc.minetest.net/minetest-dev/2023-09-12#i_6112683:

This is one of the things on my to do list.

min and max_minetest_version

ContentDB allows you to do min_minetest_version = 5.8 in game.conf to do version limits, would be nice to reuse that. See https://content.minetest.net/help/package_config/

So for Minetest Game you'd do min_minetest_version = 5.8 and max_minetest_version = 5.9-dev I guess? The -dev there is a bit messy

Update detection without entering CDB

Instead of setting a max version, another option would be to only set min_minetest_version = 5.8 and have support for update detection without entering CDB. You could design it so that users are coaxed into updating before starting the game, or you could have support for auto-updating by default (this may be problematic if the user quickly joins a server/game). You could also force updates (which would be the case if you set max_minetest_version)

(I've been working on faster APIs that make it possible to quickly check for updates without loading the full package information)

Protocol version vs version name

The use of the version name rather than protocol may be an issue (probably should have made it an engine feature before putting it in CDB but alas).

I went with MT version rather than protocol version as 1) it's more understandable 2) not all MT minor versions have distinct protocol versions. Note that CDB ignores the patch number

So to support forks, we could have a concept of API version which happens to be the same as the Minetest version. So a fork could have version 1.1.1 or whatever but api version 5.8

@rubenwardy
Copy link
Member

Closely related issue: #10336

@rollerozxa
Copy link
Member

rollerozxa commented Sep 12, 2023

In what ways could a game like MTG be incompatible with a newer engine?

If it uses features only found in a newer engine server version. See the existing engine feature check in default which currently checks for the existence of ItemStack.add_wear_by_uses, added in 5.6. disregard, misread

@rubenwardy
Copy link
Member

rubenwardy commented Sep 12, 2023

TL;DR: I think we should support min_minetest_version = 5.8 in Minetest Game and have better support for update detection in the client

@sfan5
Copy link
Member Author

sfan5 commented Sep 12, 2023

In what ways could a game like MTG be incompatible with a newer engine?

22:05 <+sfan5> rubenwardy: ideally only mtg ever uses this feature so I wouldn't worry too much about fork compat
22:07 <+sfan5> AFAIK the only reason MTG does this anyway is because it does one or two things that aren't guaranteed to be portable across engine versions

for example this is not very portable

@sfan5 sfan5 changed the title Allow games to enforce lock-step upgrades with versions Allow games (just MTG?) to enforce lock-step upgrades with versions Sep 12, 2023
@ancientmarinerdev
Copy link

ancientmarinerdev commented Sep 14, 2023

In what ways could a game like MTG be incompatible with a newer engine?

22:05 <+sfan5> rubenwardy: ideally only mtg ever uses this feature so I wouldn't worry too much about fork compat
22:07 <+sfan5> AFAIK the only reason MTG does this anyway is because it does one or two things that aren't guaranteed to be portable across engine versions

for example this is not very portable

Isn't there an argument for making it portable? MTG got frozen to make maintenance easier, but in reality it became this immovable object that the engine has to work around.

rubenwardy is right though. There needs to be better update detection in the client. If the engine gets updated, every single game and every single mod could be at risk of breakage, and you'd want users to know that they should update, so they can not suffer breaks and crashes.

Also, if a base game changes the way it does something, and a mod unknown to the project breaks, even if you fix it, users won't know, and they'll just suffer a break until they bang their head against a wall, hopefully raise their concern and get told to update. In reality, many will give up and describe mt as a steaming pile of poo and the reputation of minetest is impacted with this person and anyone they talk to, online or offline.

I don't think there is a case against doing this, and it's a high priority imho.

@rubenwardy
Copy link
Member

rubenwardy commented Apr 15, 2024

I suggest that all we need is a way for the engine to read min and max_minetest_version. That can be used to enforce lockstep updates.

So I would suggest we should either convert this issue into that or close and open a new one

In any case, I don't think this is needed for 5.9.

@rubenwardy rubenwardy removed this from the 5.9.0 milestone Apr 15, 2024
@sfan5
Copy link
Member Author

sfan5 commented Apr 15, 2024

Agreed.
Especially since MTG now moved away from the strict engine compatibility requirement (though it keeps the unportable item_entity hack mentioned).

@rubenwardy
Copy link
Member

Closing in favour of #10336 then

@rubenwardy rubenwardy closed this as not planned Won't fix, can't repro, duplicate, stale Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@ Content / PkgMgr Feature request Issues that request the addition or enhancement of a feature @ Mainmenu
Projects
None yet
Development

No branches or pull requests

5 participants