Add QuestItemAudienceEntry to enforce quest item retention#408
Add QuestItemAudienceEntry to enforce quest item retention#408RenaudRl wants to merge 14 commits intogabber235:developfrom
Conversation
Introduces QuestItemAudienceEntry, an audience entry that forces players to keep a specific quest item in their inventory. Prevents dropping, moving to containers, and losing the item on death, ensuring players retain the item throughout the quest.
Changed the color for the 'quest_item_audience' entry from GREEN to MEDIUM_SEA_GREEN to improve visual distinction.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
WalkthroughAdds a QuestItemAudience entry and display that enforce a specific quest item: assign it when a player joins, block drops/moves/crafting shifts/container transfers, remove it from death drops and restore on respawn, and clean up on player leave or dispose. Changes
Sequence DiagramsequenceDiagram
actor Player
participant Audience as QuestItemAudienceDisplay
participant Inventory as BukkitInventory
participant Events as BukkitEventSystem
rect rgb(220,235,255)
Note over Player,Audience: Player added to audience
Player->>Audience: onPlayerAdd(player)
Audience->>Inventory: ensure quest item in inventory (handle full inventory)
Audience-->>Player: notify if replacement occurred
end
rect rgb(255,235,220)
Note over Player,Events: Interaction prevention
Player->>Events: drop/click/shift-click/craft attempts
Events->>Audience: onItemDrop/onInventoryClick
Audience-->>Events: cancel/prevent action for quest item
end
rect rgb(220,255,230)
Note over Player,Events: Death & respawn
Player->>Events: PlayerDeath
Events->>Audience: onPlayerDeath
Audience->>Inventory: remove quest item from drops
Player->>Player: Respawn
Audience->>Inventory: re-add quest item to player
end
rect rgb(245,230,255)
Note over Player,Audience: Player removed from audience / dispose
Player->>Audience: onPlayerRemove(player) / dispose()
Audience->>Inventory: remove all quest item instances
Audience->>Audience: clear per-player tracking and unregister listeners
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
extensions/*Extension/**
📄 CodeRabbit inference engine (extensions/AGENTS.md)
When adding a new extension, create its project directory with a name ending in "Extension" (e.g., MyPluginExtension)
Files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
extensions/{*Extension/src/**/*.kt,*Extension/**/*.kts}
📄 CodeRabbit inference engine (extensions/AGENTS.md)
Use 4 spaces for indentation and wrap lines at 120 characters in Kotlin code and Gradle Kotlin scripts
Files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
extensions/*Extension/src/**/*.kt
📄 CodeRabbit inference engine (extensions/AGENTS.md)
extensions/*Extension/src/**/*.kt: Keep functions short and focused; prefer guard clauses over nested conditionals
Document public APIs with KDoc, focusing on when to use them rather than restating method behavior
Avoid inline comments; refactor unclear code instead of adding explanatory comments
Files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
🧠 Learnings (10)
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Ensure imports/types compile: Ref, Var, ConstVar, OnlyTags, and correct colors/icons/description in Entry
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Annotate all entry classes with Entry(id, description, Colors, icon); use Tags on definition entries; use OnlyTags on data lists to constrain allowed data types
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : InteractEntityObjective text may include <entity> placeholder; obtain positions by querying active AudienceEntityDisplay instances
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : If interaction support is needed, ensure EntityInteractEventEntry filters can target the new definition and InteractEntityObjective can resolve positions
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Keep visual and structural parity with similar entities (icons, colors, descriptions, class structure) like AllayEntity.kt or ArmorStandEntity.kt
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Choose correct activity scope: SharedEntityActivityEntry for shared behavior/position, IndividualEntityActivityEntry for per-viewer behavior/position
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Instance entries must implement SimpleEntityInstance (or an Advanced variant) with fields: id, name, definition: Ref<...Definition>, spawnLocation, data, and activity Ref of the correct scope
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Access Var values via .get(player) where applicable in activities and entries
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : LookAtEntityInteractionBoundEntry must force player look within radius, adjust movement speed via attribute modifiers, and remove modifiers on teardown
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Avoid mutating Bukkit entities directly; operate via WrapperEntity and Typewriter EntityProperty APIs
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
🔇 Additional comments (5)
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt (5)
32-57: LGTM!Entry definition follows project conventions with proper annotation, KDoc documentation, and clean implementation of
AudienceEntry.
59-67: LGTM!Thread-safe
ConcurrentHashMapand proper listener registration pattern.
90-100: LGTM!Cleanup logic correctly removes all matching quest items from inventory.
102-110: LGTM!Drop prevention correctly identifies and blocks quest item drops.
176-181: LGTM!Proper cleanup with listener unregistration and state clearing.
…ic/entries/audience/QuestItemAudienceEntry.kt Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt (2)
76-84: Critical: Assignment to defensive copy has no effect.Line 81 assigns to
storageContents[firstNonEmptySlot], which is a defensive copy of the inventory array. The assignment modifies the copy, not the actual inventory, so the quest item is never placed when the inventory is full.Apply this diff to fix:
if (firstNonEmptySlot >= 0) { val droppedItem = player.inventory.storageContents[firstNonEmptySlot] if (droppedItem != null) { player.world.dropItemNaturally(player.location, droppedItem) - player.inventory.storageContents[firstNonEmptySlot] = questItem + player.inventory.setItem(firstNonEmptySlot, questItem) player.sendMessage(inventoryFullMessage.parsePlaceholders(player).asMini()) } }
176-182: Respawn handling lacks safety checks.The delayed task has multiple reliability gaps:
- Player may disconnect before respawning—
event.entityreference could be stale- Player may have been removed from the audience during the death screen
addItem()may fail silently if inventory is full on respawn, losing quest itemsApply this diff to add validation and handle full inventory:
// Re-add the quest item when the player respawns + val playerId = event.entity.uniqueId server.scheduler.runTaskLater(plugin, Runnable { + val player = server.getPlayer(playerId) ?: return@Runnable + if (!contains(player)) return@Runnable questItemStacks.forEach { item -> - event.entity.inventory.addItem(item) + val leftOver = player.inventory.addItem(item) + if (leftOver.isNotEmpty()) { + // Drop at player location if inventory is full + player.world.dropItemNaturally(player.location, item) + } } }, 1L)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
extensions/*Extension/**
📄 CodeRabbit inference engine (extensions/AGENTS.md)
When adding a new extension, create its project directory with a name ending in "Extension" (e.g., MyPluginExtension)
Files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
extensions/{*Extension/src/**/*.kt,*Extension/**/*.kts}
📄 CodeRabbit inference engine (extensions/AGENTS.md)
Use 4 spaces for indentation and wrap lines at 120 characters in Kotlin code and Gradle Kotlin scripts
Files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
extensions/*Extension/src/**/*.kt
📄 CodeRabbit inference engine (extensions/AGENTS.md)
extensions/*Extension/src/**/*.kt: Keep functions short and focused; prefer guard clauses over nested conditionals
Document public APIs with KDoc, focusing on when to use them rather than restating method behavior
Avoid inline comments; refactor unclear code instead of adding explanatory comments
Files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
🧠 Learnings (10)
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Ensure imports/types compile: Ref, Var, ConstVar, OnlyTags, and correct colors/icons/description in Entry
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Annotate all entry classes with Entry(id, description, Colors, icon); use Tags on definition entries; use OnlyTags on data lists to constrain allowed data types
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Use snake_case, descriptive IDs (e.g., armor_stand_definition, allay_instance) and practical descriptions
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Keep visual and structural parity with similar entities (icons, colors, descriptions, class structure) like AllayEntity.kt or ArmorStandEntity.kt
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : InteractEntityObjective text may include <entity> placeholder; obtain positions by querying active AudienceEntityDisplay instances
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Instance entries must implement SimpleEntityInstance (or an Advanced variant) with fields: id, name, definition: Ref<...Definition>, spawnLocation, data, and activity Ref of the correct scope
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Choose correct activity scope: SharedEntityActivityEntry for shared behavior/position, IndividualEntityActivityEntry for per-viewer behavior/position
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Instance class <EntityName>Instance must be annotated with Entry and declare fields: id, name, definition: Ref<<EntityName>Definition>, spawnLocation, data (+OnlyTags as above), activity: Ref<out SharedEntityActivityEntry>
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Definition class <EntityName>Definition must be annotated with Entry (and optional Tags) and declare fields: id, name, displayName: Var<String>, sound: Var<Sound>, data: List<Ref<EntityData<*>>>
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : If interaction support is needed, ensure EntityInteractEventEntry filters can target the new definition and InteractEntityObjective can resolve positions
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
🔇 Additional comments (5)
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt (5)
32-57: LGTM!The entry annotation follows conventions with a descriptive snake_case ID, practical description, and appropriate icon. The KDoc effectively explains when to use this audience entry and its behavioral constraints.
59-67: LGTM!The use of
ConcurrentHashMapfor thread-safe player item tracking and proper event listener registration ininitialize()is correct.
90-100: LGTM!The cleanup logic correctly removes all instances of the quest item using
setItem()and properly identifies matching items withisSameAs().
102-110: LGTM!The drop prevention logic is correct and uses appropriate event priority.
184-189: LGTM!The disposal sequence correctly unregisters the listener and clears state, preventing memory leaks.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt (1)
132-173: Inventory protection gaps: cursor→container still bypasses, hotbar swaps allow container moves, and crafting table slots aren’t covered.There are a few issues in
onInventoryClickthat weaken the “cannot move to containers / cannot be crafted” guarantees:
Cursor→container on empty slot still bypasses protection (duplicate of earlier concern).
On Line 153,val currentItem = event.currentItem ?: returncauses an early return when clicking an empty container slot. In that casecurrentItemisnull, butcursorcan hold the quest item. The new cursor-check block at Lines 156–162 never runs, so clicking an empty chest slot with the quest item on the cursor still moves it into the chest.
- Scenario: Open chest, pick up quest item onto cursor, click empty chest slot → quest item ends up in container.
Number-key / hotbar swaps into containers are not blocked.
If the quest item is in a hotbar slot and the player clicks a container slot while pressing a number key, Bukkit will swap the hotbar item with the container slot. In this case:
clickedInventoryis the container,currentItemis the container’s slot item,- the quest item is only visible via
event.hotbarButton/player.inventory.getItem(...).
None of the current checks examine the hotbar slot, so the quest item can be moved into a container.Crafting table (WORKBENCH) grid isn’t covered.
The crafting guard currently only checksInventoryType.CRAFTING(the 2×2 grid in the player inventory). Using a crafting table (InventoryType.WORKBENCH) still allows the quest item to be placed in crafting input slots, which contradicts the “cannot be lost via crafting” requirement.A possible fix that addresses all three points:
val clickedInventory = event.clickedInventory ?: return - - // Block ANY interaction with crafting slots (slots 1-4 in CRAFTING inventory type) - if (clickedInventory.type == InventoryType.CRAFTING && event.slot in 1..4) { - val currentItem = event.currentItem - val cursorItem = event.cursor - - // Check if the item being placed or already in the slot is the quest item - if ((currentItem != null && entry.item.get(player).isSameAs(player, currentItem)) || - (cursorItem != null && entry.item.get(player).isSameAs(player, cursorItem))) { - event.isCancelled = true - return - } - } - - val currentItem = event.currentItem ?: return - val cursorItem = event.cursor - - // Prevent placing quest item from cursor into container - if (clickedInventory.type != InventoryType.PLAYER && clickedInventory.type != InventoryType.CRAFTING) { - if (cursorItem != null && !cursorItem.isEmpty && entry.item.get(player).isSameAs(player, cursorItem)) { - event.isCancelled = true - return - } - } - - // Prevent moving quest item to non-player inventories (containers) - if (clickedInventory.type == InventoryType.PLAYER && event.view.topInventory.type != InventoryType.PLAYER) { - if (entry.item.get(player).isSameAs(player, currentItem)) { - // Check if player is trying to move item to container - if (event.click.isShiftClick) { - event.isCancelled = true - } - } - } + + // Block ANY interaction with crafting slots (player 2x2 and workbench) when the quest item is involved + if ((clickedInventory.type == InventoryType.CRAFTING && event.slot in 1..4) || + (clickedInventory.type == InventoryType.WORKBENCH && event.slot in 1..9) + ) { + val currentItem = event.currentItem + val cursorItem = event.cursor + + if ((currentItem != null && entry.item.get(player).isSameAs(player, currentItem)) || + (cursorItem != null && entry.item.get(player).isSameAs(player, cursorItem)) + ) { + event.isCancelled = true + return + } + } + + val cursorItem = event.cursor + + // Prevent placing quest item from cursor into any non-player inventory (even on empty slots) + if (clickedInventory.type != InventoryType.PLAYER && clickedInventory.type != InventoryType.CRAFTING) { + if (cursorItem != null && !cursorItem.isEmpty && entry.item.get(player).isSameAs(player, cursorItem)) { + event.isCancelled = true + return + } + } + + val currentItem = event.currentItem ?: return + + // Prevent moving quest item to non-player inventories (containers) via shift-click from player inventory + if (clickedInventory.type == InventoryType.PLAYER && event.view.topInventory.type != InventoryType.PLAYER) { + if (entry.item.get(player).isSameAs(player, currentItem) && event.click.isShiftClick) { + event.isCancelled = true + return + } + } + + // Prevent moving quest item into containers via hotbar number-key swaps + if (event.hotbarButton in 0..8 && + clickedInventory.type != InventoryType.PLAYER && + event.view.topInventory.type != InventoryType.PLAYER + ) { + val hotbarItem = player.inventory.getItem(event.hotbarButton) + if (hotbarItem != null && entry.item.get(player).isSameAs(player, hotbarItem)) { + event.isCancelled = true + return + } + }This keeps normal player-inventory rearrangements intact while closing the remaining escape hatches to containers and crafting.
🧹 Nitpick comments (1)
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt (1)
69-107: SimplifyplayerItemstracking and remove unused value.
playerItemsis only used as a presence flag inonPlayerRemove(theItemStackvalue is never read), and all event checks usecontains(player)fromAudienceDisplay. You can simplify by dropping the map entirely and always cleaning up via an inventory scan on removal; that scan is cheap and safe even if no quest item was ever added.Suggested refactor:
- private val playerItems = ConcurrentHashMap<UUID, ItemStack>() - @@ override fun onPlayerAdd(player: Player) { @@ - playerItems[player.uniqueId] = questItem }) } @@ override fun onPlayerRemove(player: Player) { - val questItem = playerItems.remove(player.uniqueId) ?: return val entry = ref.get() ?: return // Remove all instances of the quest item from the player's inventory @@ override fun dispose() { super.dispose() unregister() - playerItems.clear() }This keeps behavior the same while removing state that doesn’t add value.
Also applies to: 110-120
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
extensions/*Extension/**
📄 CodeRabbit inference engine (extensions/AGENTS.md)
When adding a new extension, create its project directory with a name ending in "Extension" (e.g., MyPluginExtension)
Files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
extensions/{*Extension/src/**/*.kt,*Extension/**/*.kts}
📄 CodeRabbit inference engine (extensions/AGENTS.md)
Use 4 spaces for indentation and wrap lines at 120 characters in Kotlin code and Gradle Kotlin scripts
Files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
extensions/*Extension/src/**/*.kt
📄 CodeRabbit inference engine (extensions/AGENTS.md)
extensions/*Extension/src/**/*.kt: Keep functions short and focused; prefer guard clauses over nested conditionals
Document public APIs with KDoc, focusing on when to use them rather than restating method behavior
Avoid inline comments; refactor unclear code instead of adding explanatory comments
Files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
🧠 Learnings (11)
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Ensure imports/types compile: Ref, Var, ConstVar, OnlyTags, and correct colors/icons/description in Entry
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Annotate all entry classes with Entry(id, description, Colors, icon); use Tags on definition entries; use OnlyTags on data lists to constrain allowed data types
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : InteractEntityObjective text may include <entity> placeholder; obtain positions by querying active AudienceEntityDisplay instances
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Use snake_case, descriptive IDs (e.g., armor_stand_definition, allay_instance) and practical descriptions
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Instance entries must implement SimpleEntityInstance (or an Advanced variant) with fields: id, name, definition: Ref<...Definition>, spawnLocation, data, and activity Ref of the correct scope
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Keep visual and structural parity with similar entities (icons, colors, descriptions, class structure) like AllayEntity.kt or ArmorStandEntity.kt
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Instance class <EntityName>Instance must be annotated with Entry and declare fields: id, name, definition: Ref<<EntityName>Definition>, spawnLocation, data (+OnlyTags as above), activity: Ref<out SharedEntityActivityEntry>
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Choose correct activity scope: SharedEntityActivityEntry for shared behavior/position, IndividualEntityActivityEntry for per-viewer behavior/position
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Definition class <EntityName>Definition must be annotated with Entry (and optional Tags) and declare fields: id, name, displayName: Var<String>, sound: Var<Sound>, data: List<Ref<EntityData<*>>>
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Access Var values via .get(player) where applicable in activities and entries
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
Repo: gabber235/Typewriter PR: 0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : EntityInteractEventEntry should handle left/right click with optional shift and interaction type filters to trigger dialogues/flows
Applied to files:
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt
🔇 Additional comments (2)
extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/audience/QuestItemAudienceEntry.kt (2)
32-37: Entry and display wiring look solid and match extension guidelines.The
@Entrymetadata, KDoc, and thedisplay()implementation are consistent with other audience entries, and the display’s lifecycle correctly registers/unregisters the listener ininitialize()/dispose(). No issues here.Also applies to: 49-57, 59-68, 202-206
122-130: Drop and death handling correctly preserve the quest item across death (including edge cases).The drop handler and death handler together:
- Block manual drops of the quest item.
- Strip quest items from
PlayerDeathEvent.dropsand re-add them after respawn using a delayed task.- Safely handle disconnects (
getPlayernull) and audience removal between death and respawn viacontains(player).- Handle full inventories on respawn by dropping leftovers naturally so the item is never silently lost.
Behavior here looks robust and aligned with the retention requirements.
Also applies to: 175-200
Refactor quest item handling to use persistent data for identification. Implement methods to tag items and check if they are quest items, enhancing inventory interaction events.
Refactor QuestItemAudienceEntry to improve item handling and interactions. Added cooldowns for interactions, improved inventory management, and ensured quest items are not lost on player death.
Removed unused properties and methods related to item consumption and interaction. Simplified the QuestItemAudienceEntry class by eliminating unnecessary code.
|
Hey, I noticed that you were using ItemMeta. For your information, this API has been deprecated in favour of DataComponents |
|
Hey @RenaudRl, I have some bad news. While refactoring the code to make it more readable I thought of several problems. Since the item is a Var it should be able to be changed (and we definitely want this as a feature). However this poses some issues. Mostly what happens if the player spreads out the item (i.e., having the item in multiple slots). What if the amount of the quest item changes? What if the player is holding the item when it needs to change, etc. To circumvent all of this I think it's better not to allow splitting up the item. However this requires some additional code in the event handlers (so right‑click and other actions don't place one item but always the entire stack). The item also shouldn't be draggable. This also requires extra testing to make sure things work correctly. Finally I think there is still a bug where you can use hotbar keys to move the item into another container. If you have the quest item in your hotbar, hover over a chest slot, then press 0‑9, it will move the item. We need to block this as well. Could you make these changes ❤️? Because they take up too much time to do myself, which I'd rather spend on v1.0 |
I agree. In fact, I was referring to the current support, which is part of version 1.21.3. It would be best to upgrade to at least version 1.21.4. |
Enhance quest item interaction handling in inventory events.
gabber235
force-pushed
the
develop
branch
2 times, most recently
from
59e27cf to
4d9cde8
Compare
3 participants
Introduces QuestItemAudienceEntry, an audience entry that forces players to keep a specific quest item in their inventory. Prevents dropping, moving to containers, and losing the item on death, ensuring players retain the item throughout the quest.
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.