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

Conversations doubled #300

Closed
HollishKid opened this issue Mar 14, 2016 · 4 comments
Closed

Conversations doubled #300

HollishKid opened this issue Mar 14, 2016 · 4 comments
Assignees
Labels
Bug A bug in the code or in a documentation. Confirmed Something is confimed.

Comments

@HollishKid
Copy link

Hello Co0sh,

Betonquest seems to work almost perfectly with 1.9 update.
I simply get conversations launched twice when clicking npc, could this be linked to double hand?

Moreover, I got the following error:

[20:34:06] [pool-3-thread-527/WARN]: Exception in thread "pool-3-thread-527" 
[20:34:06] [pool-3-thread-527/WARN]: org.apache.commons.lang.UnhandledException: Plugin BetonQuest v1.8.2 generated an exception while executing task 1045839
    at org.bukkit.craftbukkit.v1_9_R1.scheduler.CraftAsyncTask.run(CraftAsyncTask.java:56)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1110)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:603)
    at java.lang.Thread.run(Thread.java:722)
Caused by: java.lang.NullPointerException
    at pl.betoncraft.betonquest.conversation.ConversationData.getPrefix(ConversationData.java:189)
    at pl.betoncraft.betonquest.conversation.Conversation$Starter.run(Conversation.java:451)
    at org.bukkit.craftbukkit.v1_9_R1.scheduler.CraftTask.run(CraftTask.java:53)
    at org.bukkit.craftbukkit.v1_9_R1.scheduler.CraftAsyncTask.run(CraftAsyncTask.java:53)
    ... 3 more

It seems this error came from the fact "no conditions" were true to enable conversation. Should this really throw an error in console?

Hollish

@RiledUpCrow RiledUpCrow changed the title 1.9 Conversations doubled Mar 14, 2016
@RiledUpCrow RiledUpCrow added the Confirmed Something is confimed. label Mar 14, 2016
@RiledUpCrow
Copy link
Contributor

Thanks for the report. A few other people reported this error, it seems that it has something to do with 1.9, maybe you're right.

@RiledUpCrow
Copy link
Contributor

Hey @HollishKid, do the doubled conversations still happen? If so, could you send me your Spigot version, BetonQuest version, list of hooked plugins (shown when plugins are enabling) and the package containing the doubling conversation? I don't know how to reproduce this bug.

@HollishKid
Copy link
Author

I'll get you all that by tonight.

@RiledUpCrow
Copy link
Contributor

I figured this out. It's because PlayerInteractEvent is called second time if the main hand does not do anything (in 1.9). So if you click on the NPC with empty hand, it will get called twice and the conversation will be started twice.

That wouldn't be a problem since the second conversation will not start if the player is already in another one, but...

Conversations are started asynchronously. If those two are started in separate threads it may happen that second one is started before the first one tells the plugin that it has already started.

The fix would be to start the conversation in a synch thread and switch to async after the check has been done.

@RiledUpCrow RiledUpCrow self-assigned this May 6, 2016
@RiledUpCrow RiledUpCrow added Bug A bug in the code or in a documentation. and removed Bug A bug in the code or in a documentation. labels Jan 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in the code or in a documentation. Confirmed Something is confimed.
Projects
None yet
Development

No branches or pull requests

2 participants