Version specific packet implementation pattern

[Klotzi111]

This PR is built on top of #77

This PR adds the pattern for minecraft version specific packet implementation that was discussed in #68.

The newly added PacketSourceGenerator project runs when building the MineSharp.Protocol project and generates C# files that add (via partial keyword) methods/fields/properties to the packet types.

To debug the source generator:
Select Debug build and create the file:
Components/MineSharp.PacketSourceGenerator/debugSourceGenerator.txt.user
then do a rebuild of the MineSharp.Protocol project. This will then run the source generator.
The source generator will look for the file mentioned above and if it exists will call Debugger.Launch().
Then you can attach with your debugger.

TODO:
There are still some things that could be improved:

• add interfaces for IPacketClientbound and IPacketServerbound to add type constraints for On<Packet> and SendPacket
• let the source generator emit diagnostics when it detects something wrong (currently this error messages can only be seen when debugging the source generator)

[Klotzi111]

As I was implementing the new version specific packet pattern for RespawnPacket I encountered this problem:

Since Minecraft 1.16.3 this packet changed multiple times but only slightly.
And making multiple records that all have almost the same fields and also almost the same Read and Write methods feels wrong.
How should we deal with this?

Possible solutions:

1. let (version specific) packet sub types inherit other packet sub types
2. create a common data container record that is then used in the packet sub types

My thoughts on the solutions:

1. I do not like it. All packet sub types should only inherit the base packet type. Because otherwise this would break type checking (is keyword)
2. This is better but still not super nice to use. Usage would be something like respawnPacket.SharedData.DimensionName

What do you think?

Code for RespawnPacketV_1_19_4:

    public sealed partial record RespawnPacketV_1_19_4(
        Identifier DimensionType,
        Identifier DimensionName,
        long HashedSeed,
        GameMode Gamemode,
        GameMode? PreviousGamemode,
        bool IsDebug,
        bool IsFlat,
        DataKeptFlags DataKept,
        bool HasDeathLocation,
        Identifier? DeathDimensionName,
        Position? DeathLocation
    ) : RespawnPacket
    {
        /// <inheritdoc />
        public override void Write(PacketBuffer buffer, MinecraftData data)
        {
            buffer.WriteIdentifier(DimensionType);
            buffer.WriteIdentifier(DimensionName);
            buffer.WriteLong(HashedSeed);
            buffer.WriteByte((byte)Gamemode);
            buffer.WriteSByte(PreviousGamemode.HasValue ? (sbyte)PreviousGamemode : (sbyte)-1);
            buffer.WriteBool(IsDebug);
            buffer.WriteBool(IsFlat);
            buffer.WriteByte((byte)DataKept);
            buffer.WriteBool(HasDeathLocation);
            if (HasDeathLocation)
            {
                buffer.WriteIdentifier(DeathDimensionName!);
                buffer.WritePosition(DeathLocation!.Value);
            }
        }

        /// <inheritdoc />
        public static new RespawnPacketV_1_19_4 Read(PacketBuffer buffer, MinecraftData data)
        {
            var dimensionType = buffer.ReadIdentifier();
            var dimensionName = buffer.ReadIdentifier();
            var hashedSeed = buffer.ReadLong();
            var gamemode = (GameMode)buffer.ReadByte();
            var previousGamemodeValue = buffer.ReadSByte();
            var previousGamemode = previousGamemodeValue == -1 ? null : (GameMode?)previousGamemodeValue;
            var isDebug = buffer.ReadBool();
            var isFlat = buffer.ReadBool();
            var dataKept = (DataKeptFlags)buffer.ReadByte();
            var hasDeathLocation = buffer.ReadBool();
            Identifier? deathDimensionName = null;
            Position? deathLocation = null;
            if (hasDeathLocation)
            {
                deathDimensionName = buffer.ReadIdentifier();
                deathLocation = buffer.ReadPosition();
            }

            return new(dimensionType, dimensionName, hashedSeed, gamemode, previousGamemode, isDebug, isFlat, dataKept, hasDeathLocation, deathDimensionName, deathLocation);
        }
    }

RespawnPacketV_1_20_0 then only added the int PortalCooldown field at the end.

[psu-de]

Hi,
sorry for the delay, I'm currently a little stressed out.

I'd also go with option 2. But maybe we can put the shared fields right into the super class, instead of putting them into another SharedData record? It would be syntactically a little bit nicer or is there another drawback?

[Klotzi111]

But maybe we can put the shared fields right into the super class, instead of putting them into another SharedData record? It would be syntactically a little bit nicer or is there another drawback?

The drawback is that with this approach we are now back to our orignal problem for which we started the sub class for every version refactoring. The base class would get fields that the actual packet (sub class) does not have.
For example:
We might want to add the "common" fields (DimensionType, DimensionName, HashedSeed, ...) to the base class. Then the sub classes for V_1_19_4 and V_1_20_0 could use and share these fields. But the sub class for V_1_7_0 would have some fields that are not actually contained in the packet. The V_1_7_0 packet only has these fields: Dimension, Difficulty, Gamemode, LevelType. So the HashedSeed field in the base class is "fake" for V_1_7_0

[psu-de]

Okay I guess there is not really a way around it (without code duplication). But the SharedData record shouldn't have nullable types as well right? So maybe we'd need multiple of theses for a packet.

[Klotzi111]

But the SharedData record shouldn't have nullable types as well right?

Yes. Fields should only be nullable if the packet for the matching protocol version contains nullable (wiki.vg language: Optional) fields.

So maybe we'd need multiple of theses for a packet.

Yes that is correct. And was the idea. SharedData is a bad name then. The fields for the shared data records need names that give some info about the data fields they contain.

We then can build packets like this (only example names):
TestPacketV1: PlayerData, string Name
TestPacketV2: PlayerData, WorldData, string Name
TestPacketV3: WorldData, int Id
(The *Data fields are shared data records)

This way we could make sub packets for multiple versions without much code duplication and copying/processing this data is easier between minecraft versions.
We might also need some shared data records that contain other shared data records. For example when there are 6 fields in the first shared data record and in some later version 2 more fields are added (and versions after that also contain these fields) then we need a new shared data record that contains the first one and also these 2 new fields.
At this point we could also go with inheritance of the shared data records but I think composition is better.

What do you think?

[psu-de]

Okay, lets go with the composition approach.

One more idea: We could pass the properties of shared records through to the packet class.
Something like this:

internal PlayerData PlayerData { get; private set; }

public Uuid Uuid
{
     get => PlayerData.Uuid;
     set => PlayerData.Uuid = value;
}

This way we can hide the (probably messy) data records from the outside and keep the Api concise. But it would add a lot of boiler plate code again...
Would it be possible to source generate this? (I have never worked with source generators).

[Klotzi111]

One more idea: We could pass the properties of shared records through to the packet class.

I though of this too. (see below)

Would it be possible to source generate this?

Yes that is possible.

We could do this. But I would not hide the shared data records (instead add these properties in addition to the shared data records). Because these records bundle the fields that most use cases of these packets will need.
What I mean is:
When receiving these packets you probably want to check a condition based on the fields of the packet. And you want to do this check regardless of the actual protocol version. If this check only contains one field then the properties will be helpful. But if this check contains (for example) 6 fields. Then you probably want to extract this check in its own function. And now you would need to pass these 6 fields to the function for every versioned packet class. This is not so nice. If the shared data records are accessible then you could just pass that to the check function.

But you would still need to switch (pattern matching) all the versioned packet classes that provide this shared data record.
Maybe we also need to let the packet classes implement interfaces that provide such shared data records. Then a type check for that interface would be enough to get the shared data record for every versioned packet class that has these values.
These interface could also be source generated.