Skip to content

Potion fixes in items.csv#1448

Closed
dscalzi wants to merge 3 commits into
EssentialsX:2.xfrom
dscalzi:patch-1
Closed

Potion fixes in items.csv#1448
dscalzi wants to merge 3 commits into
EssentialsX:2.xfrom
dscalzi:patch-1

Conversation

@dscalzi
Copy link
Copy Markdown

@dscalzi dscalzi commented Aug 7, 2017

Fixes for splash potions

The item id for splash potions is 438, in the file they were listed as 373, which is the id of only a regular potion. This has been resolved.

Fixes for mundane potions

The metadata of mundane potions is 64.

To calculate the metadata of the splash mundane potions you use the general formula effect >> 32 | (tier << 5) | (extended << 6) | (upgraded << 13) | (splash << 14). This is the same formula used to calculate the metadata for splash awkward potions, and splash thick potions. Filling in the variables the formula becomes 64 >> 32 | (0 << 5) | (0 << 6) | (0 << 13) | (1 << 14) for mundane potions specifically.

See more information here https://minecraft.gamepedia.com/Potion#Base_potions

I have also removed the extended mundane potions as they do not exist in the game, and I have no idea where the id values listed for them in this file originated. It may be a good idea to cleanup this file and remove other potions which are non-existant in the game. Those are:

  • Any potion with an alias containing dualbit. More information on the link. They are non-existant in at least version 1.12. The Spigot API also prohibits their creation.
  • Individual non-existant potions listed below. Excuse the info messages, copied this list from my testing plugin's output.
    [00:34:11] [Server thread/INFO]: 373:6 clearpotion
    [00:34:11] [Server thread/INFO]: 373:7 clearextendedpotion
    [00:34:11] [Server thread/INFO]: 373:11 diffusepotion
    [00:34:11] [Server thread/INFO]: 373:13 artlesspotion
    [00:34:11] [Server thread/INFO]: 373:14 thinpotion
    [00:34:11] [Server thread/INFO]: 373:15 thinextendedpotion
    [00:34:11] [Server thread/INFO]: 373:22 bunglingpotion
    [00:34:11] [Server thread/INFO]: 373:23 bunglingextendedpotion
    [00:34:11] [Server thread/INFO]: 373:27 smoothpotion
    [00:34:11] [Server thread/INFO]: 373:29 suavepotion
    [00:34:11] [Server thread/INFO]: 373:30 debonairpotion
    [00:34:11] [Server thread/INFO]: 373:31 debonairextendedpotion
    [00:34:11] [Server thread/INFO]: 373:38 charmingpotion
    [00:34:11] [Server thread/INFO]: 373:39 charmingextendedpotion
    [00:34:11] [Server thread/INFO]: 373:43 refinedpotion
    [00:34:11] [Server thread/INFO]: 373:45 cordialpotion
    [00:34:11] [Server thread/INFO]: 373:46 sparklingpotion
    [00:34:11] [Server thread/INFO]: 373:47 sparklingextendedpotion
    [00:34:11] [Server thread/INFO]: 373:48 potentpotion
    [00:34:11] [Server thread/INFO]: 373:54 rankpotion
    [00:34:11] [Server thread/INFO]: 373:55 rankextendedpotion
    [00:34:11] [Server thread/INFO]: 373:59 acridpotion
    [00:34:11] [Server thread/INFO]: 373:61 grosspotion
    [00:34:11] [Server thread/INFO]: 373:62 stinkypotion
    [00:34:11] [Server thread/INFO]: 373:63 stinkyextendedpotion
    [00:34:11] [Server thread/INFO]: 373:8227 fireresistancepotion
    [00:34:11] [Server thread/INFO]: 438:16391 splashclearextendedpotion
    [00:34:11] [Server thread/INFO]: 438:16399 splashthinextendedpotion
    [00:34:11] [Server thread/INFO]: 438:16406 splashbunglingpotion
    [00:34:11] [Server thread/INFO]: 438:16407 splashbunglingextendedpotion
    [00:34:11] [Server thread/INFO]: 438:16411 splashsmoothpotion
    [00:34:11] [Server thread/INFO]: 438:16413 splashsuavepotion
    [00:34:11] [Server thread/INFO]: 438:16414 splashdebonairpotion
    [00:34:11] [Server thread/INFO]: 438:16415 splashdebonairextendedpotion
    [00:34:11] [Server thread/INFO]: 438:16422 splashcharmingpotion
    [00:34:11] [Server thread/INFO]: 438:16423 splashcharmingextendedpotion
    [00:34:11] [Server thread/INFO]: 438:16429 splashcordialpotion
    [00:34:11] [Server thread/INFO]: 438:16430 splashsparklingpotion
    [00:34:11] [Server thread/INFO]: 438:16431 splashsparklingextendedpotion
    [00:34:11] [Server thread/INFO]: 438:16432 splashpotentpotion
    [00:34:11] [Server thread/INFO]: 438:16438 splashrankpotion
    [00:34:11] [Server thread/INFO]: 438:16439 splashrankextendedpotion
    [00:34:11] [Server thread/INFO]: 438:16443 splashacridpotion
    [00:34:11] [Server thread/INFO]: 438:16445 splashgrosspotion
    [00:34:11] [Server thread/INFO]: 438:16446 splashstinkypotion
    [00:34:11] [Server thread/INFO]: 438:16447 splashstinkyextendedpotion

If you would like me to remove these before this PR is merged, let me know. I have a script written up which will do it quickly.

The metadata of mundane potions is 64.

To calculate the metadata of the splash mundane potions you use this formula `64 >> 32 | (0 << 5) | (0 << 6) | (0 << 13) | (1 << 14)`. This is the same formula used to calculate the metadata for splash awkward potions, and splash thick potions.

See more information here https://minecraft.gamepedia.com/Potion#Unbrewable_potions

I have also removed the extended mundane potions as they do not exist in the game, and I have no idea where the id values listed for them in this file originated.
@dscalzi dscalzi changed the title Fixing mundane potion meta. Potion fixes in items.csv Aug 7, 2017
@SupaHam
Copy link
Copy Markdown
Member

SupaHam commented Aug 7, 2017

Hi, thank you for taking the time to make this. I must ask, what current problem does this resolve?

My only concern is this PR would break pre 1.11 servers which represents 22% of our userbase.

If there is no immediate issue that needs resolving, I would highly recommend we delay this for the next update.

@SupaHam SupaHam added the type: enhancement Features and feature requests. label Aug 7, 2017
@dscalzi
Copy link
Copy Markdown
Author

dscalzi commented Aug 7, 2017

This doesn't fix a specific bug, it's mainly just correcting inconsistent data. One of my plugins uses this file to assign aliases to items, however potion support has always been broken. To fix this in my plugin specifically, I've been working on a conversion utility to convert this file into json format. The code performs a lot of validations on existing data and uses the metadata algorithm to identifiy which alias has which potion effect, since the metadata is no longer in the game. In the json format that this is converted to, the potion meta is explicitly saved in its own object. While performing these conversions I've noticed some incorrect data, specifically with mundane and splash potions, so I corrected those and thought I should share the fixes.

I'm going to see if I can run these validations for versions 1.9, 1.10, 1.11, and 1.11.2 also. A potential fix for the version issues in essentialsx would be to rename the file to items{version}.csv and to only use the items file specific to the loaded minecraft version. I also need to perserve compatibility in the plugin I mentioned above, so this is definately an issue I'm thinking about.

I don't have much more information to share about the .json format of this file at the moment, but I can keep you posted if it interests you.

@mdcfe
Copy link
Copy Markdown
Member

mdcfe commented Aug 7, 2017

A potential fix for the version issues in essentialsx would be to rename the file to items{version}.csv and to only use the items file specific to the loaded minecraft version.

I was actually surprised that this still hadn't been done up until this point. One file per version might not be necessary as item IDs may be compatible between versions, so perhaps one items.csv for a set of compatible versions such as one for 1.7.10-1.10, one for 1.11+ etc (these are just examples because I'm not sure exactly which item IDs have changed over time nor in which versions).

@dscalzi
Copy link
Copy Markdown
Author

dscalzi commented Aug 8, 2017

Also just a note, as it is this PR should still work with pre 1.11 servers. I went back as far as 1.9, and the id of splash potions in the game was still 438. As for the mundane potions, there was no splash variant. The potion metadata has not been in use for a while so the change on that will have no effect. Even if it did, this PR corrects the value.

@dscalzi
Copy link
Copy Markdown
Author

dscalzi commented Aug 19, 2017

I'm sure you guys have heard that in 1.13 items no longer have data values, which will break this file big-time. I've been working on some solutions to this issue but I'm not entirely sure of the best way to approach it. An idea that I'm liking a lot is converting this file to json-format. Example entries:

Potion:

{
      "materialSpigot": "SPLASH_POTION",
      "legacyID": "438",
      "legacyData": "16452",
      "versions": [
        "1.12"
      ],
      "potionMeta": {
        "effect": "POISON",
        "extended": true,
        "upgraded": false
      },
      "aliases": [
        "splashpoisonextendedpotion",
        "splashacidextendedpotion",
        "splashpoisonexpotion",
        "splashacidexpotion",
        "splashpoisonextendedpot",
        "splashacidextendedpot",
        "splashpoisonexpot",
        "splashacidexpot",
        "splpoisonextendedpotion",
        "splacidextendedpotion",
        "splpoisonexpotion",
        "splacidexpotion",
        "splpoisonextendedpot",
        "splacidextendedpot",
        "splpoisonexpot",
        "splacidexpot",
        "sppepot"
      ]
}

Regular Block:

{
      "materialSpigot": "OBSIDIAN",
      "legacyID": "49",
      "legacyData": "0",
      "versions": [
        "1.12"
      ],
      "aliases": [
        "obsidian",
        "obsi",
        "obby"
      ]
}

Tipped Arrow

{
      "materialSpigot": "TIPPED_ARROW",
      "legacyID": "440",
      "legacyData": "0",
      "versions": [
        "1.12"
      ],
      "potionMeta": {
        "effect": "NIGHT_VISION",
        "extended": false,
        "upgraded": false
      },
      "aliases": [
        "arrownightvision",
        "nightvisionarrow",
        "arrownv",
        "nvarrow"
      ]
 }

This format would take up more lines/filesize, however as a while I believe it's better than csv. Mojang is moving further away from numeric block data and more towards NBT states, which can be more easily stored in json format.

This format could also support different versions per block. I haven't finalized how that could function, however there are several ways. The first is included in the json snippets above. We include a versions array which will store the versions that this block is present on. This can become redundant, however.

Another option is to include a since parameter which will record the first version this block appeared in. If the block was removed there can be an additional parameter, maybe until, where we record the last version this block appeared in. If the block's data has changed, we could mark the old data using this versioning syntax, and create a new entry for it. An example of this would be like what happened with the different wood style doors in 1.12.

I've already written an ad hoc script to convert the existing data into this format, the only thing which would need to be written is a merge script to merge older item data. Let me know what you think.

@mdcfe
Copy link
Copy Markdown
Member

mdcfe commented Aug 19, 2017

The above comment would be better suited to a separate issue so we can keep track of discussions more clearly.

@TheIntelloBox
Copy link
Copy Markdown

Merge this ?

@mdcfe
Copy link
Copy Markdown
Member

mdcfe commented Jan 20, 2018

I can't see that this will be merged, as item data will change quite a bit with 1.13 and so will our handling of items.

@dscalzi
Copy link
Copy Markdown
Author

dscalzi commented Jan 20, 2018

I still recommend using the JSON format for item aliases, especially since data values are essentially JSON in the game.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: enhancement Features and feature requests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants