garbagemule / MobArena

MobArena plugin for Minecraft
GNU General Public License v3.0
196 stars 150 forks source link

Support new way to provide potions on upgrade waves in 1.9? #291

Closed mibby closed 7 years ago

mibby commented 8 years ago

Currently upgrade waves are pointless since there are no potion IDs in 1.9. You can't provide potions to everyone. Need to look into an alternative way to provide potions for upgrade waves?

garbagemule commented 8 years ago

Perhaps it's time to revisit the idea of grantables.

All I know about potions is that "they don't work", but no one that I have spoken to has mentioned anything about how they are created programmatically now. That'd be the first step.

TheComputerGeek2 commented 8 years ago

I seem to recall that they use nbt fields rather than the damage value on the item. If you want, I can look up in more detail what the nbt structure is for them, although the bukkit api provides a nice interface for this with the item meta and potion meta interfaces. https://hub.spigotmc.org/javadocs/bukkit/org/bukkit/inventory/meta/ItemMeta.html https://hub.spigotmc.org/javadocs/bukkit/org/bukkit/inventory/meta/PotionMeta.html You can go about obtaining the PotionMeta from the item stack with getItemMeta as shown in the link below. https://hub.spigotmc.org/javadocs/bukkit/org/bukkit/inventory/ItemStack.html#getItemMeta%28%29 The getItemMeta should return a potion meta when the item is a potion. Then you can do whatever you like with the meta and then re apply it to the itemstack. As far as I can tell, this should work just fine in 1.9.

Is that of any assistance?

garbagemule commented 8 years ago

Great stuff! I've only taken a quick look at it, but it does look like this is the way to go about it. Do you know how mature Spigot's 1.9 builds (and more important, the API) are? I really don't want to spend time on something that'll break the next time they sneeze in Spigot land.

I suppose the only other step left would be the actual representations and the parsing. How do other plugins handle this change? Is there a common string representation for, say, a splash potion of healing?

TheComputerGeek2 commented 8 years ago

I believe that the section of the API that is used here has actually existed prior to 1.9 and has not been marked for removal in the near future. Furthermore, it seems to be part of the Bukkit API, not just the spigot implementation of it. I'm not sure on stability of spigot as a whole yet, but I think that part of it is stable.

As for parsing, I figure that we can base it off of an existing format that you are using: [<id>|<name>]:<data>:<amount> the data field could be expanded to accommodate for the potion info. I was thinking like [D|S]~<effects> the D or S would mean drinkable or splash respectively. Then, to expand <effect name>@<amplifier>@<duration>|<effect name>@<amplifier>@<duration> using the pipe to separate the different effects. What we would end up with is [<id>|<name>]:<effect name>@<amplifier>@<duration>|<effect name>@<amplifier>@<duration>:<amount>

Now, I realize that this is currently problematic for servers that use custom resource packs to create a use for potions with data values, or I at least would expect that some server does. So to resolve that, we can amend the [D|S]~<effects> format to [D|S]@<dv>~<effects> with the @<data value> being an optional part.

Or, if you really don't want to change the part of the existing format, we could append the potion information to the end of the sequence rather than shoving it in the middle. For example: [<id>|<name>]:<data>:<amount>:<effect name>@<amplifier>@<duration>|<effect name>@<amplifier>@<duration>

Thoughts? Not sure how other plugins are handling it, mainly because I don't know of many plugins that try to handle it in a single line without just compact yaml. But if you want to go the compact yaml route, by all means, go for it, it can just be a little difficult for some people to get use to. I figured that you would prefer to have minimal changes to the existing syntax.

Hope this helps.

garbagemule commented 8 years ago

I think I've come up with a tentative, fairly simple way to solve this. At this point in time, I'd rather release something that works, but may not be feature complete, than keep people waiting.

Basically, I'm leaving the potion kind to the actual material/ID part, so potion, splash_potion, or lingering_potion. Then, the data value part becomes the potion type (or effect), as it is now, but supporting only the PotionType enum names, e.g. instant_heal for a healing potion, poison for a poison potion, etc.

I would really love it if you guys could test out the item parser for me and report back with your findings. Here's a link to a new build: Super secret Dropbox link.

If you can, please test if the parser works both ways, i.e. reading from the config-file (class items, upgrade waves, etc.), and writing to it (with the setclass command).

Thanks!

Edit: Also, super sorry for keeping you guys waiting so long. Time flies when you don't have much of it xD

Bodyash commented 8 years ago

image Cant load plugin with my MEGA config from 1.8 server :sob:

Bodyash commented 8 years ago

Working PERFECT with Chest to Inventory items when class selecting!

Bodyash commented 8 years ago

Which slot in chest = shield slot?

garbagemule commented 8 years ago

Thanks @Bodyash. Could I get you to try and figure out which item is causing this NullPointerException?

Glad to hear the class chests are working. The fifth last slot is for the off-hand item.

Edit: Note that old potion data will NOT work here, because the data values are now names rather than IDs.

Bodyash commented 8 years ago

Lost my old config, trying to find

Bodyash commented 8 years ago

default Maybe that, idk, totally lost old config.

And i found some bugs in Chest to Class Inventory

there is no slot for shield? If u put Elytra to armor slot - elytra will be not autoequiped. And 1 bug with Shulker - he can teleport out of arena :D 2016-04-18_23 06 30

Bodyash commented 8 years ago

Any updates?

mibby commented 8 years ago

@garbagemule I'd also like an update if you have anything in the works. :) A fix for classes having potions (in config rather than class chest) and restocking waves granting potions is greatly needed! Primarily restock waves are important for some of my classes since granting potions can be done with the class chest workaround.

garbagemule commented 8 years ago

@mibby Have you tried the build I linked? Can you get stuff working with it?

mibby commented 8 years ago

@garbagemule I actually haven't yet, as I've been using slipcor's fork for this fix. https://github.com/garbagemule/MobArena/pull/288/commits/f11c45cfad83a7ea4843467f315024a3fbeb65e9

Unless your build includes that now?

How would your build support potions with higher amplification levels? As there is no PotionType for higher potion effects or the different potions that have various durations (i.e. poison for 5 or 11 seconds.) I assume it would only work something like this;

potion:SPEED splash_potion:instant_heal

garbagemule commented 8 years ago

That's included, yeah.

I think you might be right that only the "base" potions are supported. If you could test out if they work, I'll try to figure out a way to mix levels in with them as well. It'll probably just boil down to another colon and another number (however utterly unreadable that is).

mibby commented 8 years ago

Apologies for the late testing, been busy with work.

Funny enough, the build I have that is compiled against Slipcor's fork works fine with the old potion IDs in upgrade waves. I'm assuming this is because EssentialsX has a legacy potion remapper? [Essentials] Using 1.9+ BasePotionData provider as potion meta provider. https://github.com/drtshock/Essentials/commits/2.x

With your custom build, the old potion IDs don't work. (As one would assume since the old IDs were removed in MC.) :P

[18:24:48] [Server thread/WARN]: [MobArena] Failed to parse item: potion:8229:1
[18:24:48] [Server thread/WARN]: [MobArena] Failed to parse item: potion:16418:3
[18:24:48] [Server thread/WARN]: [MobArena] Failed to parse item: potion:16449:3
[18:24:48] [Server thread/WARN]: [MobArena] Failed to parse item: potion:16451:1
[18:24:48] [Server thread/WARN]: [MobArena] Failed to parse item: potion:16450:1
[18:24:48] [Server thread/WARN]: [MobArena] Failed to parse item: potion:16457:1
[18:24:48] [Server thread/WARN]: [MobArena] Failed to parse item: potion:8229:1
[18:24:48] [Server thread/WARN]: [MobArena] Failed to parse item: potion:16385:1

Adding the potions via the new method to the upgrade wave does seem to work, at least it does not cause any errors on config reload and arena join.

    waves:
      recurrent:
        upgrade1:
          priority: 5
          type: upgrade
          wave: 11
          frequency: 10
          upgrades:
            All: GOLDEN_APPLE, potion:SPEED, splash_potion:instant_heal, LINGERING_POTION:speed
          give-all-items: true

As well as adding them to a class via config style instead of classchest seems to work.

  Archer:
    items: potion:SPEED:1, splash_potion:instant_heal:1, LINGERING_POTION:speed:1

That is assuming this is the syntax it follows.

<potiontype>:<potioneffect>:<amountofpotions>

Besides that, I can not find any way to specify a potion type that is amplified or one that has a different duration. Such as the different splash potions of poison. Splash potion of poison for 0:45, splash potion of poison for 1:30, and splash potion of poison II for 0:21.

Perhaps until there is a better way to specify exact potions, add your own legacy potion meta provider? In the meantime, I've just been using classchests to specify different potions to classes and the EssentialsX remapper seems to work for the old IDs in upgrade waves with slipcor's forked build.

garbagemule commented 8 years ago

Great stuff!

Yeah, the new build shouldn't support old potion IDs, since it switches on the Material type and goes into "potion parsing" if it's one of the three types of potions. When parsing potions, it only accepts the potion types. I suppose a quick workaround would be to only use the new potion parsing if the data value is not a number, and the default item parsing otherwise.

Do you know if the EssentialsX's legacy mapper applies to all ItemStacks? It seems odd that slip's fork would work unless EssentialsX's mapper is completely ubiquitous.

Bodyash commented 8 years ago

Im Using EssentialsX on my server over 5 month. Better that original Essentials.

mibby commented 8 years ago

@garbagemule EssentialX's legacy mapper might be completely ubiquitous as I know slipcor didn't specifically fix anything regarding legacy potion IDs in his fork.

I'd assume this is where the remapper is located. https://github.com/drtshock/Essentials/blob/dcb14170b9f5b4097ee5925e4c794d70f897a59c/Essentials/src/com/earth2me/essentials/utils/PotionMetaUtil.java

https://github.com/drtshock/Essentials/tree/731455649e134a0a93f2af9b6566d21d26d571cf/nms/UpdatedMetaProvider/src/net/ess3/nms/updatedmeta

https://github.com/drtshock/Essentials/tree/731455649e134a0a93f2af9b6566d21d26d571cf/nms/LegacyProvider/src/net/ess3/nms/legacy

https://github.com/drtshock/Essentials/tree/731455649e134a0a93f2af9b6566d21d26d571cf/nms/NMSProvider/src/net/ess3/nms

slipcor commented 8 years ago

mibby, I did fix it :P tl;dr: don't use/read "custom potion effect" but set "main potion effect" - iirc

Bodyash commented 8 years ago

New mob type support 1.10 ?

garbagemule commented 8 years ago

@Bodyash If Spigot supports the mobs, MobArena does too. Since 0.97, MobArena only needs mob updates for "special" mobs (like elder guardians, which are just guardians with a specific flag on them).

Bodyash commented 8 years ago
mibby commented 8 years ago

@slipcor :o When did you fix it? I thought this https://github.com/garbagemule/MobArena/pull/288 was the only change to fix class chest loading.

slipcor commented 8 years ago

Without putting money on it,  I'd say I fixed inventory assignment, regardless where the definition came from.  I am not sure if I even knew class chests existed back then oO

slipcor commented 8 years ago

Oh @mibby if you were talking about potions, I was referring to fixes in my plugin, not my pull to MA! 

mibby commented 8 years ago

@slipcor Fixes in your MA fork you sent me or are you talking about PA? :P I'm so confused.

slipcor commented 8 years ago

Class items = mobarena Potions = pvparena

mibby commented 8 years ago

Right, so you didn't fix potion ID loading for waves and such in MA with your fork then. Just class chest content loading. :P In other words, EssentialsX's legacy ID remapper may be completely ubiquitous and 'temporarily' allows for the old IDs to be used.

Though regardless of EssentialsX, I think MA should have its own remapper for legacy IDs for compatibility across all environments (not relying on EssX) or a system in place to properly allow all potion types of varying amplification and duration to be defined. (Current test build doesn't allow this.)

Bodyash commented 8 years ago

Any updates? 1.10.2 now, and no updates

slipcor commented 7 years ago

I just verified that this works on the latest version. For class items, upgrade waves, supply waves and for rewards.

I only tested the old-fashioned way though :P

potion:8197:1

No essentials involved, plain vanilla Spigot 1.11 and MobArena, nothing else!