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

BlocksRegistry API #40

Open
osipxd opened this issue May 21, 2023 · 7 comments · May be fixed by #44
Open

BlocksRegistry API #40

osipxd opened this issue May 21, 2023 · 7 comments · May be fixed by #44

Comments

@osipxd
Copy link
Member

osipxd commented May 21, 2023

Add API similar to ItemsRegistry but for custom blocks.

Features:

  • check if a custom block with a given name exists
  • get the block id (as string) of a Block or ItemStack (for blocks in the hand of a player)
  • create an ItemStack for a given block id and amount
  • place a custom block with given id at a destination Block
@knokko
Copy link

knokko commented May 21, 2023

More specifically, I would propose the following interface. (This is just my first draft and by no means finalized.)

public interface BlocksRegistry<ItemStackT : Any, BlockT : Any> : MimicService {

    /** Returns all known block IDs. */
    public val knownIds: Collection<String>

    /** Returns `true` if given [block] represented with given [blockId]. */
    public fun isSameBlock(block: BlockT, blockId: String): Boolean = getBlockId(block) == blockId

    /** Returns `true` if given [blockItem] is an item stack containing the block represented with given [blockId]. */
    public fun isSameBlockItem(blockItem: ItemStackT, blockId: String): Boolean = getBlockItemId(blockItem) == blockId

    /** Returns `true` if block with given [blockId] exists. */
    public fun isBlockExists(blockId: String): Boolean

    /** Returns ID representing given [block], or `null` if the ID not found in this registry. */
    public fun getBlockId(block: BlockT): String?

    /** Returns ID representing the block contained in given [blockItem], or `null` if the ID not found in this registry. */
    public fun getBlockItemId(blockItem: ItemStackT): String?

    /** Returns item containing block by given [blockId], or `null` if the ID not found in this registry. */
    public fun getBlockItem(blockId: String): ItemStackT? = getBlockItem(blockId, payload = null, amount = 1)

    /**
     * Returns item containing block with specified [payload] by given [blockId], or `null` if the ID not found in this registry.
     *
     * If [payload] is not `null`, block will be configured using it.
     */
    public fun getBlockItem(blockId: String, payload: Any?): ItemStackT? = getBlockItem(blockId, payload, amount = 1)

    /**
     * Returns item containing block with specified [amount] by given [blockId], or `null` if ID not found in this registry.
     *
     * If given [amount] is greater than maximum possible, will use maximum possible amount.
     * Amount shouldn't be less than `1`.
     */
    public fun getBlockItem(blockId: String, amount: Int): ItemStackT? = getBlockItem(blockId, payload = null, amount)

    /**
     * Returns item containing block with specified [amount] and [payload] by given [blockId],
     * or `null` if ID not found in this registry.
     *
     * If given [amount] is greater than maximum possible, will use maximum possible amount.
     * Amount shouldn't be less than `1`.
     *
     * Given [payload] may be used to configure block.
     */
    public fun getBlockItem(blockId: String, payload: Any?, amount: Int): ItemStackT?

    /** 
     * Places block by given [blockId] at [destination].
     * 
     * Returns `true` if the block was placed, and `false` if ID not found in this registry.
     */
    public fun placeBlock(blockId: String, destination: BlockT): Boolean = placeBlock(blockId, payload = null, destination)

    /** 
     * Places block with specified [payload] by given [blockId] at [destination].
     * 
     * Given [payload] may be used to configure block.
     * 
     * Returns `true` if the block was placed, and `false` if ID not found in this registry.
     */
    public fun placeBlock(blockId: String, payload: Any?, destination: BlockT): Boolean
}

@osipxd osipxd linked a pull request Nov 30, 2023 that will close this issue
6 tasks
@osipxd
Copy link
Member Author

osipxd commented Nov 30, 2023

@knokko, the only thing I don't like in this interface is that blocks and items are mixed here. So we can theoretically get the same item both from ItemsRegistry and from BlocksRegistry 🤔

@knokko
Copy link

knokko commented Dec 1, 2023

The motivation for the whole block/item distinction is a bit complex, but here we go:

There are basically 2 sides here:

  • The implementer (the plug-in that implements BlockRegistry)
  • The consumer (a plug-in that wants to do something with custom blocks of other plug-ins)

Example usage of getBlockItem

The consumer is a plug-in that adds mining machines. When a mining machine breaks a custom block, the consumer would like to drop the right item (or put it in some chest). To do so, the consumer can use getBlockId to get the ID of the block that was mined, and use getBlockItem to the dropped block as item in a chest.

Example usage of getBlockItemId

The consumer is a plug-in that adds building machines. The building machine wants to build some structure with a given shape, using blocks that are stored inside a chest. Players can also store custom blocks in that chest. The consumer can scan the inventory of the chest, use getBlockItemId to get the ID of the stored block, and use placeBlock to actually place it (after which it decrements the stacksize of the placed block).

The benefits and drawbacks of mixing

From the point of view of the implementer, it may or may not make sense to mix blocks and items like this, depending on how the implementing plug-in is structured (most importantly, are custom blocks in the inventory considered to be custom items?).

From the point of view of the consumer, the mix makes perfect sense. At least in the examples, the consumer doesn't really care whether the block item is a custom item: he simply wants to convert blocks from/to item stacks. I don't think the consumer would like it when he additionally needs the ItemRegistry just to work with blocks.

@osipxd
Copy link
Member Author

osipxd commented Dec 1, 2023

The consumer is a plug-in that adds mining machines. When a mining machine breaks a custom block, the consumer would like to drop the right item (or put it in some chest). To do so, the consumer can use getBlockId to get the ID of the block that was mined, and use getBlockItem to the dropped block as item in a chest.

Hm. Are we sure the item used to place the block is the same item that should be dropped?
Moreover, a single block may drop many different items and experience orbs or other entities

Maybe it will be better to make methods similar by functionality to Bukkit methods?

If we need to access the item used for the block placement, we can slightly change the signature of this method getBlockItem(block: Block): ItemStack

The consumer is a plug-in that adds building machines. The building machine wants to build some structure with a given shape, using blocks that are stored inside a chest. Players can also store custom blocks in that chest. The consumer can scan the inventory of the chest, use getBlockItemId to get the ID of the stored block, and use placeBlock to actually place it (after which it decrements the stacksize of the placed block).

I agree this method is useful, but I consider renaming it to getBlockId to make it clear that the returned ID is a block ID, not item ID. Also, we can add method to place block using items:

/**
 * Places block associated with the given [itemStack].
 * Returns `true` if the block placed successfully, of `false` if there is no associated block.
 * Remember to reduce [itemStack] amount if you want to.
 */
fun placeBlock(itemStack: ItemStack): Boolean = placeBlock(getBlockId(itemStack))

@knokko
Copy link

knokko commented Dec 2, 2023

I agree that the design of getBlockItem(blockID: String) was bad. I propose the following replacement/addition:

/**
 * Destroys the custom block at the given [block]. Nothing happens if the given [block] is not a custom block.
 *
 * If [droppedItems] is `null`, the drops of the custom block are spawned as items at the block location.
 *
 * If [droppedItems] is not `null`, the drops of the custom block are added to [droppedItems] rather than
 * spawned as items.
 */
fun breakBlock(block: Block, droppedItems: Collection<ItemStackT>)

This solution solves two problems:

  • I just realized we didn't have a method to break custom blocks yet, so breakBlock(block, null) is an easy way to break custom blocks.
  • Mining machines/lasers/far-range pickaxes can use the droppedItems parameter to catch the block drops rather than scanning through all items around the block after it's destroyed.

@osipxd
Copy link
Member Author

osipxd commented Dec 2, 2023

Yes, I thought about a similar method but with slightly different second parameter:

fun breakBlock(block: Block, dropsConsumer: DropsConsumer = DropsConsumer.drop())

fun interface DropsConsumer {
    /**
     * Called if as a result of [breakBlock] method, items should be dropeed. 
     * Should return `true` if the [drops] consumed, and `false` if it is not.
     * If `false` is returned, [drops] will be dropped on the ground near the destroeyed block.
     */
    fun consume(drops: Collections<ItemStackT>): Boolean
    
    /** Default consumers implementations. */
    companion object {
        /** Drops will not be consumed, and will be dropped on the ground. */
        @JvmStatic
        fun drop() = DropsConsumer { false }
        
        /** Drops just disappear and will not be dropped on the ground. */
        @JvmStatic
        fun disappear() = DropsConsumer { true }
    }
}

Do we need parameters entity and usedTool like in Bukkit's dropNaturally method?

And maybe instead of returning Boolean, consumer should return Collection<ItemStackT> containing unconsumed items (that going to be dropeed).

@osipxd
Copy link
Member Author

osipxd commented Dec 16, 2023

Current API version:

public interface BlocksRegistry<ItemStackT : Any, BlockT : Any> : MimicService {

    /** Returns all known blocks IDs. */
    public val knownIds: Collection<String>

    /** Returns `true` if the given [block] represented with the given [blockId]. */
    public fun isSameBlock(block: BlockT, blockId: String): Boolean = getBlockId(block) == blockId

    /** Returns `true` if the given [blockItem] is an item stack containing the block represented with the given [blockId]. */
    public fun isSameBlockItem(blockItem: ItemStackT, blockId: String): Boolean = getBlockIdFromItem(blockItem) == blockId

    /** Returns `true` if block with given [blockId] exists. */
    public fun isBlockExists(blockId: String): Boolean

    /** Returns ID representing given [block], or `null` if the ID not found in this registry. */
    public fun getBlockId(block: BlockT): String?

    /** Returns ID representing the block contained in given [blockItem], or `null` if the ID not found in this registry. */
    public fun getBlockIdFromItem(blockItem: ItemStackT): String?

    /**
     * Breaks the given [block]. Drops will be dropped on the ground.
     * Returns `false` if the block can't be broken or do not belongs to this registry.
     */
    public fun breakBlock(block: BlockT): Boolean = breakBlock(block, tool = null, DropsConsumer.drop())
    
    /**
     * Breaks the given [block] using the specified [tool]. Drops will be dropped on the ground.
     * Returns `false` if the block can't be broken or do not belongs to this registry.
     */
    public fun breakBlock(block: BlockT, tool: ItemStackT?): Boolean = breakBlock(block, tool, DropsConsumer.drop())

    /** 
     * Breaks the given [block] using the specified [tool] and passes drops to the given [dropsConsumer].
     * Returns `false` if the block can't be broken or do not belongs to this registry.
     */
    public fun breakBlock(block: BlockT, tool: ItemStackT?, dropsConsumer: DropsConsumer): Boolean

    /** 
     * Places block by given [blockId] at [destination].
     * 
     * Returns `true` if the block was placed, and `false` if ID not found in this registry.
     */
    public fun placeBlock(blockId: String, destination: BlockT): Boolean = placeBlock(blockId, destination, payload = null)

    /** 
     * Places block with specified [payload] by given [blockId] at [destination].
     * 
     * Given [payload] may be used to configure block.
     * 
     * Returns `true` if the block was placed, and `false` if ID not found in this registry.
     */
    public fun placeBlock(blockId: String, destination: BlockT, payload: Any?): Boolean
}

/** Resposible for consuming drops. */
public fun interface DropsConsumer {
    /**
     * Called if in result of [breakBlock] method, items should be dropeed. 
     * Returns unconsumed items to be dropped on the ground near the destroyed block.
     */
    public fun consume(drops: Collection<ItemStackT>): Collection<ItemStackT>

    /** Default consumers implementations. */
    public companion object {
        /** Drops will not be consumed, and will be dropped on the ground. */
        @JvmStatic
        public fun drop() = DropsConsumer { it }

        /** Drops just disappear and will not be dropped on the ground. */
        @JvmStatic
        public fun disappear() = DropsConsumer { emptyList() }
    }
}

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

Successfully merging a pull request may close this issue.

2 participants