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.55k forks source link

[RFC] Enhancements to mcpe-protocol in plugin.yml #4102

Closed fuyutsuki closed 3 years ago

fuyutsuki commented 3 years ago

This is an RFC. See the guidelines for RFCs in CONTRIBUTING.md.

Introduction

The current mcpe-protocol directive in the plugin.yml disables the plugin when the protocol is updated. This RFC proposes to improve and make it more developer-friendly.

Changes

The current spec uses only the protocol described in mcpe-protocol to determine whether to enable or disable, regardless of the packets used by the plugin, but this is not desirable because developers with low update frequency are rushed to support the latest version by Poggit and others.

So, I think it would be a good to disable the plugin only when the specific packet used by the plugin is updated by using the following structure for mcpe-protocol and writing the last updated protocol version of the packet.

Examples

plugin.yml

mcpe-protocol:
  AddPlayerPacket: 428
  StartGamePacket: 428

PacketVersion(temporary)

final class PacketVersion {
    ...
    public const ADD_PLAYER_PACKET = 291;
    ...
    public const START_GAME_PACKET = 428;
    ...
}

Result

If even one of the versions is less than the tested version of the packet, disable the plugin. PacketVersion::{PACKET_NAME} >= mcpe-protocol.{PacketName} = true PacketVersion::{PACKET_NAME} < mcpe-protocol.{PacketName} = false

BCBreak

With this change, mcpe-protocol will accept associative array values, but since the values it receives are originally protocol version integers. It can easily coexist with the old format using is_array and not cause a BCBreak. The old format should be removed in the future, but I don't think it needs to be now.

However, there are only a handful of plugins that use this directive....

Follow-up maintainer (...dktapps)

When updating packet, you need to update the packet version at the same time.

Reference

SOF3 commented 3 years ago

There are quite some protocol changes that aren't packet-specific, and sometimes it's unclear which packets to bump protocol version for, and sometimes it's unclear which packets to constraint protocol to.

fuyutsuki commented 3 years ago

Yes, you are right. Maybe we should also keep the old directives for plugins that deal with packet-independent stuff, and implement this RFC in a new one. Think of this RFC as covering packet-dependent plugins.