Add compatibility with MC 1.20.6 #125
Comments
josemmo
added
type: feature
josemmo
changed the title
- Updated Internals class > Related to #125
Started! |
|
Still not fixed |
|
1.20.4 version up to 1.20.6 Yamipa does not output any error logs. |
|
Maybe because of the ProtocolLib? |
|
Yes, this is related to ProtocolLib. Some bugs first need to be fixed in this library before they can be addressed in Yamipa, so be patient. |
|
Is there an open issue for that in dmulloy's repo? Can I have a look? |
plugins/YamipaPlugin/config.yml verbose: trueIs it possible to check the problem with Yamipa from this log? Do you know what ProtocolLib API is causing the image not to be displayed? ProtocolLib still has an issue where MAP_CHUNK, SCOREBOARD_OBJECTIVE, and ENTITY_SOUND packets cannot be created. |
|
Yamipa cannot render images on 1.20.6 because ProtocolLib is having issues sending or crafting the packets for that. I don't know the internals enough to pinpoint exactly the cause inside ProtocolLib, but other plugins are having similar problems. I see new progress has been made on ProtocolLib, whenever I find time I'll try to upgrade the plugin. If you desperately need this to be fixed, then clone Yamipa's repository, decompile the Minecraft server, download the symbol mappings and get to it yourself. |
The following APIs appear to be missing in ProtocolLib 5.3.0.
for more informations: |
|
That technically wasn't an API, it was internal code that a plugin shouldn't depend on because it's part of the injector. The plugin used reflections to even get access to the class because we don't expose it: This should however be an easy fix simply remove the code that tries to access |
|
Thank you for answering. |
|
@josemmo src/main/java/io/josemmo/bukkit/plugin/renderer/FakeEntity.java: @@ -32,6 +32,8 @@ public abstract class FakeEntity {
if (PLAYER_INJECTION_HANDLER == null) {
throw new RuntimeException("No valid candidate field found in ProtocolManager");
}
+ } catch (NoClassDefFoundError e) {
+ LOGGER.severe("Failed to get PlayerInjectionHandler from ProtocolLib", e);
} catch (Exception e) {
LOGGER.severe("Failed to get PlayerInjectionHandler from ProtocolLib", e);
} |
|
I just tried to apply the proposed fix in #128. Now the plugin loads without errors, but images are still not visible in the game. Anyways, it's a progress. |
|
Found another issue. At this line, ProtocolLib throws an exception, which is not visible in the logs for some reason, but got caught by my debugger. I'm trying to figure out why it happens. |
|
Same problem at |
|
Just to be clear: the main reason Yamipa is not rendering images in Minecraft 1.20.5+ is because of dmulloy2/ProtocolLib#3019. Yes, the missing So for second time, please be patient until this gets fixed. |
|
Yeah, sorry, I was just trying to investigate what is going wrong to maybe help you out a little, I wasn't aware you already got to the root of the problem 😅 |
|
Since it seems that it will take many many time to support ProtocolLib, I decided to try to support 1.20.6 and 1.21 using fork of Custom Images as a reference. |
|
The ProtocolLib team seems to be very busy and can't keep up with the fixes. I was recommended by developer friends to use PacketEvents or TinyProtocol/nms directly. ProtocolLib supposedly offers hardly any more advantages. |
|
I disagree while both ProtocolLib and PacketEvents address similar issues, their underlying mechanics are quite different. ProtocolLib acts as a wrapper around Mojang's game objects, while PacketEvents implements specific read/write methods for each packet across supported Minecraft versions. One of PacketEvents' main advantages is its interoperability between server and client code, which is less critical for server-only plugins. However, because PacketEvents creates its own implementations of packets and wrapped objects, there's a higher risk of mismatches with what Mojang's client expects. In contrast, ProtocolLib uses the original Mojang packets, simply adding its own interface on top, minimizing compatibility issues. Another key difference is that PacketEvents requires developers to manage threading carefully. Each packet is processed on the Netty thread it’s received or sent on, which can lead to errors if one listener blocks that thread. Since multiple connections share a single thread, a delay in one listener could impact multiple players. Their official example on thread safety illustrates another shortcoming: it clones a packet event object, and if the developer doesn’t clean up that clone, it can lead to memory leaks as Netty won’t release the underlying buffer. ProtocolLib's default API does not have this issue. While PacketEvents allows for lower-level coding, it also increases the potential for bugs. This doesn't necessarily mean PacketEvents is a bad plugin; rather, switching between the two isn't straightforward and requires careful consideration of these differences. @josemmo Have you maybe look into fixing the issue you're having with ProtocolLib yourself. I could also look into the issue but as far as I know NMS for items got removed in 1.21. |
|
The current blocker with ProtocolLib lies in the yamipa/src/main/java/io/josemmo/bukkit/plugin/renderer/FakeItemFrame.java Lines 149 to 162 in 424aa67 If someone manages to adapt these pieces of code to the latest versions of Minecraft using ProtocolLib, the next release of Yamipa becomes pretty straightforward. |
|
But why are you using reflections to set the map id? I just checked the implementation of 1.16.5 if (hasMapId()) {
tag.setInt(MAP_ID.NBT, getMapId());
}1.20.5 if (hasMapId()) {
tag.put(MAP_ID, new MapId(getMapId()));
}Regarding the |
|
Hello @Ingrim4, I don't understand where should I put that code. Can you give more details or submit a PR fixing the issue? |
|
What I'm suggesting is to use the ItemStack map = new ItemStack(Material.FILLED_MAP);
ItemMetaMap mapMeta = (ItemMetaMap) map.getItemMeta();
mapMeta.setMapId(maps[step].getId());
map.setItemMeta(mapMeta); |
|
I solved it a few months ago, but I'm stuck on another part. https://wiki.vg/Protocol#Map_Data Is this information only for the latest version? Does it match? |
|
In the latest version map packet class is defined as follows: public record ClientboundMapItemDataPacket(
MapId mapId,
byte scale,
boolean locked,
Optional<List<MapDecoration>> decorations,
Optional<MapPatch> colorPatch
) implements Packet<ClientGamePacketListener> {
public ClientboundMapItemDataPacket(
MapId mapId,
byte scale,
boolean locked,
@Nullable Collection<MapDecoration> decorations,
@Nullable MapPatch colorPatch
) {
this(
mapId,
scale,
locked,
decorations != null ? Optional.of(List.copyOf(decorations)) : Optional.empty(),
Optional.ofNullable(colorPatch)
);
}
}
public static record MapPatch(int startX, int startY, int width, int height, byte[] mapColors) {} |
|
On 1.21.1 (Spigot) the plugin doesn't start at all. Fortunately, images that have already been set are still retained. |
|
Unfortunately, the more I debug this issue, the more I'm convinced there's little I can do without changing the code of ProtocolLib (which I'm not familiar enough with). Here's where we're at:
This is the exception that is thrown at Unless someone at ProtocolLib looks at this issue, the more feasible approach going forward is to migrate to PacketEvents as suggested by @RedstoneFuture. |
|
Will the Migration to PacketEvents include 1.21.x Support? |
|
That isn't a ProtocolLib issue your just calling an internal method of ProtocolLib with an invalid type ( |
|
@Ingrim4 Could you provide a PR? |
|
Some points I'd like to clarify: Yes, Yamipa uses an internal method of ProtocolLib. This is done for performance reasons, as rendering images takes a bunch of bandwidth and afaik Let's not use the internal protected static void tryToSendPacket(@NotNull Player player, @NotNull PacketContainer packet) {
try {
+ CONNECTION.sendServerPacket(player, packet);
- if (NETWORK_MANAGER_INJECTOR == null) { // Use single-threaded packet sending if reflection failed
- CONNECTION.sendServerPacket(player, packet);
- } else { // Use non-blocking packet sending if available (faster, the expected case)
- NETWORK_MANAGER_INJECTOR.getInjector(player).sendClientboundPacket(packet, null, false);
- }
} catch (IllegalStateException e) {
// Server is shutting down and cannot send the packet, ignore
} catch (Exception e) {
LOGGER.severe("Failed to send FakeEntity packet", e);
}
}Well, even after that, Yamipa is still not rendering the images. To make matters worse, there seems to be an issue with multithreading, as some packets fail to be sent correctly without throwing an exception. For instance, this test world should have item frames covering all iron blocks (but there are seemingly random gaps):
In conclusion: I have no idea why this doesn't work in the latest versions of Minecraft. I'm pretty sure it has something to do with ProtocolLib, though I'd love to be proven wrong because that would mean someone finally found a fix for this issue. |
|
The PR #137 is done, and no changes to ProtocolLib were necessary. The only issue I encountered with ProtocolLib is that |
Which Version of ProtocolLib did you use? |
|
Fixed, forgot that paper now has mojang class mappings all the time. ProtocolLib = latest dev build |
Plugin is now starting but pictures are invisible. |
|
I just tried it with paper 1.21, spigot 1.20.4 and 1.21 and it works perfectly fine for me. Are you sure you build the latest commit from my fork and are you using the latest dev build of ProtocolLib? |
|
I have confirmed that it works properly with PR #137. Confirmation procedure
git remote add Ingrim4 https://github.com/Ingrim4/yamipa.git
git fetch Ingrim4 feat/support-1.21
git switch feat/support-1.21
mvn package
services:
mc:
image: itzg/minecraft-server:latest
container_name: ${TEST_CONTAINER:-test}
environment:
TZ: "${TZ:-Asia/Tokyo}"
EULA: "true"
TYPE: "${TEST_TYPE:-paper}"
VERSION: "${TEST_VERSION:-1.21.1}"
MODE: "creative"
FORCE_GAMEMODE: "true"
MODRINTH_PROJECTS: "viaversion,viabackwards"
MODS: "https://ci.dmulloy2.net/job/ProtocolLib/lastSuccessfulBuild/artifact/build/libs/ProtocolLib.jar"
RCON_CMDS_STARTUP: |-
${TEST_OPERATOR:-op Muscle_p}
ports:
- "${TEST_PORT:-25565}:25565"
volumes:
- ./data-${TEST_TYPE:-paper}-${TEST_VERSION:-1.21.1}:/data
- ./plugins:/plugins:ro
cd test
cp ../target/YamipaPlugin-1.3.2-SNAPSHOT-b2.jar plugins/Yamipa.jar
docker compose up
docker compose down |
|
Thanks @Ingrim4 for all your work! 👏👏 Will try to find some time this weekend to draft a new release. |
|
The wait is finally over, Yamipa v1.3.2 is out! 🥳🎉 Thanks to everyone who worked on this, especially @Ingrim4! |
I am always indebted to Yamipa.
I am just reporting this because it did not start with 1.20.6.
The text was updated successfully, but these errors were encountered: