PyvesB / advanced-achievements

:fireworks: Popular plugin that adds unique and challenging achievements to Minecraft servers.
https://www.spigotmc.org/resources/83466
GNU General Public License v3.0
199 stars 215 forks source link

Block Break not tracking #985

Closed ImaGorilla closed 3 years ago

ImaGorilla commented 3 years ago

I have the latest update and my blocks are not being tracked anymore.

Here is my config: https://pastebin.com/FgCEB8aK

Every other category works as intended. Block breaks does not. The numbers don't increase anymore.

No console errors. Please help

PyvesB commented 3 years ago

Hello @ImaGorilla! 👋🏻

You did not fill the information from the issue template, this will unfortunately make helping you harder.

It would be surprising it it would be a bug with the new versions of the plugin, as the Breaks category has not been changed, and hundreds of users have been happily using the new versions for several weeks now. Surely such a critical bug would have been reported by now.

For peace of mind, I've tried your configuration on my server and everything seems to work as expected: 2021-03-19_07 39 14

What I would suggest would be that you double check your permissions and that you try removing all other plugins but Advanced Achievements to see if you can reproduce. A bug in another plugin update may have broken Bukkit's BlockBreakEvent which Advanced Achievements uses to track block breaks. 😉

ImaGorilla commented 3 years ago

So I've done some more testing and it seems VKAutoPickup is causing some issue with the BlockBreakEvent. Does the achievement plugin not support autopickup plugins?

ImaGorilla commented 3 years ago

Okay I've worked with the plugin dev for VKAutoPickup and done more testing.

Luckily VKAutoPickup has an EventPriorityMap EventPriorityMap: ItemSpawnEvent: "HIGH" PlayerFishEvent: "HIGH" VehicleDestroyEvent: "LOW" VehicleExitEvent: "LOW" BlockBreakEvent: "MONITOR" EntityDeathEvent: "NORMAL" TEBlockExplodeEvent: "LOW" When I set the BlockBreakEvent to MONITOR on my test server, the achievements tracked! However, when I set the same on my main server, they did not. I am guessing that advancedachievements tracks blocks at MONITOR level priority (the very last), and not LOWEST or LOW, since those didn't work. If there a way you could maybe add a eventproritymap for advancedachievements for issues like this? Or maybe is there a way to already do this?

Thanks!

PyvesB commented 3 years ago

I am guessing that advancedachievements tracks blocks at MONITOR level priority (the very last), and not LOWEST or LOW, since those didn't work.

All achievement listeners are set to MONITOR priority. According to the API reference for events:

If you want to act when an event happens, but not change the outcome, use MONITOR. It's really really important that you use MONITOR, or an event might get cancelled after you've acted on it [...]

Advanced Achievements complies with the Bukkit/Spigot API.

Here's a simple example of what could go wrong if Advanced Achievements stopped complying with the API. The Bukkit/Spigot ecosystem has hundreds of plugins that offer some form of block protection, for instance WorldGuard, Factions, GriefPrevention and many many others. If the priority of listeners was changed to anything else than MONITOR, Advanced Achievements would start incorrectly counting some block break events that are cancelled by these other plugins. Players could then repeatedly try breaking blocks in a protected zone and collect all Breaks achievements without even using up their tools. This is just one plugin abuse scenario, but you can probably imagine many more and for pretty much every achievement category.

Using anything else than MONITOR would essentially be discarding the Bukki/Spigot API guidance and introducing bugs in Advanced Achievements. This should not be done.

In my humble opinion, VKAutoPickup needs to fix their own implementation. The correct way to avoid items from dropping using the Bukkit/Spigot API would be to use setDropItems(false) (see here) on the BlockBreakEvent instance. Advanced Achievements and other plugins relying on these BlockBreakEvent should then happily work again if it's done this way. Alternatively, VKAutoPickup can integrate with the Advanced Achievements and call the incrementCategoryForPlayer method when it cancels the break events from players.