Hazebyte / CrateReloaded

Issue tracker for the Bukkit plugin, CrateReloaded
17 stars 19 forks source link

Scrolling Animation Broken w/o Inventory Update #164

Closed mibby closed 1 year ago

mibby commented 6 years ago

Updated to CrateReloaded 1.3.98.1. The CSGO crate scrolling animation seems to be broken and staying on certain heads during the rotation.

https://gyazo.com/31b6360c1689dd1cf3fd306879482522

imWillX commented 6 years ago

The issue appears to come from items that are in the roulette and occur in succession. i.e HEAD -> HEAD -> HEAD, or DIRT -> DIRT -> DIRT.

This could be fixed via player#updateInventory, more specifically PacketPlayOutWindowItems however both methods cause hand flickering. https://hub.spigotmc.org/javadocs/spigot/org/bukkit/entity/Player.html#updateInventory--

Last method which requires testing is to create an new inventory for each animation update and transfer the items over. I'm not too sure on the performance since, if we choose this method, we're creating up to 50 inventories.

mibby commented 6 years ago

Last method referring to updating the item meta of the items inside the container every tick rotation / animation update rather than force an inventory update via player.updateInventory - which causes hand flickering.

For reference. https://github.com/MaTaMoR/CustomInventoriesAPI/blob/master/src/main/java/us/swiftex/custominventories/inventories/CustomInventory.java#L89

mibby commented 6 years ago

Untested method found on spigot forums from 2015. Would sending a new PacketPlayOutOpenWindow packet with a new title resolve the hand flickering problem?

ProtocolLib example.

Object ep = ReflectionUtil.call(player.getClass(), "getHandle", new Class[0], player, new Object[0]);
Integer containerCounter = (Integer) ReflectionUtil.get(ep.getClass(), "containerCounter", ep);
if(containerCounter != null)
{
    ProtocolManager manager = ProtocolLibrary.getProtocolManager();
    final PacketContainer packet = manager.createPacket(PacketType.Play.Server.OPEN_WINDOW);
    packet.getIntegers().write(0, containerCounter);
    packet.getStrings().write(0, "minecraft:container");
    packet.getChatComponents().write(0, WrappedChatComponent.fromJson("{\"text\": \"" + title + "\"}"));
    packet.getIntegers().write(1, 9);
    try
    {
        manager.sendServerPacket(player, packet);
        // One downside: the clientside Inventory will be empty. Luckily,
        // this allows up to force Spigot to update this in our stead :P
        player.updateInventory();
    }
    catch (InvocationTargetException ex)
    {
        ex.printStackTrace();
    }
}
imWillX commented 6 years ago

I'll be sticking to the temporary solution. I don't have time to investigate atm.

mibby commented 6 years ago

RE: https://github.com/Hazebyte/CrateReloaded/issues/113

@imWillX Had a user report a few animation issues as of CrateReloaded 1.3.98.3.

https://i.gyazo.com/d3e820808ebc026e5834060ac6311254.mp4 The line shuffled over except for the center item.

https://i.gyazo.com/66248ef92830a3583ca2be6ff13b86a3.mp4 There were 2 items in the same spot. The shovel and elf head both co-existed until it finished the end rotation.

https://i.gyazo.com/ebc2f691483b6ee024b07d2a0a43a896.mp4 Robbed of the last tick rotation. It stopped rotating 1 tick before ending. When it was seemingly going to land on a shovel, the rotation stopped while the center still updated. I know item reward calculation is done prior to the animation rotation even beginning, but some players may think they got robbed of winning something great when this happens. :P

TPS of the server is 20 and relatively stable -- no spikes. The ping of the player who tested is averaged ~100ms.

imWillX commented 6 years ago

@mibby

I could transfer over the animation from 2.0.0 and we could try testing that out. As of right now, with the new animation already written, I prefer not to investigate old code.