bergerhealer / BKCommonLib

An extensive library used in bergerhealer's plugins
Other
181 stars 45 forks source link

parseMaterial replaces underscores - names can't match MAT_NAME_MAP entry #52

Closed JohReher closed 6 years ago

JohReher commented 6 years ago
BkCommonLib version: 1.13.1-v1
Spigot version: 1.13.1

Problem or bug:

Since 1.13 the method com.bergerkiller.bukkit.common.utils.ParseUtil.parseMaterial(String text,Material def,boolean legacy) doesn't parse IDs anymore. So if you want to get e.g. an iron block as a Material you would have to call the method with 'iron_block' instead of '42' as text parameter. But the MAT_ALIASES replacement eliminates every underscore, so the lookup in MAT_NAME_MAP tries to find IRONBLOCK instead of IRON_BLOCK. Since IRONBLOCK is no valid entry the parsing fails and it returns the default.

Expected behaviour:

A lookup with a materialname which contains an underscore should not be compromised by the alias mapping.

Steps to reproduce:

I used TrainCarts-1.13.1-v1 for reproducing this issue. I set up a test railway, spawning single storage carts with TrainCarts-Sign & Button and set up a chest next to the rails. I filled the chest with ironblocks and put a TrainCart-Sign next to the rails with following content:

  1. [+train]
  2. chest out
  3. iron_block

The storage cart didn't collect anything out of the chest. So I debugged the plugin and got to the point that iron_block was changed to IRONBLOCK and that there was no match in MAT_NAME_MAP.

Workaround

I just removed the Mapping "_" to "" from the MAT_ALIASES. This works for me, but I'm not sure, if this affects anything else.

bbayu123 commented 6 years ago

... there is no match in MAT_NAME_MAP because IRON_BLOCK is not entered into that Map. According to my understanding of the source code, the change is intentional and correct.

The matching of "iron_block" to Material.IRON_BLOCK is not done in MAT_NAME_MAP, it's done in parseArray.

bergerkiller commented 6 years ago

What should work is using LEGACY_IRON_BLOCK instead. LEGACY_IRON_BLOCK also works on versions 1.12.2 and earlier. The idea was that plugins can use a 1.13-minded api without worrying about alternative material names on 1.12.2.

I don't know how I feel about matching legacy material types when no legacy_ is prefixed, as it can cause confusing issues matching the legacy material instead of a post-1.13 material.

JohReher commented 6 years ago

Thanks bbayu123, that's right, it should match in parseArray. So it's not the underscore I have problems with.

Just set up a new server just to reproduce it on a new spigot installation and there it's working. So I think I messed something up with my server configuration.