Add packet-based GUI backend - #1
Open
Keviro wants to merge 57 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces an internal “packet GUI” backend for the Bukkit platform, intended to render chest-style GUIs using PacketEvents (with a fallback to standard Bukkit inventories when unavailable or unsupported).
Changes:
- Adds a
GuiBackendabstraction with a default Bukkit implementation and an optional PacketEvents-driven packet backend. - Introduces packet GUI session/render/click tracking to open/update/close GUIs via packets.
- Adds PacketEvents as an optional (soft) dependency and updates build configuration to include the required repository and version catalog entry.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
- add handling for closing inventory sessions based on user actions - improve session management by ensuring proper windowId checks - update session inventory state after closing to reflect changes
- implement normalization for item display names and lore - utilize reflection to access ItemMeta properties - ensure non-italic display names for better consistency
Adds coalesced SetSlot-based updates and per-session item conversion caching so normal GUI interactions no longer rebuild and resend the full window. Keeps full WindowItems only for open/reopen/hard repair paths, preserves Folia-safe scheduling, and maintains PacketEvents as the packet API. This massively reduces repeated ItemStack conversion and fullResync overhead during clicks, pagination, and dynamic shop GUI updates.
Handle QUICK_MOVE, SWAP, and CLONE click actions in the packet GUI backend while keeping unsafe drag/drop/double-click actions cancelled and repaired. Shift and keyboard-style clicks now enter the normal click pipeline and trigger a hard repair to avoid client-side ghost item movement.
Handle server-side close packets without sending an extra close packet or forcing updateInventory, preventing packet GUI cleanup from immediately closing dialogs opened from click handlers.
Force a post-click repair for normal left/right PICKUP clicks so client-side prediction cannot leave fake GUI items missing or stuck on the cursor. Keeps shift, swap, clone, and unsafe click repair behavior intact.
Ensure offhand swap clicks repair the client-side offhand slot and cursor so fake GUI items cannot appear as ghost items in the player's hand.
Author
|
Die neuste Version ist auf dem Dev Survival Server zum Testen |
Replace normal-click full window repairs with targeted slot/cursor repair scopes, reduce full WindowItems usage, and simplify packet item caching to avoid expensive render hashing.
Avoid expensive full window item resyncs during normal GUI clicks, remove debug toString calls from hotpaths, and batch packet writes through a guarded send plan.
Build packet GUI render/repair plans on the player scheduler, but move the expensive Bukkit-to-PacketEvents item conversion onto dedicated worker threads. This keeps GUI state access Folia-safe while reducing region thread load from PacketItemConverter/SpigotConversionUtil during packet GUI renders and click repairs. Async conversion is enabled by default and logs a warning when disabled.
Use native outbound inventory packets for packet GUI rendering so surf-api PacketLore can process clientbound GUI items, while falling back to the Bukkit backend when the current Minecraft version is not explicitly supported.
`cmd /c gradlew.bat` resolves against PATH rather than the task's workingDir, so the delegating shadowJar and publish tasks failed with "gradlew.bat is either misspelled or could not be found". They now invoke the wrapper by absolute path. Adds publishToMavenLocal to the same set, so a branch build can be installed into ~/.m2 and consumed by surf-api, which shades this fork instead of depending on it at runtime. docs/testing-with-surf-api.md describes that round trip and why a composite build is the wrong tool for it.
Picks up the only substantive change from ced7b1c on origin/packet-guis, which is otherwise a whole-file reindentation of SlotClickContext.java from spaces to tabs. That reindentation is not merged: it contradicts the palantir format the project applies everywhere else and turns an eighty-line change into a four-hundred-line diff, which is exactly what makes the click-origin refactor hard to review.
…flection The backend reached the entity scheduler through its own reflection layer, so the module carried two unrelated Folia strategies that had to be kept in sync by hand: this one and the FoliaLib instance BukkitElementFactory already uses. FoliaLib covers every case the reflection did, and exposes the failure signal natively. isOwnedByCurrentRegion now goes through PlatformScheduler, which resolves to a real region check on Folia and to Server#isPrimaryThread on Paper and Spigot, so the "method absent" and "invocation failed" branches disappear along with the cached Method handles. The inline fast path stays: FoliaLib always defers, while several call sites depend on the work having happened by the time they return - open() publishes the session before rendering it, and closeSession runs the close pipeline synchronously. Scheduling failures keep their meaning. runAtEntityWithFallback and the retired callback of runAtEntityLater both discard the session, and a null WrappedTask from the next-tick path does the same, so a dropped render task cannot leave the render request latched.
… opened Opening a GUI while another one was already open sometimes produced no window at all - the command had to be issued several times. The window id was allocated each time, so open() ran; the session was torn down again right after. handleInventoryClosePacket ignored the window id entirely, so any outbound ClientboundContainerClose for the viewer finalized whatever session was registered. Publishing the new session before closing the previous one, which the re-entrancy fix introduced, put that new session in the map exactly while the previous view's CLOSE pipeline was running - and that pipeline is arbitrary developer and framework code which routinely ends up calling Player#closeInventory. Its close packet then finalized the session that was still being opened. open() now marks the viewer while it runs, and close packets seen during that window are ignored: they are caused by our own teardown, not by the viewer. Independently, the handler only reacts to a close for its own window or for the player container - the latter being what Player#closeInventory emits, since the server never knows about the fake window. A close for any unrelated window no longer touches the session.
PacketViewerInventory kept a 46-slot copy of every viewer's inventory plus their cursor, fed from intercepted outbound packets. Nothing ever read it: the bottom rows of a GUI are rendered from live Bukkit reads. It was kept because of a rule that live reads would break surf-api's PacketLore. A test on the server disproves that: the bottom rows go through the native outbound sender, i.e. the same path as vanilla, and an enchanted item in the viewer's inventory shows its enchantment lore inside a packet GUI, decorated exactly once. Making the mirror authoritative would have been the wrong direction anyway - its contents come from packets PacketLore may already have decorated, so resending them risks decorating twice. What is left is PacketViewerWindowTracker, which only remembers the id of the real container window so PacketGuiWindowIds never hands out a colliding fake id. The per-slot repaints stay; they are live behaviour and read from Bukkit, not from a mirror. The listener now skips decoding outbound inventory packets for viewers without a session, so players who have no GUI open stop paying for the interception at all. With the mirror gone, PacketItemConverter no longer converts to PacketEvents items either, which removes the last use of SpigotConversionUtil.
Rendering built a plan of conversion operations, then walked those operations to build a second plan of outbound packets, then sent that. Both phases ran on the viewer thread in the same tick, so the split bought nothing - it was left over from an earlier attempt to convert items off-thread. It cost something, though. The two plans were the same (channel, windowId, generation, List) value object written twice, and canConvertPlan and canSendPlan were byte-identical five-clause guards forty lines apart, so a fix applied to one would silently miss the other. The phases had already drifted: addWindowItems keyed its packet on the plan window id while addPlayerSlotRepairs used the session one. There is now a single PacketGuiSendPlan that collects outbound packets as the render is built, and a single canSend guard, re-checked before each packet exactly as before. Item copies happen while building rather than during a separate conversion walk, which is the same thread and the same tick.
Fills in the twelve checklist rows that were actually exercised on SurfCanvas (Minecraft 26.2) while bringing this branch up, and names the source for each: some were watched on screen, some read off the debug-clicks log. The remaining fourteen rows are left empty on purpose rather than assumed. Two of them matter before this is called done - nobody has yet confirmed that no GUI display item can end up in a real inventory, which is the claim the whole backend exists to make, and the window id collision guard has only been unit-tested, never run end-to-end against a real chest. Also reorders the activation section: the sentence explaining what native=off does had ended up behind the debug-clicks block and read as if it described that instead.
…cursor fix Closing a GUI sends a cursor correction and then a close packet. The correction went through the native connection, which queues, while the close was written straight to the channel by PacketEvents. With a backed-up queue the close can overtake the correction, and the client then applies it to its own inventory menu - reinstating exactly the ghost item the correction exists to clear. ClientboundContainerClosePacket takes nothing but the window id, so the native sender can build it without any chat-component conversion. It is resolved optionally and covered by the startup self-check; if a server does not expose it, the PacketEvents path is still used. The open-screen packet deliberately stays on PacketEvents. It carries a title component, so sending it natively would mean converting Adventure components to NMS ones, and it has no ordering hazard to fix: the content packets that follow it are queued behind the connection and cannot overtake a direct channel write.
The version was still 1.0.4, the same value master carries, so merging would have republished an existing release coordinate with entirely different contents - either failing the release job or silently handing every consumer a different 1.0.4. Nothing verified pull requests either: publish.yml only triggers on pushes to master, so the packet GUI backend and its tests had never been built by anything but a workstation. The new workflow applies the patches, runs the target repository's tests through a delegating task, and fails if makePatches would produce a diff - a patch that no longer round-trips is the one mistake this fork layout makes easy.
The allocator cycled 1..127 and only refused the one real window id the viewer had open at the moment of allocation. Vanilla allocates real container ids from 1..100, so the ranges overlapped almost completely and the guard covered a single instant. The unprotected case was a real container opening while a packet GUI was already on screen. If the server's counter landed on the GUI's id, isGuiWindow() claimed the packet as the backend's own, handleExternalInventoryOpen never ran, the session stayed open on a window the real container now owned, and every click in that container was cancelled and routed into the stale GUI pipeline - a container the player could neither use nor escape. Fake ids now come from 101..127, which vanilla never hands out and which still fits in a signed byte. The forbidden-id skip is kept as defence against another plugin's fake window in the same range.
…doned sessions openingViewers is a set, so a nested open() - which the close pipeline can trigger, since opening a view from onClose is a normal pattern - removed the marker in its own finally block while the outer invocation was still running. The remainder of the outer close pipeline then executed unguarded, and any Player#closeInventory it performed killed the session that had just been opened. That is the same failure 39a19c8 fixed, one level of nesting down. Only the outermost invocation lifts the marker now, and it is taken before closeRealInventory so it covers the whole open sequence. abandonSession dropped a live view at FINE level, which no production server logs, and never released the viewer, so the view kept listing someone who had no window and would never get one back. It now warns and detaches the viewer from the root. The CLOSE pipeline stays unrun on purpose: it executes developer code, and this path exists precisely because no thread will accept work for that viewer any more.
PacketSlotClickEvent exists so that consumers written against the Bukkit backend keep working unchanged, and it got the two most-read accessors wrong. getSlot() returned the view-wide raw slot. Bukkit reports the index inside the clicked inventory, so a bottom click on a 54-slot GUI produced 88 where Bukkit produces 7, and player.getInventory().getItem(event.getSlot()) - the most common thing such a consumer does - read the wrong slot or threw. getClickedInventory() returned null for every click, and null is Bukkit's canonical "clicked outside the window" signal. The usual `if (getClickedInventory() == null) return;` guard therefore discarded every packet GUI click, and comparing it against the player inventory threw. Bottom clicks now report the viewer's inventory; top clicks keep returning null, because there genuinely is no Bukkit inventory behind them. An outside click also reported InventoryAction.PICKUP_ALL while its slot type said OUTSIDE. A packet GUI never fills the real cursor, so NOTHING is both truthful and what Bukkit reports for the same click.
…e window tracker honest The mirror pass sent one slot packet per mirrored slot - 36 of them for every player#updateInventory that happened while a GUI was open, which the backend itself performs on close and other plugins call routinely. It now sends the single container content packet that path already had available. PacketViewerWindowTracker only ever learned about a real window closing when the server initiated it. Vanilla does not echo a close packet for a window the client closed, so pressing escape on a chest left the tracker naming a window that no longer existed. The inbound close packet is already intercepted, so the non-GUI branch now clears the tracker there. Also drops a line in handleWindowClose that could never fire - it asked the tracker to close the fake GUI id, which the tracker never holds - and removes the session monitor from the accessors of immutable fields, which netty threads read on every inbound packet.
The latch and the repair merging were the most state-machine-shaped code in the backend and the only part of it with no tests: a lost update there shows up as a window that silently stops matching what the view thinks it shows. They could not be tested where they were, because PacketGuiSession needs a viewer, a PacketEvents user and a final PacketViewContainer that Mockito 4 cannot mock. PacketGuiRenderRequests now owns that logic and needs nothing but the top size. The session delegates to it. Ten tests pin the behaviour the rest of the backend relies on: the latch only schedules once and reopens after a consume, clear() releases it without discarding the pending frame, flags only ever merge upwards, a full resync drops targeted repairs, and slots outside the window are ignored.
…ange The bottom-click write-back was described as opt-in, requiring setCancelled(false). The click context starts uncancelled, exactly like the Bukkit backend, so it is opt-out - a handler that changes currentItem for display purposes only has to cancel, and the old wording would have led someone to destroy an item. Also records what the synthesized InventoryClickEvent now reports for getSlot() and getClickedInventory(), why fake window ids live in 101-127, and that a discarded session is a WARNING that means onClose did not run.
Sending one container content packet instead of 36 slot packets was not an equivalent optimization. The protocol has no "set these slots" form: a content packet addresses the whole window, so the client redraws the top rows as well. A refresh that was meant to repaint only the viewer's own rows came out as a full window redraw. Reverts only the mirror change from 846d399; the window tracker fix, the dead line removal and the accessor cleanup in that commit stay.
InventoryFramework builds a new context and a new container on every open, including when a view reopens itself to refresh. The backend answered each of those with a freshly allocated window id and an open-screen packet, so the client tore the screen down and built a new one: cursor back to the centre, the whole window flashing, for what the view intended as an in-place update. In surf-shop the refresh button made that obvious - every click produced a new window id. A new session now takes over the window of the one it replaces when the row count matches, adopting its applied render and container state id. renderSession then diffs against what the client is actually showing and sends only the changed slots; a different title or row count still fails sameWindow() and produces a real reopen. Outbound close packets are dropped while a viewer is inside open(). They were already ignored, but ignoring is not enough once no open-screen packet follows: the client does not check which window a close belongs to, it closes whatever is on screen.
…ming The guard was set and cleared on the viewer's thread but is read on netty threads. A close packet emitted during open() - by closeRealInventory, or by the replaced view's close handlers - is written from the viewer's thread and observed on the connection's event loop, because the server defers sends made off that loop. Clearing the marker in a finally block let that observation land after the marker was already gone, and the close was then read as the viewer closing the window that had just been opened. That race was survivable while every open ended in an open-screen packet, which put the screen back. Window reuse removed that safety net, so a miss now loses the GUI outright. The marker is released through the same event loop instead. The loop is FIFO per channel, so everything the open sequence queued is observed first - the ordering holds by construction rather than by luck. Disconnect, quit and unregister clear the marker too, because a channel that dies mid-open would otherwise leave it behind, and a stuck marker silently drops close packets. abandonSession also removed whatever session the viewer happened to be on rather than the one whose task was refused. Scheduler fallbacks fire asynchronously and can arrive long after the task was queued, so this could destroy a live window and detach the viewer from a view it had just opened. It now takes the session and removes it conditionally, the way closeSession already did. Call sites without a session log instead of guessing at one.
…es not own toNmsItem normalized every item it converted, and it converts everything the backend sends - GUI icons, but also the viewer's own items in the bottom rows, on the cursor and in player-inventory slot updates. Styling items the backend does not own is not the transport layer's job, so it now converts verbatim and the normalization happens where GUI icons are produced, in PacketGuiRender#bukkitItem. No behaviour change was observed from this. An anvil-renamed item renders italic inside a GUI both before and after, so the case this was expected to fix - a name that leaves its italic state unspecified, which vanilla renders in italics - does not appear to reach the normalization on a live server. Why is not established; the reflection resolves (adventure.text is not relocated by the shaded build), so the likeliest explanation is that Paper does not hand out such a name with the state unspecified in the first place. Recorded as a hypothesis, not a finding. What is left is the structural argument plus one measurable saving: the clone/getItemMeta/setItemMeta round trip no longer runs for the 36 mirrored slots on every player#updateInventory while a GUI is open. Six tests pin the distinction the normalization rests on: an unspecified italic state is pinned to false, an explicit one - either way - is returned untouched.
…send Two gaps in the same promise: a mismatch is supposed to warn once and leave every GUI on real Bukkit inventories. register() and the native sender's initialize() caught only RuntimeException, but both resolve NMS classes and run their static initializers. A class that is present yet cannot be linked raises NoClassDefFoundError, NoSuchMethodError or ExceptionInInitializerError - all Errors, all escaping the catch, aborting the owning plugin's enable rather than falling back. The presence probe in GuiBackendFactory uses Class.forName without initializing, so this is reachable whenever PacketEvents is on the classpath but broken. Both now also catch LinkageError, which is exactly that family; Throwable is deliberately not caught so OutOfMemoryError still propagates. The send path was never probed at all, because reaching a player's connection needs a live player - so a server whose connection field or send method does not match reported "Packet mode enabled" and then failed every single GUI open, forever, with the Bukkit fallback already discarded. selfCheck now resolves as much of that chain as it can statically and reports what it cannot, without refusing the server over it: an obfuscated runtime may name the field differently and still work through the field-scanning fallback. If the chain does turn out to be unusable, the first failure now disables packet mode and releases every open session back to real inventories, instead of retrying per GUI. A dedicated exception type keeps that apart from a single packet failing to build, which still only takes its own session down.
…ong thread Three findings on the same theme - work happening where it should not. detachAbandonedViewer mutated the context's viewer list, the framework's global viewer map and the root's context list inline. Those are plain collections the main thread iterates, and the method is reached from a scheduler fallback that can fire on a netty thread. It now hops to the global tick thread, which is where framework state belongs - not the viewer's own scheduler, since that one refusing work is what led here. open() returned without doing anything when PacketEvents had no user for the player. By then IF has already registered the viewer with the framework and the context and committed the container, so the viewer stayed bound to a window that did not exist and that nothing could finalize: no close packet, quit handler or external open reaches a session that was never created. The likeliest cause is a connection PacketEvents has not registered yet, so it retries once on the next tick and releases the view properly if that still fails. The inbound close packet for a GUI window was force-uncancelled, which overrode whatever another listener had decided and let vanilla run its close handling for a container the server does not have open - firing an InventoryCloseEvent for the player's own inventory on every packet GUI close. It is now cancelled, like the click packet next to it.
…acket GUI The observable consequence of cancelling the inbound close packet: a plugin listening for InventoryCloseEvent will not hear about a packet GUI closing. A view's own onClose is unaffected. Also notes what the event used to describe, since it was not the GUI - vanilla ran its close handling against the player's own inventory menu, so anything that relied on it was reacting to a misleading event.
The wrapper was committed as 100644, so every Unix invocation of ./gradlew in this repository fails with "Permission denied" (exit 126). The new verify workflow was the first thing to actually run it on Linux and it failed on its first step; the generated repository's own wrapper is 100755, which is why the delegating tasks were unaffected.
…er-wide Method#invoke reports a throwing target method as InvocationTargetException, which is a ReflectiveOperationException - so send() laundered every failure raised inside the server's own send(Packet) into SendPathUnusableException, the one signal that turns packet mode off for the rest of the server's life. One rejected packet for one viewer killed the feature for everyone. That exception is now reserved for the resolution failures it was meant for, and a rejected packet closes only its own session, as intended. When packet mode is disabled the native sender is dropped first, so the teardown takes closeSession's PacketEvents branch instead of reaching for the path that just proved unusable, and the close packet is actually sent - otherwise every viewer kept a window on screen that the server had already forgotten. Two netty-side paths tore a session down without marking it close-requested first. Because the teardown hops to the viewer's thread, the session stayed eligible for window reuse for up to a tick, so a reopen in that window adopted the id and applied render of a screen the client had already destroyed and sent no open packet - the viewer was left bound to an invisible session. Outside clicks reported isLeftClick()/isRightClick() as false while clickIdentifier() called the same click LEFT/RIGHT. THROW is the form a packet GUI actually receives, so the accessors disagreed with the synthesized ClickType on every outside click a consumer sees. The CI patch-drift check could never fail: makePatches stages the patches it regenerates, so `git diff -- patches` compared them against themselves. Verified by regenerating a genuinely drifted patch - `git status` reports it as staged, the old check passes, `git diff HEAD` catches it.
… artifacts in CI Mode CLONE was treated as a middle click for every button and in every game mode. Vanilla only accepts button 2 (`case CLONE: if (packet.buttonNum() == 2)`) and reports ClickType.UNKNOWN otherwise, and it only produces CLONE_STACK for a creative viewer - in survival the same click is NOTHING. Other buttons now take the existing fail-closed path instead of reaching a view as a middle click the Bukkit backend would never have produced. CI ran only the tests, so nothing on the release path was built. A javadoc or shadow-jar break in the 4800 new lines would have passed the PR check and failed the publish job on master instead. Also removes a stale javadoc block left above forceNonItalic, whose text described a recursion concern the method does not have.
…ntract in the docs PacketGuiRepairScope.PLAYER_INVENTORY was never asserted as an output anywhere, so the bottom-click routing decision - this backend's headline click behaviour - could be deleted outright with the whole suite still green. Verified by mutation: stubbing isSafePlayerInventoryClick to return false passed before these tests and fails after them. Slots past the bottom rows are pinned too. PacketGuiClick.of has carried a varargs changedSlots parameter for tests since it was written and no test ever passed one, so scheduleChangedPlayerSlotRepairs had never run with data - the repair scope produced by shift-click and number-key swap, the two most common item-moving clicks, was scheduled by nothing. The documented bottom-click contract was wrong in the direction that costs items. It named the handler as the only thing that suppresses the write-back, but GlobalClickInterceptor cancels every click on a view built with cancelOnClick(), which most GUI views are - so there the write-back is opt-in, not opt-out, and only View#onClick can restore it because component handlers run before the config is applied. Also corrects three narrower claims: debug-clicks cannot log the case it was advertised for (a click that matches no session produces no line at all), the close/cursor ordering guarantee holds only when the optional close constructor resolved, and the probe line is not "always logged".
…he item copy The refusal safety net did not work on the platform it exists for. FoliaLib's Folia implementation reports a retired entity scheduler two ways, and neither was handled: runAtEntityWithFallback completes its future with ENTITY_RETIRED or SCHEDULER_RETIRED without invoking the fallback runnable, and that future was discarded; runAtEntityLater wraps whatever the scheduler returned, so a retired one makes it throw while wrapping null instead of returning null - the NPE escaped onto a netty thread and left the session holding a latched render request that swallowed every later one. Both channels are now read, and the handler is wrapped so it runs at most once however the refusal arrives, rather than making abandonSession idempotent for a second reason. Also records what cannot be papered over in the click compatibility shim: getCurrentItem() returns a copy, while the Bukkit backend returns a CraftItemStack that wraps the clicked slot by reference. An in-place edit that works on the classic backend is silently discarded here, so setCurrentItem(...) is mandatory. Noted at the three places a consumer would look - the docs, getClickOrigin's javadoc and the synthesized event - since the previous wording implied the opposite.
…e connection field InventoryInteractEvent defines cancellation and result in terms of each other, and its own javadoc calls setResult(...) the preferred way to stop a click. The synthetic packet click event routed only setCancelled to the context, so a handler that used the documented API was silently ignored and the click was applied anyway. All four accessors are overridden together now, with the context as the single source of truth and the inherited field kept for the ALLOW/DEFAULT distinction. PacketViewContainer#getTitle() read the title the container was constructed with, so every updateTitleForEveryone(...) stayed invisible through PlatformContext#getUpdatedTitle(), which is that call. It now resolves like the per-viewer accessor already did, matching what the Bukkit container reports from the live InventoryView. The native sender trusted any field literally named "connection" and cached it before dereferencing it. On a fork whose player handle carries an unrelated field of that name, the send that followed would have looked like a broken send path and taken packet mode down for the whole server. Resolution moves to PacketGuiConnectionFields, which validates a field before trusting or caching it and keeps the hierarchy scan as a one-off fallback. Tests for the bukkit module compile against the Paper API. On a Spigot classpath the Paper-only methods the item converter reaches reflectively do not exist, its Method handles resolve to null and the whole normalization degrades to a no-op that no assertion could tell apart from working.
applyPatches writes inventory-framework/ from upstream plus patches/. It was neither tracked nor ignored, and because it carries its own .git a `git add -A` in the root adds it as a gitlink rather than skipping it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This pull request adds an optional packet-based GUI backend for InventoryFramework.
The goal is to render GUI contents as virtual packet items instead of placing real items into Bukkit inventories. This makes GUI item rendering safer against duplication issues while keeping the existing InventoryFramework API unchanged for consumers.
PacketEvents is used as the packet abstraction layer, with the Bukkit inventory backend remaining available as a fallback. The packet backend is designed to work on the current Paper/SurfCanvas/Folia runtime and keeps player-facing inventory state synchronized through packet updates.
Motivation
Inventory GUIs should not need to store their visual items as real server-side inventory contents. Rendering them through packets allows the framework to display the same GUI items to players without making those items physically exist in the server inventory state.
This improves safety for GUI-heavy plugins and provides a cleaner backend model for future inventory rendering behavior.
Notes