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.

Sign up for GitHub

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

fixes and stuff #34

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

fixes and stuff #34

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a524d6c
fix error when trying to create a inv with over 54 slots (buying max …
PhanaticD Jan 29, 2020
cb811a6
turn off import grouping
PhanaticD Jan 29, 2020
e8dfacd
wrong spot
PhanaticD Feb 3, 2020
67858e7
dont display items for all double chests
PhanaticD Feb 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/java/com/robomwm/prettysimpleshop/command/BuyCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ public boolean buyCommand(Player player, int amount, boolean confirm)
shopInfo = new ShopInfo(shopInfo, itemStack.getAmount());
player.getServer().getPluginManager().callEvent(new ShopBoughtEvent(player, shopInfo));

int rows = ((itemStack.getAmount() / itemStack.getMaxStackSize()) + 1) / 9 + 1;
int rows = Math.min(((itemStack.getAmount() / itemStack.getMaxStackSize()) + 1) / 9 + 1, 54);
ShopInventoryHolder shopInventoryHolder = new ShopInventoryHolder();
Inventory inventory = player.getServer().createInventory(shopInventoryHolder,
rows * 9,
Expand Down
66 changes: 49 additions & 17 deletions src/main/java/com/robomwm/prettysimpleshop/feature/ShowoffItem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,7 @@
import com.robomwm.prettysimpleshop.event.ShopSelectEvent;
import com.robomwm.prettysimpleshop.shop.ShopAPI;
import com.robomwm.prettysimpleshop.shop.ShopInfo;
import org.bukkit.Chunk;
import org.bukkit.ChunkSnapshot;
import org.bukkit.Location;
import org.bukkit.World;
import org.bukkit.*;
Copy link
Member

Choose a reason for hiding this comment

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

Any possibility you can tell your IDE not to do this? Otherwise I can just fix it before I merge then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there done

import org.bukkit.block.Chest;
import org.bukkit.block.Container;
import org.bukkit.block.DoubleChest;
Expand All @@ -25,6 +22,7 @@
import org.bukkit.event.block.BlockPlaceEvent;
import org.bukkit.event.entity.EntityPickupItemEvent;
import org.bukkit.event.entity.ItemDespawnEvent;
import org.bukkit.event.entity.ItemMergeEvent;
import org.bukkit.event.inventory.InventoryPickupItemEvent;
import org.bukkit.event.inventory.InventoryType;
import org.bukkit.event.world.ChunkLoadEvent;
Expand Down Expand Up @@ -82,7 +80,7 @@ private void saveCache()
}
}

@EventHandler
@EventHandler(ignoreCancelled = true, priority = EventPriority.MONITOR)
Copy link
Member

Choose a reason for hiding this comment

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

ChunkLoadEvent isn't cancelable so the first param is useless. Have you been able to determine what's going on here? And since the plugin is doing stuff to the chunk (albeit asynchronously) it really should be at highest priority at the max instead of monitor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you can change if you want

private void onChunkLoad(ChunkLoadEvent event)
{
if (!config.isWhitelistedWorld(event.getWorld()))
Expand Down Expand Up @@ -127,16 +125,18 @@ public void run()
boolean noShops = true;
for (Location location : blocksToCheck)
{
if (!location.getWorld().isChunkLoaded(location.getBlockX() >> 4, location.getBlockZ() >> 4))
if (!location.getWorld().isChunkLoaded(location.getBlockX() >> 4, location.getBlockZ() >> 4)) {
return;
}
Container container = shopAPI.getContainer(location);
if (container == null || !shopAPI.isShop(container, false))
continue;
ItemStack item = shopAPI.getItemStack(container);
if (item == null)
continue;
if (spawnItem(new ShopInfo(shopAPI.getLocation(container), item, plugin.getShopAPI().getPrice(container))))
if (spawnItem(new ShopInfo(shopAPI.getLocation(container), item, shopAPI.getPrice(container)))) {
noShops = false; //Shops exist in this chunk
}
}
if (noShops)
removeCachedChunk(chunk);
Expand All @@ -146,7 +146,7 @@ public void run()
}.runTaskAsynchronously(plugin);
}

@EventHandler(ignoreCancelled = true)
@EventHandler(ignoreCancelled = true, priority = EventPriority.MONITOR)
private void onChunkUnload(ChunkUnloadEvent event)
{
if (!config.isWhitelistedWorld(event.getWorld()))
Expand All @@ -157,7 +157,7 @@ private void onChunkUnload(ChunkUnloadEvent event)
while (locations.hasNext()) //can optimize later via mapping chunks if needed
{
Location location = locations.next();
if (location.getChunk() == event.getChunk())
if (location.getBlockX() >> 4 == event.getChunk().getX() && location.getBlockZ() >> 4 == event.getChunk().getZ())
Copy link
Member

Choose a reason for hiding this comment

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

What's up here? Is it possible for there to be two different Chunks to exist on getChunk which correspond to the same world/chunk coordinate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

might just be leftover, was just making sure

{
Item item = spawnedItems.get(location);
item.remove();
Expand All @@ -184,19 +184,23 @@ private void onDoubleChest(BlockPlaceEvent event)
return;
if (!config.isShopBlock(event.getBlock().getType()))
return;
new BukkitRunnable()
{

if(event.getBlockPlaced().getType() != Material.CHEST && event.getBlockPlaced().getType() != Material.TRAPPED_CHEST)
return;

new BukkitRunnable() {
@Override
public void run()
{
InventoryHolder holder = ((Container)event.getBlock().getState()).getInventory().getHolder();
public void run() {
Container container = shopAPI.getContainer(event.getBlock().getLocation());
InventoryHolder holder = container.getInventory().getHolder();
if (!(holder instanceof DoubleChest))
return;
DoubleChest doubleChest = (DoubleChest)holder;
despawnItem(((Chest)(doubleChest.getLeftSide())).getLocation().add(0.5, 1.2, 0.5));
despawnItem(((Chest)(doubleChest.getLeftSide())).getLocation().add(0.5, 1.2, 0.5));
despawnItem(((Chest)(doubleChest.getRightSide())).getLocation().add(0.5, 1.2, 0.5));
spawnItem(new ShopInfo(shopAPI.getLocation(container), shopAPI.getItemStack(container), shopAPI.getPrice(container)));
}
}.runTask(plugin);
}.runTaskLater(plugin, 1L);
Copy link
Member

Choose a reason for hiding this comment

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

What's up with the runnable here, are doublechests not being properly handled anymore?

Also, runTask(plugin) is equivalent to runTaskLater(plugin, 1)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

didnt seem to be ever handled for when they were made or broke

Copy link
Member

Choose a reason for hiding this comment

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

Ok, so what you actually mean is you respawn the shop item when the doublechest is created instead of just having it despawn (I guess I was under the impression that I had another method cover this - that, or it's that I wasn't able to determine if the new doublechest is supposed to be a shop or not. The left side overrides, so the resulting doublechest may not be a shop, and thus the newly-spawned item won't be able to be despawned until the chunk is unloaded.)

}

@EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST)
Expand Down Expand Up @@ -233,9 +237,37 @@ private void onShopOpen(ShopOpenCloseEvent event)
private void onShopBreak(ShopBreakEvent event)
{
despawnItem(event.getShopInfo().getLocation().add(0.5, 1.2, 0.5));
Container container = shopAPI.getContainer(event.getShopInfo().getLocation());
InventoryHolder holder = container.getInventory().getHolder();
if (!(holder instanceof DoubleChest)) {
return;
}
new BukkitRunnable() {
@Override
public void run() {
spawnItem(new ShopInfo(shopAPI.getLocation(container), shopAPI.getItemStack(container), shopAPI.getPrice(container)));
}
}.runTaskLater(plugin, 1L);
Copy link
Member

Choose a reason for hiding this comment

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

Um, isn't the idea to despawn the item, not attempt to spawn another??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no

Copy link
Member

Choose a reason for hiding this comment

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

Um, then I'm not following why you made this change then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if you just cancel the despawn event (which is what was done before this change) it just creates a new item without the metadata tag

Copy link
Member

@RoboMWM RoboMWM Feb 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Ok, I don't think I'm following what's going on - I guess I'll have to review the code.

Perhaps this may help me better understand: what did you change, and why did you make this change?

}
@EventHandler
private void onItemDespawn(ItemDespawnEvent event)
{
if (event.getEntity().hasMetadata("NO_PICKUP")){
event.setCancelled(true);
new BukkitRunnable() {
@Override
public void run() {
despawnItem(event.getEntity().getLocation());
Container container = shopAPI.getContainer(event.getLocation().subtract(0,0.5,0));
if(container != null) {
spawnItem(new ShopInfo(shopAPI.getLocation(container), shopAPI.getItemStack(container), shopAPI.getPrice(container)));
}
}
}.runTaskLater(plugin, 1L);
}
}
@EventHandler
private void onItemMerge(ItemMergeEvent event)
{
if (event.getEntity().hasMetadata("NO_PICKUP"))
event.setCancelled(true);
Expand All @@ -256,7 +288,7 @@ private boolean spawnItem(ShopInfo shopInfo)
item.setCustomName(name);
item.setCustomNameVisible(true);
item.setVelocity(new Vector(0, 0.01, 0));
item.setMetadata("NO_PICKUP", new FixedMetadataValue(plugin, this));
item.setMetadata("NO_PICKUP", new FixedMetadataValue(plugin, "NO_PICKUP"));
spawnedItems.put(location, item);
cacheChunk(location.getChunk());
try //spigot compat (switch to Paper!)
Expand Down
1 change: 0 additions & 1 deletion src/main/java/com/robomwm/prettysimpleshop/shop/ShopAPI.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,6 @@ private boolean isSimilar(ItemStack item1, ItemStack item2)
*/
public Container getContainer(Location location)
{
location.getBlock();
BlockState state = location.getBlock().getState();
if (state instanceof Container)
return (Container)state;
Expand Down