SkriptLang / Skript

Skript is a Spigot plugin which allows server admins to customize their server easily, but without the hassle of programming a plugin or asking/paying someone to program a plugin for them.
https://docs.skriptlang.org
GNU General Public License v3.0
1.06k stars 367 forks source link

Add support for block data from itemtype #6087

Open Fusezion opened 1 year ago

Fusezion commented 1 year ago

Suggestion

Add itemtype support to block data of %block%

Why?

Bukkit allows the creation of block data from a block or material, some people do not have a block reference. And with skript not having a widely known system to get an itemtype from blockdata (type of coak_ore[]) I believe a better alternative is to simply allow itemtypes to be used within block data of... syntax.

Other

No response

Agreement

nylhus commented 1 year ago

Seems like something I could do

moraedu commented 1 year ago

Seems like something I could do

You'd have to expose the data field in NewBlockValues and only then do something along the lines of:

itemtype.stream(event)
.flatMap(ItemType::getTypes)
.map(item -> {
    NewBlockValues blockValues = (NewBlockValues) item.getBlockValues();
    return blockValues.getBlockData(); // exposed method
})
.toArray(BlockData[]::new);
TheLimeGlass commented 1 year ago

@nylhus This has been on my todo list, but you can take it.

Essentially you would use the BlockStateMeta in Bukkit and apply it to an itemstack. I have the exact syntax in Khoryl, and I was planning to add to Skript eventually. https://github.com/TheLimeGlass/Khoryl/blob/master/src/main/java/me/limeglass/khoryl/elements/item/EffApplyBlockStateItem.java

This allows for

set {_shield} to a shield
target block is a lime standing banner #can have banner patterns
apply the block state from the target block of player to {_shield}
set the offhand of the player to {_shield}

Do note that using itemtypes in the pattern does not work properly. I would recommend making it itemstack only! ItemType's store their ItemMeta until applied to an ItemStack, and when we utilized this syntax, we found out that itemtypes did not save the block state for some reason.

For this reason, I made it an expression which acts just like ExprLore and applies it like an expression returning the ItemStack. (Changes not applied to the Khoryl remote repo yet)

Similarly to BlockStateMeta there is also BlockDataMeta which is essentially the exact same class just for BlockData.

Fusezion commented 1 year ago

I believe the BlockStateMeta is off topic for what I wanted this suggest to be. You're taking it as should we add it to give player westward facing oak stairs all I want here is send blockdata of westward facing oak stairs. The addition of that should be left to another issue or pr.

TheLimeGlass commented 1 year ago

I believe the BlockStateMeta is off topic for what I wanted this suggest to be. You're taking it as should we add it to give player westward facing oak stairs all I want here is send blockdata of westward facing oak stairs. The addition of that should be left to another issue or pr.

They're very close to being the same class and the implementation is the same, it's a developer note.

Fusezion commented 1 year ago

I don't see how it's the same class, you're modifying 1 class not multiple. It's just going to ExprBlockData and adding itemtype and it's usage. You're suggesting it being done directly to ItemType class from what I understand

TheLimeGlass commented 1 year ago

I don't see how it's the same class, you're modifying 1 class not multiple. It's just going to ExprBlockData and adding itemtype and it's usage. You're suggesting it being done directly to ItemType class from what I understand

No, I said it shouldn't be done with ItemType.

Fusezion commented 1 year ago

Alright, so now I'm even further confused what you're talking about after rereading it.

  1. This suggest is to add block data of %itemtype% nothing about BlockStates
  2. if any form of change to item meta is to happen it's BlockDataMeta to support westward facing oak stairs to also fix issues like give play oak wall sign not giving an item, which I repeat is better for a whole other issue and pr.
  3. Your idea is way off of what I want this issue to even do...and am very confused where you got it from this.
TheLimeGlass commented 12 months ago

Alright, so now I'm even further confused what you're talking about after rereading it.

1. This suggest is to add `block data of %itemtype%` nothing about BlockStates

2. if any form of change to item meta is to happen it's BlockDataMeta to support `westward facing oak stairs` to also fix issues like `give play oak wall sign` not giving an item, which I repeat is better for a whole other issue and pr.

3. Your idea is way off of what I want this issue to even do...and am very confused where you got it from this.

BlockStateMeta and BlockDataMeta are very similar classes. I have an existing example using BlockStateMeta for banner data.

I get that this suggestion is BlockDataMeta, I'm helping aid an example of usage, you just swap BlockStateMeta for BlockDataMeta.