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

Add support for 1.20.5/1.20.6 part 2 #2910

Merged

Conversation

Ingrim4
Copy link
Collaborator

@Ingrim4 Ingrim4 commented May 5, 2024

Description

See #2894

Related Issue

closes #2907
closes #2900
closes #2903

How Has This Been Tested?

Untested as of yet.

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

@ClumsyIsNotReal
Copy link

ClumsyIsNotReal commented May 5, 2024

Compiled version with the changes and still getting errors using Latest Paper 1.20.6. Paper uses Mojang mapping which could be the cause between the differences of it working on spigot and not paper

[19:59:57` INFO]: [ProtocolLib] Loading server plugin ProtocolLib v5.2.1-SNAPSHOT
[19:59:57 WARN]: [ProtocolLib] Version (MC: 1.20.6) has not yet been tested! Proceed with caution.
[19:59:58 ERROR]: [ProtocolLib] Assuming package version: v1_20_R1
[19:59:58 ERROR]:   [ProtocolLib] INTERNAL ERROR: Cannot load ProtocolLib.
  If this problem hasn't already been reported, please open a ticket
  at https://github.com/dmulloy2/ProtocolLib/issues with the following data:
  Stack Trace:
  java.lang.RuntimeException: Failed to find NMS class: network.ProtocolInfo$a
        at ProtocolLib.jar//com.comphenix.protocol.utility.MinecraftReflection.lambda$getMinecraftClass$2(MinecraftReflection.java:1383)
        at java.base/java.util.Optional.orElseThrow(Optional.java:403)
        at ProtocolLib.jar//com.comphenix.protocol.utility.MinecraftReflection.getMinecraftClass(MinecraftReflection.java:1383)
        at ProtocolLib.jar//com.comphenix.protocol.injector.packet.PacketRegistry.createRegisterV1_20_5(PacketRegistry.java:344)
        at ProtocolLib.jar//com.comphenix.protocol.injector.packet.PacketRegistry.initialize(PacketRegistry.java:498)
        at ProtocolLib.jar//com.comphenix.protocol.injector.packet.PacketRegistry.getClientPacketTypes(PacketRegistry.java:538)
        at ProtocolLib.jar//com.comphenix.protocol.injector.PacketFilterManager.<init>(PacketFilterManager.java:120)
        at ProtocolLib.jar//com.comphenix.protocol.injector.PacketFilterBuilder.build(PacketFilterBuilder.java:121)
        at ProtocolLib.jar//com.comphenix.protocol.ProtocolLib.onLoad(ProtocolLib.java:183)
        at io.papermc.paper.plugin.storage.ServerPluginProviderStorage.processProvided(ServerPluginProviderStorage.java:59)
        at io.papermc.paper.plugin.storage.ServerPluginProviderStorage.processProvided(ServerPluginProviderStorage.java:18)
        at io.papermc.paper.plugin.storage.SimpleProviderStorage.enter(SimpleProviderStorage.java:39)
        at io.papermc.paper.plugin.entrypoint.LaunchEntryPointHandler.enter(LaunchEntryPointHandler.java:36)
        at org.bukkit.craftbukkit.CraftServer.loadPlugins(CraftServer.java:508)
        at net.minecraft.server.dedicated.DedicatedServer.initServer(DedicatedServer.java:287)
        at net.minecraft.server.MinecraftServer.runServer(MinecraftServer.java:1140)
        at net.minecraft.server.MinecraftServer.lambda$spin$0(MinecraftServer.java:326)
        at java.base/java.lang.Thread.run(Thread.java:1583)
  Dump:
  Parameters: 
    [NULL]
  Sender:
    com.comphenix.protocol.ProtocolLib@7723e60a[
      statistics=<null>
      packetTask=<null>
      tickCounter=0
      configExpectedMod=-1
      updater=com.comphenix.protocol.updater.SpigotUpdater@1c138bfd
      redirectHandler=<null>
      scheduler=com.comphenix.protocol.scheduler.DefaultScheduler@74683dc3
      commandProtocol=<null>
      commandPacket=<null>
      commandFilter=<null>
      packetLogging=<null>
      skipDisable=false
      isEnabled=false
      loader=io.papermc.paper.plugin.manager.DummyBukkitPluginLoader@70864bf9
      server=CraftServer{serverName=Paper,serverVersion=git-Paper-48,minecraftVersion=1.20.6}
      file=plugins/.paper-remapped/ProtocolLib.jar
      description=org.bukkit.plugin.PluginDescriptionFile@11b502f7
      pluginMeta=org.bukkit.plugin.PluginDescriptionFile@11b502f7
      dataFolder=plugins/ProtocolLib
      classLoader=PluginClassLoader{plugin=ProtocolLib v5.2.1-SNAPSHOT, pluginEnabled=false, url=plugins/.paper-remapped/ProtocolLib.jar}
      naggable=true
      newConfig=YamlConfiguration[path='', root='YamlConfiguration']
      configFile=plugins/ProtocolLib/config.yml
      logger=com.destroystokyo.paper.utils.PaperPluginLogger@1c7ec9f2
      lifecycleEventManager=io.papermc.paper.plugin.lifecycle.event.PaperLifecycleEventManager@74bd6ce9
      allowsLifecycleRegistration=true
    ]
  Version:
    ProtocolLib v5.2.1-SNAPSHOT
  Java Version:
    21.0.3
  Server:
    git-Paper-48 (MC: 1.20.6)
[19:59:58 INFO]: Server permissions file permissions.yml is empty, ignoring it
[19:59:58 INFO]: [ProtocolLib] Enabling ProtocolLib v5.2.1-SNAPSHOT
[19:59:58 ERROR]: ******************************************************
[19:59:58 ERROR]: *** ProtocolLib does not support plugin reloaders! ***
[19:59:58 ERROR]: *** Please use the built-in reload command!        ***
[19:59:58 ERROR]: ******************************************************
[19:59:58 INFO]: [ProtocolLib] Disabling ProtocolLib v5.2.1-SNAPSHOT
```

@Ingrim4
Copy link
Collaborator Author

Ingrim4 commented May 5, 2024

Compiled version with the changes and still getting errors using Latest Paper 1.20.6. Paper uses Mojang mapping which could be the cause between the differences of it working on spigot and not paper

Works fine for me, did you compile the code from this PR or some older code?

@ClumsyIsNotReal
Copy link

Compiled version with the changes and still getting errors using Latest Paper 1.20.6. Paper uses Mojang mapping which could be the cause between the differences of it working on spigot and not paper

Works fine for me, did you compile the code from this PR or some older code?

Could you add me on discord: clumsyisnotreal

make it easier to see what is up

dmulloy2
dmulloy2 previously approved these changes May 6, 2024
@@ -231,6 +253,11 @@ private static void initializePacket() {
}
}

// write and read methods are no longer part of the packet interface since 1.20.5
if (MinecraftVersion.v1_20_5.atOrAbove()) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we want to throw an exception here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I guess we could throw an exception when the read/write method getters get called on 1.20.5 upwards

}
}
}
PacketType.onDynamicCreate = (type, className) -> System.out.printf("%s, %s = new PacketType(PROTOCOL, SENDER, %s, \"%s\")\n", type.getProtocol(), type.getSender(), formatHex(type.getCurrentId()), className);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

love it

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seemed like the easiest way to me I would also like to parse the packet names and only print the short name if possible:
e.g. ServerboundClientInformationPacket -> ClientInformation

@dmulloy2
Copy link
Owner

dmulloy2 commented May 6, 2024

looks great! any particular reason it's still a draft?

@Ingrim4
Copy link
Collaborator Author

Ingrim4 commented May 6, 2024

Still would like to change some stuff and add missing packet types.

@dmulloy2
Copy link
Owner

dmulloy2 commented May 6, 2024

Still would like to change some stuff and add missing packet types.

gotcha. well let me know when it's ready. even if it requires a follow up PR, seems like this would help with some of the errors people are seeing

@Ingrim4
Copy link
Collaborator Author

Ingrim4 commented May 6, 2024

gotcha. well let me know when it's ready. even if it requires a follow up PR, seems like this would help with some of the errors people are seeing

@dmulloy2 I think I'm done for now with everything 1.20.5/1.20.6 and packet related. Some wrappers probably won't work I also didn't test everything just some basic async packet listener.

Completely off-topic but related to may work on this PR: After delving into ProtocolLib's internal code I feel like certain aspects, like packet types (packet IDs), require a revamp to align better with the current state of the game. However, such changes might break compatibility with older game versions pre 1.7. That's why, it's uncertain whether such modifications are desirable for this project. My proposal is to introduce ProtocolLib 6.0.0, designed for Minecraft 1.16+ and Java 17+. This approach would enable us to overhaul significant components of the system while retaining support for the majority of versions still in use today. If that approach is to extreme I would still like some feedback on this since I would also be interested in a less extreme approach that tries to keep comparability with 1.8, 1.12 and 1.16+. Might also be a good idea to start a github discussion on this and get some feedback from other people.

@Ingrim4 Ingrim4 marked this pull request as ready for review May 6, 2024 16:11
@dmulloy2 dmulloy2 merged commit b0c4b7f into dmulloy2:master May 6, 2024
2 of 3 checks passed
@dmulloy2
Copy link
Owner

dmulloy2 commented May 6, 2024

agreed. worth looking into mojang mapping as well

@GroovyNoah
Copy link

can u ad me on discord groovynoah.eth I have an issue with protocollib id like to discuss

@Ingrim4 Ingrim4 deleted the feat-1.20.5-support-part2 branch June 6, 2024 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1.20.6 1.20.5 Paper 1.20.5/6 Failed
4 participants