max1mde / PassengerAPI

A passenger management API to make cross plugin compatibility possible.
https://www.spigotmc.org/resources/passengerapi-entity-passenger-bug-fixes-more.117017/
3 stars 0 forks source link

Concurrency Issue in your PassengersAPI Packet Listener #1

Closed retrooper closed 1 month ago

retrooper commented 1 month ago

Greetings. A member in our discord server has reported an exception and I took the time out of my day to address it to you directly. This was the exception provided by the user: image

In our documentation we explain that you must be cautious when designing your packet listeners as Bukkit often limits asynchronous access: image

You have two ways you can fix this.

  1. You could use our utilities to "hackily" bypass this async catcher and find entities by ID:
    //World may be null if you wish to search through all worlds (advised to provide a world)
    World world = ...;
    Entity entity = SpigotConversionUtil.getEntityById(world, entityID);
  2. You could schedule your task onto the main thread and access the world the way Minecraft expects you to do so:
    Bukkit.getScheduler().runTask(plugin, () -> {
        //Your entity finding code here...
    });

    In fact, you already do so here: https://github.com/max1mde/PassengerAPI/blob/e68a8e75ffd4c30922c65a252f38ebed1a2ef601/src/main/java/com/maximde/passengerapi/listeners/PacketSendListener.java#L45-L56

Therefore, I'm not sure why you didn't carry this out everywhere.

retrooper commented 1 month ago

This can be replaced with event.getPlayer() https://github.com/max1mde/PassengerAPI/blob/e68a8e75ffd4c30922c65a252f38ebed1a2ef601/src/main/java/com/maximde/passengerapi/listeners/PacketReceiveListener.java#L27

max1mde commented 1 month ago

Thanks for reaching out! I'm gonna fix this asap.

max1mde commented 1 month ago

This can be replaced with event.getPlayer()

https://github.com/max1mde/PassengerAPI/blob/e68a8e75ffd4c30922c65a252f38ebed1a2ef601/src/main/java/com/maximde/passengerapi/listeners/PacketReceiveListener.java#L27

Yeah I just wasn't sure if event.getPlayer() returns a bukkit player.

retrooper commented 1 month ago

In the new snapshots you don't need to explicitly cast it.