Event-driven notifications and Essentials as an adapter module - #25
Merged
Conversation
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014kvZD6gWHjdMkN9Ebt3UbF
Depend on com.minecraftcitiesnetwork:plugin-infrastructure from realty-paper and replace the overlapping paper-side utilities with the shared versions. - Realty owns a ModuleLifecycleManager over plugins/Realty/modules: modules start last in onEnable and stop first in onDisable. New /realty module list and /realty module reload <module>, plus module refresh on /realty reload; all manager access is marshalled onto the main thread. - Dropped local DateFormatter, DurationParserUtil, ComponentSerializer and SimpleDateFormatSerializer in favour of the shared ones. - localisation.MessageContainer extends the shared one, adding only deserializeRaw for paginated commands, which substitute a command into a <click> tag argument that no TagResolver can fill. - The library is a realty-paper dependency only. Its module system imports the Paper API, so the backend layers must not see it: realty-backend and realty-backend-api keep their own CurrencyFormatter, MigrationStep and MariaSchemaMigrator. CurrencyFormatter's DecimalFormat is now ThreadLocal, since commands format concurrently on the async executors. - plugin-infrastructure is shaded but deliberately not relocated: module jars are compiled against those types and loaded into this plugin's class loader. Bump version to 1.4.0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014kvZD6gWHjdMkN9Ebt3UbF
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014kvZD6gWHjdMkN9Ebt3UbF
…n events Inventory showed the listener already renders notifications from domain events - the model this plan completes. Only its delivery path needs changing, which is a smaller change than relocating its nine handlers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014kvZD6gWHjdMkN9Ebt3UbF
Replace all 14 notificationService.queueNotification(...) calls in the agent, auction and offer command classes with events.fireSync(new RealtyNotificationEvent(List.of(target), message, region)), firing the event alongside the existing domain events. Collapse the OfferCommandGroup reject-all loop into a single multi-target event, guarded against an empty offerer list. Drop the now-unused NotificationService constructor components/imports from these command classes and their construction sites in Realty.java. Also add a fireSync(RealtyNotificationEvent) overload to RealtyEventDispatch, since RealtyNotificationEvent extends Event directly rather than RealtyRegionEvent (it allows a null region) — fireOrHop/cancelled are widened from RealtyRegionEvent to Event to support both overloads without duplicating the threading logic.
Removes a published realty-paper-api type. RegionNotificationListener is kept and now fires notification events instead of delivering directly.
Reuse resolveRegion for the auction-won / auction-ended-no-bids notifications so they fire unconditionally with a possibly-null region, matching the payment sweeps. Only the AuctionEndedEvent fire (whose region is @NotNull) stays gated on a resolved region.
…LocationFinder predicate Adds Realty#executorState() and Realty#paperApi() accessors, makes SafeLocationFinder's safety predicate volatile and swappable via setSafetyPredicate()/safetyPredicate(), and adds RealtyPaperApi#setSafeBlockPredicate(Predicate<Block>) delegating to the single SafeLocationFinder instance constructed in onEnable before RealtyPaperApiImpl and passed to registerCommands. This gives the upcoming chat and Essentials adapter modules the seams they need without delivering any behavior itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014kvZD6gWHjdMkN9Ebt3UbF
…te out of core Port essentials-adapter from pre-reconcile-event-driven-notifications, adapted for the synchronous RealtyNotificationEvent (getTargets()/getMessage(), no Executor marshalling). Mail goes only to offline targets so the chat adapter and this listener don't double up; a per-target send failure is logged and does not stop the remaining targets. Core no longer depends on EssentialsX: EssentialsSafeBlockPredicate moved into the module, the compileOnly dependency is dropped from realty-paper, and Realty.onEnable now constructs SafeLocationFinder unconditionally, letting the module supply the predicate when it loads. paper-plugin.yml's Essentials softdepend is untouched — join-classpath is still what lets the module resolve EssentialsX types at runtime. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014kvZD6gWHjdMkN9Ebt3UbF
- Warn when no delivery module is loaded at all, and separately when chat-adapter specifically is missing, so silent total notification outage is no longer unlogged (Realty.java). - Correct the chat/essentials adapter Javadocs: the previously documented online-ness race does not exist under a synchronous RealtyNotificationEvent dispatch; state the real invariant (exactly-once delivery per target) and its dependency on the event staying synchronous. - Marshal ModuleCommandGroup's reload-argument suggestion provider onto the main thread, matching the other handlers, since Cloud resolves suggestions asynchronously and ModuleLifecycleManager is not thread-safe. - Skip non-reloadable modules in reloadModules() so /realty reload stops logging an expected-by-design failure for essentials-adapter every time. - Reorder EssentialsAdapterModule.initialize so all fallible work happens before the listener is registered. - Document that a null RealtyNotificationEvent.getRegion() is routine (payment-expiry sweeps after region deletion), not pathological. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014kvZD6gWHjdMkN9Ebt3UbF
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
1 participant
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.
Makes every Realty notification a fired event, deletes
NotificationService, and moves delivery into adapter module jars — built on top of the event system added in #22 rather than alongside it.Why
Notifications weren't event-driven. Commands called
notificationService.queueNotification(...)directly, only nine events routed throughRegionNotificationListener, the two payment-expiry sweeps fired nothing at all, andNotificationServicepicked its delivery backend at enable time with EssentialsX compiled into core.What changed
RealtyNotificationEvent(realty-paper-api) — a standalone synchronous event carryinggetTargets(), a pre-renderedgetMessage(), and a@Nullable getRegion(). Fired viaRealtyEventDispatch.fireSync(...).RegionNotificationListener, 4 in the sweeps. Text is rendered at the fire site exactly as before, so no message wording changes.NotificationService,TransientNotificationServiceandEssentialsNotificationServiceare deleted. Core delivers nothing.realty-paper-adapters/, loaded byModuleLifecycleManager:chat-adapter(online players → chat) andessentials-adapter(offline players → Essentials mail, plus the EssentialsX teleport-safety predicate).chat-adapteris bundled in the plugin jar and extracted on enable only if absent, so a stock install keeps today's behaviour and an operator's deliberate removal survives restarts.realty-paperno longer compiles against EssentialsX.plugin-infrastructuremodule system is included here — it existed only on an unpushed local branch.What this deliberately does not touch
The 47 event classes from #22 are unmodified. The only file from that PR this one changes is
RealtyEventDispatch, additively: afireSync(RealtyNotificationEvent)overload plus widening two private helpers toEvent.RegionNotificationListeneris kept, not deleted — it already rendered notifications from domain events, which is the model this PR completes; only its delivery path changed.An earlier draft reparented ~20 post-events onto a shared base so adapters could use one handler. That was rejected: it would have required deleting both the
HANDLERSfield andgetHandlerList()from each, where missing one silently keeps that event on its own handler list and it never reaches the adapters — no error, no warning.Reviewer notes
paper-plugin.ymlkeeps itsEssentialssoftdepend withjoin-classpath: trueeven though core no longer compiles against EssentialsX. Module jars load through aURLClassLoaderparented to Realty's plugin class loader, so that entry is the only reason EssX types resolve inside the adapter. Removing it compiles cleanly and fails at module load withNoClassDefFoundError.module-manifest.yml—ModuleLoader.extractManifestreads exactly that name.chat-adapteris missing, and when Essentials is installed withoutessentials-adapter.Breaking
NotificationServiceis removed from the publishedrealty-paper-api.Testing
242 tests,
shadowJargreen. One failure —RealtyBackendImplTest > termination > "scheduling a termination blocks extension and is honoured by the sweep"— is pre-existing onmain, verified in an isolated worktree with none of these changes.Two pre-existing issues found along the way (not addressed here)
refundAmount(). It appears only in two message-formatting calls and four test assertions — expired bid/offer payments tell the player they were refunded, but no economy call is made. The deletedqueueNotificationpath didn't deposit either.🤖 Generated with Claude Code
https://claude.ai/code/session_014kvZD6gWHjdMkN9Ebt3UbF