Bukkit, CraftBukkit, Spigot & BungeeCord 1.15.1

Discussion in 'News and Announcements' started by md_5, Dec 10, 2019.

  1. Glare

    Glare Previously ItsMeGlare

    1) Added validation against creation non-item itemstacks
    2) Corrected EnchantmentTarget values
    3) VehicleBlockCollisionEvent returned the wrong block

    Do you have any of the errors? I just tried this myself and didn't get any.
     
  2. at org.bukkit.craftbukkit.v1_15_R1.inventory.CraftItemStack.<init>(CraftItemStack.java:97) ~[spigot-1.15.1.jar:git-Spigot-037559e-e8cb2f5]
    at org.bukkit.craftbukkit.v1_15_R1.inventory.CraftItemStack.asCraftCopy(CraftItemStack.java:76) ~[spigot-1.15.1.jar:git-Spigot-037559e-e8cb2f5]

    yeh, bunch of those, loads of them have itemstack errors indeed.

    ugh, ok, well, guess ill spend my 8am trying to report it to developers.
     
  3. Glare

    Glare Previously ItsMeGlare

    From what I can see, this was all that changed. upload_2020-1-15_1-7-23.png
     
  4. Yeah I have heard from several people now that many of their plugins have broken due to this.
     
  5. I had to restore Monday's built of Spigot, too.
     
  6. Welp, good thing I haven't updated mine! I shall test it anyways (ofc backups!)
     
  7. Puremin0rez

    Moderator

    I'd love if somebody could give me links to some of the plugins that break from that change.
     
  8. Well one is Skript, but .... that's a whole other world :ROFLMAO: ... currently working on trying to fix this issue. Its our own damn fault
     
  9. Puremin0rez

    Moderator

    Yeah, any plugin that's broken by that change is entirely the devs fault (sorry). I was just curious to see how devs are abusing item stacks for that to ever be an issue :p
     
  10. Oh I totally agree, no argument there. I think all of MY plugins are safe (I freaking hope so)

    But yeah, Skript itself, don't even get me started, theres some really silly code hiding in here.
     
  11. md_5

    Administrator Developer

    And I would love if you could post some stack traces.....

    I'm open to reverting the change (temporarily) but I actually need to understand why plugins are doing this.

    My bet is that they're probably silently broken already and this is just making it visible.


    E: And explaning the change. Previously you could create an ItemStack with stuff that isn't an item (eg lava block), but when you actually tried to use this stack anywhere in game it would turn into air. This wasn't good behaviour (and stumped some people in the plugin dev forum wondering why air items were popping up) so now an exception is thrown instead. I personally can't think of a situation where trying to make eg a lava block item stack that actually just gets turned into an air stack is useful and not a plugin bug, but I am happy to be proven wrong.
     
    #371 md_5, Jan 15, 2020
    Last edited: Jan 15, 2020
    • Agree Agree x 3
    • Like Like x 1
  12. ItemFrameShops: https://www.spigotmc.org/resources/itemframeshops.4667/

    Code (Text):
    [06:59:57] [Server thread/ERROR]: Could not pass event WorldLoadEvent to ItemFrameShops v6.1.0
    org.bukkit.event.EventException: null
            at org.bukkit.plugin.java.JavaPluginLoader$1.execute(JavaPluginLoader.java:320) ~[server.jar:git-Spigot-037559e-752cf95]
    [...]
     
  13. In Skript (long story short) we parse aliases from a text file, and create ItemTypes... Im not sure WHY but the way its written is each item type (regardless if its an item or block) creates a new item stack, which is then passed off to block data(an internal class), here is a stack trace of our issue
    https://paste.md-5.net/pusezejobo.apache
     
    • Agree Agree x 1
    • Useful Useful x 1
  14. If it's the plugin dev's fault I didn't want to spam this forum with it. The log files are 5mb instead of under a meg. lol.

    Here's an example, i hope this helps. Thanks for replying.
    https://pastebin.com/6QYwrVUN

    sandbox:logs spiggy$ cat 2020-01-15-3.log |grep -c "Material must be an item"
    540
     
    • Useful Useful x 1
  15. md_5

    Administrator Developer

    It's a bit weird that ItemData is a class which stores an ItemStack and a BlockData together -- they're mutually exclusive. An Item can't have BlockData and a BlockData (unless its empty) can't have an ItemStack.

    I think fixing that may be a bit of work if ItemData is used for block aliasing at some point, probably counts in favour of temporarily reverting the change.

    Yeah that's just a one line fix, the plugin does effectively:
    Code (Text):

    for every Material:
    Create new ItemStack(Material)
     
    Which is obviously broken if Material is something like a Lava block that cannot be put in your inventory.
     
  16. Yeah don't even get me started on how silly this ItemType thing is. ItemType is more of like Skript's version of Material.
    If you do revert this change, I think anyone in the community will say thank you. But this will definitely light a fire under our asses to properly write this class to properly handle materials which are/are not blocks/items.
     
    • Agree Agree x 1
  17. I am also open to reverting it.. temporarily? eh, until 1.16? the why? i dunno, not a dev.. my argument would be: because no matter how we want or think plugins should be made, they're made how they are and tens of thousands -if we add the downloads up together- are using this, excluding skript stuff even.

    I didn't expect a change like this in between 1.15.1 and 1.15.1 versions though that breaks quite a few existing plugins, and doesn't feel too essential to me.

    Anyway, was just reporting it here that i noticed todays build giving me a panic attack at 6 am in the morning. and kept me busy for a few hours that i wish i spent differently.
     
  18. Maybe simply print out into console a couple of times informational message that plugin X should update their new ItemStack(Material) method usage to fit new requirements but still pass throw and return usual value of ItemStack like it was before that? And then in like 1.16 or later version go fully with that check? There are too many plugins without checks if the material is Item type before passing throw because it was not needed, now it requires you to check if the material is Item type each time you invoke this method, not a big deal and easily fixable, but still nuisance to deal with at this point. And plugins that are no longer supported by devs or getting slow updates in general will be more or less dead from this point on.
     
    • Agree Agree x 3
  19. Latest build this morning resolved the issue. I experienced what mrfloris did as well. CMI and jobs wouldn't load for me, possibly a few others. All good at this time.
     
  20. Back when I wrote Skript's current aliases system, I assumed that it was safe to use ItemStacks internally for everything. Skript used them in many other places too; we got it mostly isolated to one place in previous release a few days ago. Good thing we did, because now we actually have a realistic opportunity to get rid of this problem.

    The fact that Skript supports MC 1.9-1.15 with one build makes this somewhat complex. On 1.13+, both block data and item meta can't be present at the same time. On older versions, this is not always the case.