Skip to content

Conversation

@Doc94
Copy link
Member

@Doc94 Doc94 commented Dec 28, 2025

Close #13398 by adding a method for determinate the slot using a Vector, works like the same than the logic for ChiseledBookshelf

test:

@EventHandler
    public void onPlayerInteractEvent(PlayerInteractEvent event) {
        final Player player = event.getPlayer();
        final Block block = event.getClickedBlock();
        if (block == null) return;
        if (block.getState() instanceof Shelf shelfState) {
            player.sendMessage(Component.text("Shelf: " + shelfState.getSlot(event.getClickedPosition())));
        } else if (block.getState() instanceof ChiseledBookshelf chiseledBookshelfState) {
            player.sendMessage(Component.text("ChiseledBookshelf: " + chiseledBookshelfState.getSlot(event.getClickedPosition())));
        }
    }
javaw_2025-12-28_13-06-20.mp4

@Doc94 Doc94 requested a review from a team as a code owner December 28, 2025 16:07
@github-project-automation github-project-automation bot moved this to Awaiting review in Paper PR Queue Dec 28, 2025
Copy link
Contributor

@Lulu13022002 Lulu13022002 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move the implementation to a common class like CraftSelectable or something.
PlayerInteractEvent#getClickedPosition should probably be renamed too but it can be done in another PR to not rely on a deprecated method.

Also next time don't show an image for your test code it's not convenient.


@Override
public int getSlot(Vector clickVector) {
BlockFace facing = ((Directional) this.getBlockData()).getFacing();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not safe to cast the block data cause a plugin could simply change it to something else.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well i base in the other implementation... Maybe a Precondition.checkState can be good here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A precondition is better -1 has some other meaning here, i did the change for you. But from a quick look it's impossible for this method to return -1?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like but not sure.. i prefer keep that docs xd

@Doc94
Copy link
Member Author

Doc94 commented Dec 28, 2025

Can you move the implementation to a common class like CraftSelectable or something.
PlayerInteractEvent#getClickedPosition should probably be renamed too but it can be done in another PR to not rely on a deprecated method.

Also next time don't show an image for your test code it's not convenient.

Okay, for the common class just move all to a new class with static methods not?

@Doc94 Doc94 force-pushed the feature/13398-get-slot-shelf branch from 9c2054c to 961f590 Compare December 29, 2025 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Awaiting review

Development

Successfully merging this pull request may close these issues.

Implement getSlot method for Shelf block

2 participants