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

Fixed errors and ddded blacklist api methods #129

Merged
merged 9 commits into from
Feb 16, 2024
Merged

Conversation

alexdev03
Copy link
Contributor

Fixed packet events error
Fixed itemstack/equitment errors

@Pyrbu
Copy link
Owner

Pyrbu commented Feb 16, 2024

What are the blacklist methods for? If you want to define your own logic for how a specific NPC is to be shown to players then you can disable processing on it's entry and write your own processor that will only show it to the players you desire.

@alexdev03
Copy link
Contributor Author

What are the blacklist methods for? If you want to define your own logic for how a specific NPC is to be shown to players then you can disable processing on it's entry and write your own processor that will only show it to the players you desire.

Ok I think at this point only the itemstack fix should remain. Right?

@Pyrbu
Copy link
Owner

Pyrbu commented Feb 16, 2024

Ok I think at this point only the itemstack fix should remain. Right?

I'd prefer if you found the actual cause of the errors and changed that instead of just doing simple patch, do you have a stack trace of the error? I forgot which properties the issue occurs with.

@alexdev03
Copy link
Contributor Author

[16:37:43] [DefaultDispatcher-worker-1/WARN]: java.lang.ClassCastException: class org.bukkit.craftbukkit.v1_20_R3.inventory.CraftItemStack cannot be cast to class lol.pyr.znpcsplus.libraries.packetevents.api.protocol.item.ItemStack (org.bukkit.craftbukkit.v1_20_R3.inventory.CraftItemStack is in unnamed module of loader com.universeprojects.uloader.R @5b0a12b0; lol.pyr.znpcsplus.libraries.packetevents.api.protocol.item.ItemStack is in unnamed module of loader 'ZNPCsPlus-2.0.0.jar' @51045cb) [16:37:43] [DefaultDispatcher-worker-1/WARN]: at ZNPCsPlus-2.0.0.jar//lol.pyr.znpcsplus.entity.properties.EquipmentProperty.apply(EquipmentProperty.java:26) [16:37:43] [DefaultDispatcher-worker-1/WARN]: at ZNPCsPlus-2.0.0.jar//lol.pyr.znpcsplus.entity.EntityPropertyImpl.applyStandalone(EntityPropertyImpl.java:56) [16:37:43] [DefaultDispatcher-worker-1/WARN]: at ZNPCsPlus-2.0.0.jar//lol.pyr.znpcsplus.npc.NpcImpl.UNSAFE_refreshProperty(NpcImpl.java:135) [16:37:43] [DefaultDispatcher-worker-1/WARN]: at ZNPCsPlus-2.0.0.jar//lol.pyr.znpcsplus.npc.NpcImpl.setProperty(NpcImpl.java:158) [16:37:43] [DefaultDispatcher-worker-1/WARN]: at ZNPCsPlus-2.0.0.jar//lol.pyr.znpcsplus.npc.NpcImpl.setProperty(NpcImpl.java:151) [16:37:43] [DefaultDispatcher-worker-1/WARN]: at me.gabber235.typewriter.entries.cinematic.ZNPCData$DefaultImpls.handleInventory(ZNPCData.kt:67)

This is the stack trace.

@Pyrbu
Copy link
Owner

Pyrbu commented Feb 16, 2024

Also I've just realised the way you've done it will most likely just break more things than it fixes, the code expects the map to contain a specific type which is enforced by the type parameter of the set method which could cause even more class cast exceptions. You're also changing how the method works fundamentally because you got rid of the else that was before the set line.

@Pyrbu
Copy link
Owner

Pyrbu commented Feb 16, 2024

After looking at this for a while longer, this will probably be the least performance-effecting way of making these properties accessible through the API since if we changed the type of the properties they would have to be converted every time a metadata packet is sent, I will fix the other issue I mentioned with your code and merge it.

@Pyrbu
Copy link
Owner

Pyrbu commented Feb 16, 2024

Unfortunately what TypeWriter does here shouldn't even be possible and is what is causing this (I'm assuming its kotlin not respecting type parameters), however this is still very much an issue with this plugin because there is currently no way of setting ItemStack properties through the API. I will include your patch to fix this issue with plugins that mistakenly use the API like this but I will also make a new method in the API for specifically ItemStack properties. I can't simply change the type of the property to a Bukkit ItemStack because that would incur massive overhead when sending the metadata packets to each player since the item would have to be re-serialized for every viewer. There is another way of fixing this which is using a wrapper class around items kind of like we do with most of the other properties and while it may be cleaner than this fix, I think its still better to do it this way because the alternative will be extremely confusing for people using the API

@Pyrbu
Copy link
Owner

Pyrbu commented Feb 16, 2024

Not sure how I managed to fuck up my push so badly, we don't talk about it.

@Pyrbu Pyrbu merged commit 828a8a3 into Pyrbu:2.X Feb 16, 2024
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.

2 participants