Chestshop ghost blocks.

[DarkChromaMC]

General Troubleshooting

Hey there! Before you report a bug or suggest a new feature, please make sure to follow these steps first!

• [Yes] I have checked for similar issues.
• [Yes] I have updated to the latest TradeShop version.
• [Yes] I have checked the branches or the maintainers' PRs for upcoming features/bug fixes.

Template

Issue Type

• [Yes] Bug Report
• [ ] Feature Request

Description

I found a bug that is about the max limit. When there is a limit set in config, i.e. 128 chestshops per chunk, it probably works, but I noticed that if I use a plugin like Advancedregionmarket to create regions that players can buy shops in, and the shop resets and all chestshops within that region are cleared, the chestshops still exist as a ghost block, since I noticed some places blocks can't be placed. I double checked there weren't too many chests, by both counting and using the insights chunk scan plugin, so I'm pretty sure its something to do with chestshops not being removed when the region is cleared. Is there any way to fix this bug? The ARM plugin uses worldedit to reset regions, and worldguard to create the shops. However, removing chestshops through worldedit, leaves an invisible chestshop(s) behind.

Credits & Disclaimer

This file contains parts that were taken from the JDA project. We do not wish to take credit for those parts. Everything is licensed under the Apache License v2.0.

[SparklingComet]

Good catch, I'll see if I can get a fix out soon.

[KillerOfPie]

Removing the shops when the region is removed is simple, we may have problems if we also want to handle restoring the region without making the player recreate their shops.

ARM has its own events when these things happen. https://github.com/alex9849/advanced-region-market/tree/master/advancedregionmarket/src/main/java/net/alex9849/arm/events

The reason we don't catch these changes is because the chunks in the region are removed whole and the blocks themselves are not broken. https://github.com/alex9849/advanced-region-market/blob/ca401e1901a6be77d75533e68dcb8f196230714d/advancedregionmarket/src/main/java/net/alex9849/arm/regions/RegionManager.java#L84 https://github.com/alex9849/advanced-region-market/blob/ca401e1901a6be77d75533e68dcb8f196230714d/advancedregionmarket/src/main/java/net/alex9849/arm/regions/RegionManager.java#L853

I'm going to see if we can talk to the maker of ARM to find a better solution, but without that I think our best solution will be to just handle the removal of shops properly and require the player to manually remake the shops if the region is restored.

[KillerOfPie]

Or there's this that I just found, will look into this more https://www.spigotmc.org/resources/advancedregionmarket-shop-bridge.77484/

[SparklingComet]

Removing the shops when the region is removed is simple, we may have problems if we also want to handle restoring the region without making the player recreate their shops.

ARM has its own events when these things happen. https://github.com/alex9849/advanced-region-market/tree/master/advancedregionmarket/src/main/java/net/alex9849/arm/events

The reason we don't catch these changes is because the chunks in the region are removed whole and the blocks themselves are not broken. https://github.com/alex9849/advanced-region-market/blob/ca401e1901a6be77d75533e68dcb8f196230714d/advancedregionmarket/src/main/java/net/alex9849/arm/regions/RegionManager.java#L84 https://github.com/alex9849/advanced-region-market/blob/ca401e1901a6be77d75533e68dcb8f196230714d/advancedregionmarket/src/main/java/net/alex9849/arm/regions/RegionManager.java#L853

I'm going to see if we can talk to the maker of ARM to find a better solution, but without that I think our best solution will be to just handle the removal of shops properly and require the player to manually remake the shops if the region is restored.

The way I see it, there is two ways to effectively handle it.

1. The clean way: Find some way to listen to chunk resets/replacement without external APIs (alternative to BlockBreakEvent).
3. The hacky way: Edit the shop listeners to check if the block is no longer a chest or the sign that is meant to be somewhere are gone, and if so, silently remove shop information from data storage. Problem is, this could be more risky because a potential bug involving sign/chest detection could go unnoticed but also delete the shop data.

I'll do some research about (1) and see if I can find anything. If not, we might want to consider the options that are left. A potentially better alternative would be an official addon that deals with this, maybe by hooking into ARM's public API (assuming there is one), that only the users who have ARM need to install.

[SparklingComet]

Thanks @KillerOfPie for finding and sharing those two links. I think option (2) could be fairly straightforward because ARM provides a custom event RemoveRegionEvent that we can listen to with MONITOR priority and delete shop chunk data for the deleted chunk(s).

[KillerOfPie]

If we are listening to their events I was just thinking of adding a new listener that we can enable only when ARM is present.

From what I've been looking at it doesnt look like anything above ARM throws events for world changes like this.

Arm itself removes regions from a list/map so we'll probably have to handle this on a case by case basis. https://github.com/alex9849/advanced-region-market/blob/ca401e1901a6be77d75533e68dcb8f196230714d/advancedregionmarket/src/main/java/net/alex9849/arm/regions/RegionManager.java#L853

[SparklingComet]

If we are listening to their events I was just thinking of adding a new listener that we can enable only when ARM is present.

In general I find it easier to make an add-on instead if handling softdepends, especially because throwing in stuff into the main plugin which is needed by a very small fraction of the users is not worth it.

[KillerOfPie]

I can agree with that.

[SparklingComet]

Linking the new add-on Tradeshop/tradeshop-ARM which addresses this.