search

pmmp / PocketMine-MP

A server software for Minecraft: Bedrock Edition in PHP
https://pmmp.io
GNU Lesser General Public License v3.0
3.27k stars 1.54k forks source link

RFC: New API versioning scheme #1769

Closed dktapps closed 6 years ago

dktapps commented 6 years ago

Issue description

It's well known by now that the API versioning introduced last year (as documented in #179) is not very good at properly describing the server API.

Background

expand Back at MCPE 1.0, API-breaking backwards-incompatible changes were made (due to the chunk handling rewrite). This necessitated a major version bump as described under the original SemVer API versioning. To recap, semver is composed of x.y.z, or major.minor.patch: - Major: Breaking changes that are not backwards compatible. ANY breaking change (regardless of how big or small it is) requires a major API bump. - Minor: New feature additions or other alterations which are backwards-compatible, but dependents using these additions cannot function on earlier versions. - Patch: Bug fixes which are not BC-breaking which should be a no-brainer to upgrade to. As of December 2016 we had come to the conclusion that there was a significant amount of work to be done on PocketMine-MP which would require freedom to make backwards-incompatible changes whenever necessary. This was a problem because it would require a major API version bump every time any backwards-incompatible change was made at all, which would probably put us on something like API version 700.0.0 by now. This is obviously not acceptable. This led to the introduction of the ALPHA and other suffix schemes, where any ALPHA may have breaking changes in between versions. This was an attempt to remove the need for major API bumps for the frequent breaking changes, while providing plugin developers with something to work against for a specific version.

Problem

expand This has self-evidently not worked as intended, and in hindsight the ALPHA API versioning is highly flawed. As stated by @SOF3 , the current system is no different from SemVer except for the fact the effective major version is constantly being bumped, and nothing else. The ALPHA system also does a poor job of describing addition of features and bugfixes. Any and all changes are aggregated into the next ALPHA release, resulting in large mixed changelogs. When users upgrade to a new ALPHA version, they have no idea if everything or nothing will break, which effectively renders the 3.point API version useless. As it stands with the current definition of an "ALPHA" we are likely to _never_ make it out of "ALPHA" since breaking changes are frequently made to Minecraft PE, which then frequently result in breaking API changes necessary to the core code. A perpetual state of ALPHA becomes meaningless when everyone is using it in production and it's likely to never end. This situation is obviously unacceptable.

Proposal

After several months of discussions, I have come up with a proposal for a new, more sane way of versioning the API. This breaks away from SemVer, since SemVer is impractical to apply to MCPE-related things (as earlier discussed), and also because PocketMine-MP is not very modular (SemVer doesn't apply very well to projects with very large exposed API).

The proposed new versioning scheme consists of a 3-point x.y.z system which is more rational to the user.

Comments and suggestions

Are welcomed, please comment on the issue.

dktapps commented 6 years ago

Some examples:

If this RFC is accepted, the starting version needs to be decided. I've proposed 3.10.0, but I'm not sure if this makes sense.

SOF3 commented 6 years ago

A few points to note:

Note &1: The following table lists what a server with API 3.12.3 does with plugins of different API versions:

Loads:
3.12.[0-3]

Loads with warning (or not load according to pocketmine.yml):
3.[0-11].x

Does not load:
2.x.x
3.12.[>=4]
3.[>=13]x
4.x.x
dktapps commented 6 years ago

To clarify on the major version issue: I don't feel comfortable with following semver to the letter, or (going by the current scheme of things, where ALPHAs are effectively new major versions) we would be producing new major versions on a monthly basis. This (in my opinion) gives a false impression of huge overhauls on a frequent basis.

Admittedly this tends more towards fuzzy versioning preferred by humans than the hard guidelines SemVer enforces.

pranavbhuv commented 6 years ago

This sounds like the new way, to go! Before everyone who didnt know how to code, just though APIs, were just numbers, that break your plugin. But this clearly shows, the reasons for that numbering. And makes it easier for players to understand, whats in the API. This may stop use of pmt.mcpe.fun. Because then, there will be no use for it

SOF3 commented 6 years ago

@QuiverlyRivalry The difference you mentioned is invalid. It is still three numbers, where changing them in a certain way will cause plugins to stop loading. What this issue suggests, in a nutshell, is just to rearrange the numbers.

pranavbhuv commented 6 years ago

But for once, the numbers have a meaning. Like some mean alpha, beta and stable. If I am correct? @SOF3

dktapps commented 6 years ago

Referring to @SOF3 's comments, I am aware that the proposal locks down two version numbers, which is not particularly ideal. I'm thinking more of a kind of 0.x.y pre-release semver pattern, except we can't return to 0.x.y because those API versions already existed several years ago.

If the current ALPHAs each became their own respective major version (we have 9 now) we would be on API version 11.0.0 by now. That's 9 major versions in 12 months. I'm not convinced that's an acceptable rate of bumps.

For example, what about tolerating an older minor version with a warning, rather than directly rejecting to load the plugin?

this sounds like a slippery slope.

Awzaw commented 6 years ago

That's a much better system which should make life easier for everyone. An option to allow 3.[0-11].x (excluding the ALPHA series) on API 3.12.3 servers would be very useful to many people, although it could indeed be a slippery slope... at least it might reduce the number of forks blindly bumping API versions, and give developers more time to update if an API bump is all that's needed - which is very often the case.

fuyutsuki commented 6 years ago

Sorry. This is both an opinion and a request. Current plugin developers can specify multiple API versions as follows:

api: [3.0.0-ALPHA8, 3,0,0-ALPHA9, 3.0.0-ALPHA10]

Therefore, plugin developers often specify future API versions as follows for ease of correspondence as much as possible.

api: [3.0.0-ALPHA10, 3.0.0-ALPHA11, 3.0.0-ALPHA12, .......]

In this case, since the meaning of the API version disappears, please allow only a character string type, not an array, so that only one API version can be specified. Then, change to "equal.equal.lessthan", I think that a more appropriate API version will be specified. Therefore, I agree with this change.

Problem

However, if there are too many minor bumps, it may be impossible for non-active developers to keep up with specification changes.

I am sorry if this sentence is difficult to understand.

dktapps commented 6 years ago

Removing the multiple API specification capability has been a controversial topic for quite some time. With the ALPHA system it would surely be impossible.

AvgZing commented 6 years ago

As stated by others, it would be quite helpful to allow older versions. However, I will elaborate a bit more.

Both of the following example plugins are running ALPHA7, but neither have been updated to ALPHA10. This will result in the plugin not loading due to incompatible api.

If the plugin is a simple home teleporter plugin, then it should be allowed to load because there were no changes relating to that plugin in any of the new alphas.

On the contrary,

if the plugin is an ingame skin changer plugin, then the plugin should be disabled (with an explanation why) because ALPHA9 had skin changes that would cause errors with the plugin.

I apologize if this is confusing, I hope you understand what I'm saying.

As @fuyutsuki already said, this can help plugin developers when coding, because we wouldn't have to constantly update, it would only be when there is an update that concerns the plugin.

fuyutsuki commented 6 years ago

Additional notes:

When updating the API version (minor bump) of PMMP, I thought that it would be better to ask the developer to check the code used by the plug-in, so I proposed as above.

However, if you update without looking at changes, it will be meaningless if you can not use it. The designation of at least the current multiple ALPHA version is very meaningless if it is not available to the user.

Therefore, in order to check whether the developer's plug-in code interferes with minor bumps, it is a good idea for developers to change the API to "equal.equal.lessthan".

Thunder33345 commented 6 years ago

a proposed concept would be be separated system dependencies each system like command, entity, etc will be count as a "system" each time a relevant "system" is updated, the relavant api will be bumped(using sementic versioning) so my plugin yml can be api: commands: 1.0.0 server: 1.2.1 task: 1.0.0 and an API change of anything unrelated to commands wont break my plugin so if entities get an breaking update, my plugin will still run, because i never said my plugin uses any parts of it but this can be very hard to maintain(the versioning system) problem comes in when you want to define what is the group of X Y or Z? or that each time you need to remember to bump a number and bump WHICH parts correctly

fuyutsuki commented 6 years ago

@Thunder33345 Although I thought about that method for a moment, I think that multiple version control by genre division leads to confusion, both administrative and developer side is very troublesome.

Thunder33345 commented 6 years ago

@fuyutsuki yes i agree, but that's probably the only way for something like what @TheRoyalBlock said to happen

SOF3 commented 6 years ago

There are many things that cannot be categorized into systems, because pm was badly written and everything is tightly packed together. One change frequently changes different components. In addition, some changes may still not break everything in the system. For example, adding a return type hint to Command->getDescription() does not break must plugins, but it still breaks some plugins overriding it.

Furthermore, it's very hard to teach developers different systems. Unless we document and categorize every single class and every single method in the source, it's very hard for developers to know which system it is.

dktapps commented 6 years ago

Module API versions were considered, but we decided they were impractical for the reasons discussed above. Maybe that will change when/if PocketMine-MP ever becomes modular enough to consider it component-ised.

95CivicSi commented 6 years ago

While some of this sounds wonderful and appears to fix some potential problems, I don't think this will solve any of the issues associated with:

Current Example:

If plugin developers follow the prescribed methods as mentioned here https://forums.pmmp.io/threads/versions-support-direction.3776/#post-35977 and here: https://github.com/pmmp/PocketMine-MP/commit/c3017888647a585b834b48dc61ad1a93ac136a8c#commitcomment-26028019

they would be dead in the water for plugin development right now. The only reasonable courses of action to continue plugin development at this point are to run PMMP from source and modify the tagged and released version to match the protocol update from Dec 6th, or to update to an unreleased API version. As plugins are a primary reason for running PMMP, this shouldn't have happened. Anyone who wants to question this should consider that Poggit has 43 plugins that are marked for ALPHA10 released as early as Dec 7th.

According to the intent of any of these versioning systems, when you apply them to this example you still end up with the same problem. API version was bumped with a breaking number on 'master' branch without being completed for release. This happened because significant API changes are being committed directly to 'master' instead of their own branch.

If all non-security API related changes are isolated to their own branch, then patches such as the one for the 1.2.6 update can be applied to the master branch and the released version updated without breaking current development. This would make following version changes much easier. Once an API version reaches completion, all the changes will be documented in the associated API branch making it easy for anyone to follow along with what changed. Developers will have an opportunity to prepare for the new API change when a PR is submitted and protocol patches such as the 1.2.6 update will not break expected development behavior.

In summary, the versioning system described appears to be a good method but will be afflicted with the same problems the current system has unless this change includes a change to the behavior of making API modifications directly to the master branch. Further details and examples can be provided upon request.

dktapps commented 6 years ago

The current ALPHA10 problem actually lies in the fact that there are little to no breaking changes between the versions, yet an API bump of some description was still needed (and I haven't had time to write changelogs yet). This is why this RFC was created, because the current versioning scheme does not provide fine-grained enough control.

Effectively, versioning as ALPHAs is useless, because in the current situation we will never be out of ALPHA. The released versions are stable enough to use, just not feature complete. So while the releases will be implicitly alpha due to not being feature complete, we'll be dropping the ALPHA naming.

dktapps commented 6 years ago

Additionally breaking changes are initially made to their own branches for testing purposes, but soon merged to master. The real problem is not that master is getting breaking changes, but that there are no legacy support branches.

95CivicSi commented 6 years ago

The changes may have been needed and the API version number required to change, but if those changes had been happening in an isolated branch, then no one would have been affected by the API bump when the Protocol version was bumped to meet 1.2.6 criteria. That's the whole point. It doesn't matter what versioning system you go by if API changes are made directly to the branch that is supporting the current release.

Isolate non-security type API changes to their own branch. Bump the branch version number as necessary. Leave the API on the master branch alone unless security is flawed or the API branch is ready to release. If the API branch is ready to release, it gets a PR. Developers have time to react and VOILA! Production type usage is no longer on 'bleeding edge' API.

95CivicSi commented 6 years ago

Also, Legacy support is not feasible. Minecraft PE users are being forced to update their clients and most people aren't capable of stopping those updates. Ask the community how many legacy servers still have significant users.

dktapps commented 6 years ago

You don't understand. master is bleeding-edge, it's not supposed to be used in production. No one in any software project expects you to build from master unless you want bleeding edge.

The real problem is not that master is getting breaking changes, but that there are no legacy support branches.

dktapps commented 6 years ago

It's not really a new big wowy thing that you're trying to assert. It's just one that doesn't work, because it forces the developer to support a single version of the software at any one time. It also asserts the incorrect assumption that master is the greatest for users to use. It's not.

By legacy branch, I meant legacy branches of PocketMine-MP which would still receive protocol updates and bugfixes that were non-BC breaking.

For example, if there was an ALPHA9 legacy supported like this, then the whole 1.2.6 problem would not have happened. However that would have presented a problem in that a new version would need to be tagged, which the ALPHAs don't provide fine-grained control for.

Look at php-src - all the bleeding edge changes go onto master. Nobody uses php-src master unless they expect the latest bleeding-edge version of PHP. Instead, they check out one of the feature-frozen support branches, such as PHP-7.0, PHP-7.1, PHP-7.2. These branches are also cutting edge (hence the -DEV suffix on the version), just less so because they are only getting bug fixes and the occasional minor feature. This pattern allows the same bug fixes to be released into 3 different legacy support versions at the same time.

95CivicSi commented 6 years ago

Considering that any current Bedrock client will refuse to connect with the Alpha9 release, and the fact that Poggit already has 40+ plugins that have been released and approved for Alpha10, I'd say that no matter what the intention is, the master branch is the current go to for plugin development and production.

With the exception of master being the support branch, what we're talking about is essentially the same. Isolating API changes and focusing developers to a single area where API updates don't get mixed in with support updates. I could care less what the name of the branch it happens on is, and my example was simply that, an example. It was primarily based on the fact that most development has to happen via the master branch as that's where all the support fixes come through for the moment. Everything I've said still applies if the same principal actions happen on the support branch. I think I also mentioned this idea in the forums: https://forums.pmmp.io/threads/versions-support-direction.3776/#post-35969

3.Pick a current version of the server to promote.

  • This will give current plugin developers a stopping / focal point with the current API.
  • New users will all be facing similar issues instead of varied problems that are dependent on when they installed.
  • The 'non-development' community will settle until a significantly stable version of the server is released.
  • Time and resources will be freed up to work on stabilizing PMMP.

So if general consensus agrees with this versioning system, then for it to be successful, API changes will need to be isolated from support updates. Peel off at the point of agreement, create a support branch (or whatever you prefer to call it) and don't push API changes to that branch except for security reasons. If your method is that the master branch is bleeding-edge then this process will be repeated every time the master branch reaches a point where the API is stable enough to support and there is a feeling that significant benefit will be gained by updating.

XoticRealmsOwnerandDev commented 6 years ago

if microsoft had a mcbe launcher non of this stuff would be needed because people could just pick one version for their server and not have to update it all the time

dktapps commented 6 years ago

Closing this as resolved by c3c360f589541257908ce17fb8f021753f2d41d0. There is an independent issue (merging the server and API versions), but from a plugin perspective the server version is irrelevant. That can be dealt with separately.