DecentSoftware-eu / DecentHolograms

A lightweight but powerful hologram plugin with many features and configuration options.
https://www.spigotmc.org/resources/96927/
GNU General Public License v3.0
211 stars 101 forks source link

Loading Denizen placeholder cause async issue #159

Closed Josh65-2201 closed 1 year ago

Josh65-2201 commented 1 year ago

Just making sure

Reproduction

  1. Install Denizen https://ci.citizensnpcs.co/job/Denizen/
  2. Add %denizen_<player.flag[leaderboardOne].if_null[0]>% to hologram line
  3. See PlaceholderAPI show async access error

Solution

No response

Server Version

Paper 1.20.1 build 196

Client Version

1.20.1

Plugin Version

2.8.4

Log

https://pastebin.com/F8xk6sjb

Andre601 commented 1 year ago

I feel like this is an issue with Denizen itself and not DH.

All DH does is pass the lines to PlaceholderAPI to parse any placeholders in it. It does it on an async threat because that's just better for performance. Running it on the main thread - which would be the only alternative - would be awful design as it could tank performance hard if there are a lot of players and/or placeholders to handle here...

So having Denizen complain it's not sync and/or on main thread is honestly just a stupid design and shows bad optimization on their end.

Tho, take this with a grain of salt, as I'm no expert in this entire sync vs async thing. Tho I so far have never seen any complains about DH's async placeholder parsing causing issues.

d0by1 commented 1 year ago

Hello, Andre is absolutely correct. I don't see a reason why the placeholders should only be updated synchronously. Parsing placeholders on the main thread would just cause performance issues. With that said, I'm not going to implement this change just because one plugin complains. You will need to find another solution (or report this to Denizen's developers).

davight commented 11 months ago

I don't see a reason why the placeholders should only be updated synchronously.

Valid denizen placeholder may contain tags that go through some Bukkit API methods (nothing in Bukkit is async safe unless documentet as safe) which is exactly why they should be parsed sync

mcmonkey4eva commented 11 months ago

See for reference the giant screenshot for explanation for why Denizen gives a warning on async placeholders it can technically be disabled but it is entirely a fault with DecentHolograms using unsafe async, Denizen isn't causing an error rather it's trying to warn you that something's gone wrong and save you from it. Use async here regardless under your own willful self-destruction.

2fdd06