Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ByteItems support #81

Closed
randombyte-developer opened this issue Feb 10, 2019 · 9 comments
Closed

Add ByteItems support #81

randombyte-developer opened this issue Feb 10, 2019 · 9 comments

Comments

@randombyte-developer
Copy link
Contributor

https://ore.spongepowered.org/RandomByte/ByteItems

https://github.com/randombyte-developer/byte-items

ByteItems saves ItemStacks to a database. Several plugins implement ByteItems to have a central database with ItemStack definitions. ByteItems has an API to retrieve ItemStacks:

https://github.com/randombyte-developer/byte-items/blob/20d456db28b90b2436eb30eba19fe02ff1f8afda/src/main/kotlin/de/randombyte/byteitems/ByteItemsApi.kt#L13

It has been requested several times to implement ByteItems support to VC. If you need any help, don't hesitate to contact me.

@liach
Copy link
Contributor

liach commented Feb 10, 2019

you should have written the api in java at least. why can't byteitems support vc instead as vc is more frequently used?

@randombyte-developer
Copy link
Contributor Author

randombyte-developer commented Feb 11, 2019

why can't byteitems support vc instead as vc is more frequently used?

I don't know of any clean way to somehow interfere with VC's code from ByteItems. I could do a PR for VC myself, but I would like to be sure this is going to be merged. Maybe even get a little feedback where to implement it into VC best. Once it is decided that VC will add ByteItems support, I can do an API for Java.

@liach
Copy link
Contributor

liach commented Feb 11, 2019

afaik zzzz is friendly to prs. feel free to open one

@randombyte-developer
Copy link
Contributor Author

randombyte-developer commented Feb 11, 2019

I am trying to figure out what would be the best and non-intrusive way to put ByteItems into there.

That's an existing config:

Item {
      Count = 1
      ItemType = "minecraft:wheat"
      UnsafeDamage = 0
      DisplayName = "&lWheat seller and consumer"
      ItemLore = [
        "&e9 wheats => $8"
      ]
    }

Options:

  1. Check if the ItemType node starts with byte-items:, then get the ItemStack from ByteItems. And also apply the Count and any other data (how it is currently done in VC). Just leave out the UnsafeDamage.
  2. Add an additional node, something like ByteItems and check if it exists and use that. The ItemType could then be removed, since it doesn't do anything anymore.

I am more for the first option. Any feeback?

Edit:

So:

Item {
      Count = 12
      ItemType = "byte-items:my-special-item"
      DisplayName = "&lWheat seller and consumer"
      ItemLore = [
        "&e9 wheats => $8"
      ]
    }

It would still respect the Count=12 and the name and lore.

@SirFancyBacon
Copy link

I like option 2 personally, it seems cleaner from a user config standpoint; it is similar to using modded items you know?

While you're in there... If you could add multi item support that sure would be swell 8)

@randombyte-developer
Copy link
Contributor Author

When going for option 2 I guess it could cause confusion on the user side if 'ItemType' and 'ByteItems' is present.

What exactly is multi item and how does this relate to ByteItems?

@SirFancyBacon
Copy link

SirFancyBacon commented Feb 12, 2019

I look at it like this; Current nomenclature is ItemType = "mod:id" like ItemType = "conquest:bronze_ingot" so using ItemType = "byte-items:bronze_ingot" (or however Byte-Items ID works) makes the most sense to me.

Multi item support would be this right here: #74

It does not at all relate to byte items and i am simple highjacking in hopes that you would look at it and be so kind as to implement it :D

Though once you update Command-Utils to accept the "has in inventory" and "have given from inventory" it won't really be needed by me; yes, i am that guy from the discord lol.

@SirFancyBacon
Copy link

Oh i also see i had that backwards; i have been speaking about option 1 this whole time...... 😅

@randombyte-developer
Copy link
Contributor Author

I am in no way familiar with the VC code base. I don't think I am going to implement that.

Glad that option 1 seems reasonable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants