Shopkeepers

Shopkeepers

2M Downloads

Problem when placing villager in claimed chunk

FlynnLG opened this issue ยท 4 comments

commented

Preliminaries:

  • Shopkeepers version: 2.14.0
  • Spigot version: 1.18.1 Paper-153

Reproduction on a fresh and up-to-date Spigot server:

I was able to reproduce my issue on a freshly set up and up-to-date Spigot server (currently 1.18.1 Paper-205) with the latest version of Shopkeepers (currently 2.14.0), with only the Claim-Chunk plugin and with no kinds of other server or client mods.

The issue:

I use the Chunk-Claim plugin from cjburkey01 and placing the shop will be not blocked in claimed chunks.

I enabled these in the ShopKeepers config:
bypass-spawn-blocking: true
check-shop-interaction-result: true
When I place normal blocks (and chests) in claimed chunks the event would be blocked and interaction with entities too. But when I place the shop with the shop item the shop is placed and the placing is not blocked. The interaction is blocked, only the placement of the villager is not blocked.
I think the problem is, that ShopKeepers checks for interaction with the villager but not when placing it. It would be really cool if the same if-statement from the interaction with the shop can be added to the shop placing event.

How to reproduce:

  1. Download version 0.0.23-RC7 from cjburkey01's plugin
  2. Download ShopKeeper v2.14.0
  3. Add the following configuration to the config.yml from ShopKeepers: config.txt
  4. A player (not you) must claim a chunk with /chunk claim
  5. Execute the command /shopkeeper give
  6. Place a chest on your/unclaimed chunk and link it with the the shop item
  7. Just place the shop on the chunk that another player just claimed (you cannot interact with the villager but you can place it)

Config:
config.txt

commented

I see a few possible solutions:

  • Add an integration with this ChunkClaim plugin to check if the shopkeeper spawn location is claimed by another player: However, I try to avoid this at all costs, since I don't want to add integrations with every region claiming plugin out there.
  • Trigger a dummy interaction event with the just spawned shopkeeper and remove the shopkeeper again if the interaction check fails. However, I would much rather prefer a solution that does not rely on first having to create the shopkeeper. Also, checking the interaction would also not work if something is preventing the shopkeeper from spawning (after it has already been created).
  • Check if the interaction with the shop creation item is cancelled by another plugin: We intentionally bypass other plugins by default when handling the interaction with the shop creation item, so this would require triggering another dummy interaction event to take other plugins into account. However, this would not be able to account for shopkeepers being created by other means, e.g. via command.
  • Trigger a dummy interaction or block break/place event with the block at the spawn location and check if this event is cancelled by some plugin.
    • Using an interaction even here has the benefit that it would be less likely to mess with block logging plugins. There is also a BlockCanBuildEvent, but this seems to only be for checking if one type of block can be placed against another one (regardless of the actual location of the placed block), and most region plugins seem to ignore this event anyways.
    • This would work similar to how we check chest access.
    • However, there is one 'difficulty'/problem with this: This requires a non-air block at the spawn location, within the region of the protected chunk. The problem is: Shopkeepers can (intentionally) not only be placed on top of a block, but also against the side of a block. And there might not be any block for the shopkeeper to stand on. Or, in the case of sign shops, the block the sign is attached to might be outside the protected region.
      One option could be to temporarily place a dummy block at the spawn location and then use that for the dummy interaction or block place event.
commented

Hello @blablubbabc!
Thank you for your fast answer, but i didn't had time to answer... But yet I have :)
Of course many thanks for this much information and potential solutions. I thought about the problem and found a simple solution. I downloaded the repository and checked some classes for potential possibilities to fix that problem. In the class named CreateListener.java I added the following code between line 164 and 165.

if (!InteractionUtils.checkBlockInteract(player, clickedBlock)) {
	TextUtils.sendMessage(player, Messages.restrictedArea);
		return;
}

ShopKeeper will check if the placement of any block is possible and if not, the villager will not be placed. I think the code has been forgotten? Anyway thank you for your solution, especially for the potential solutions with the dummy block place!

Have a nice day!
Best regards,
Flynn_LG.

commented

Your suggested solution corresponds closely to my fourth suggestion above, except:

  • It checks the clicked block instead of the block location where the shopkeeper is actually going to be spawned. The latter may require the mentioned placement of a dummy block.
  • It only accounts for shopkeeper placements via the creation item (admins can enable the placement of player shops via command inside the config).

The primary purpose of my other mentioned 'solutions' above was to document other possible alternative solutions that have been considered, and why they have been declined.

I will keep this ticket open until this has been resolve inside the Shopkeepers plugin directly.

commented

Will be resolved with the next update with the new setting check-spawn-location-interaction-result (default: false)