fix: improve block drop handling for storage containers and panels#22
fix: improve block drop handling for storage containers and panels#22
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08c45cabc5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR aims to improve how custom “panel” blocks and “storage container” blocks behave when broken, particularly around item drops and cleanup of persistent block metadata.
Changes:
- Moves persistent-data cleanup for panels/containers into a
BlockDropItemEventMONITOR handler. - Renames/splits drop handlers into panel-specific and storage-container-specific paths.
- Introduces a
dropDiffsstructure intended to drive storage updates on container break.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
src/main/java/de/janschuri/lunaticstorage/gui/ContainerListGUI.java:242
destroy(Location)removes the GUI fromguiCacheand cancels its task but doesn’t close the inventory for current viewers. Given the newcloseContainerListGUIs(...)helper (and its use from listeners), it’s easy to call these in the wrong order; consider havingdestroy(...)close viewers itself before removing/cancelling.
public static void destroy(Location loc) {
ContainerListGUI gui = guiCache.remove(loc);
if (gui != null && gui.task != null) {
gui.task.cancel();
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Map<ItemStack, Integer> difference = Utils.itemStackArrayToMap(inventory.getContents(), true); | ||
| dropDiffs.put(block, difference); |
There was a problem hiding this comment.
There is a potential memory leak in the dropDiffs map. When a storage container is broken (at line 71), its inventory data is stored in dropDiffs and should be cleaned up in onStorageContainerBlockDrop (line 182). However, if the BlockDropItemEvent is cancelled by another plugin, throws an exception, or doesn't fire for any reason, the entry will never be removed. Over time, this could accumulate orphaned entries. Consider adding defensive cleanup logic, such as a scheduled task to remove stale entries after a timeout, or using weak references.
No description provided.