Repository navigation
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: c2d0d2b3c8
ℹ️ 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".
| private boolean precisionSuppresses(Effect effect, String uuid, String world) { | ||
| if (uuid == null || !precisionSneaking.containsKey(UUID.fromString(uuid)) || effect == Effect.PrecisionMode) return false; |
There was a problem hiding this comment.
When the plugin is enabled or fully reloaded while a player is already sneaking, precisionSneaking starts empty and is populated only by a later toggle event or one of the three new automation listeners. Consequently, configured targets such as NoFallDamage, Fortune, or McmmoXP remain active until the player toggles sneak, even though PrecisionMode should suppress them immediately. Initialize this cache from online players during startup/reload, or consult the current player state when evaluating precision.
AGENTS.md reference: AGENTS.md:L29-L33
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07d44dc9df
ℹ️ 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".
| var definition = getPerkHandler().getPerk(perk); | ||
| if (definition == null || !definition.isEnabled()) { | ||
| getLogger().warning("Skipping shutdown record for unavailable perk: " + perk); | ||
| continue; |
There was a problem hiding this comment.
When a perk definition is temporarily disabled, missing, or fails to load during startup, this branch skips its shutdown record, but restoreOfflinePerks() subsequently clears the entire OfflinePerk section. Re-enabling or repairing that perk later can therefore no longer restore the remaining purchased time; retain unavailable entries and clear only records that were successfully restored.
AGENTS.md reference: AGENTS.md:L29-L30
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c58888c0d9
ℹ️ 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".
| return plugin.getMcperksServerData().confirmedActivation(() -> { | ||
| recordCooldown(user); | ||
| activatePerk(user, length); | ||
| }).thenCompose(ignored -> com.bencodez.mcperks.userapi.UserStorageConfirmation.confirm(plugin, user)); |
There was a problem hiding this comment.
confirmedActivation runs recordCooldown and activatePerk—including command and reward hooks—before persistSnapshot is even submitted. If snapshot serialization rejects an oversized ServerData file, an I/O error occurs, or the server crashes in this interval, the irreversible rewards have already been granted while the cooldown/expiration may remain unconfirmed, allowing the activation to be retried after restart. Stage and durably publish the required state before invoking activation gameplay.
AGENTS.md reference: AGENTS.md:L35-L35
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b769075a14
ℹ️ 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".
| getPerkHandler().activePerk(definition, | ||
| getMcperksUserManager().getMCPerksUser(UUID.fromString(uuid)), | ||
| (int) timeLast, timestamp); |
There was a problem hiding this comment.
For an unexpired finite perk saved during shutdown, this now invokes the full activePerk activation path, which calls EffectHandler.activate; Rewards, Commands, and GlobalCommands therefore run again on every plugin reload where affected players are online (and global commands run regardless). Restore the active timer/effects through a restoration-specific path that does not replay one-shot activation hooks.
AGENTS.md reference: AGENTS.md:L29-L29
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf1ebc8702
ℹ️ 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".
| if (user.getActivations(getPerk()) - plugin.getFeatureState().reserved(owner, "PerkActivations_" + getPerk()) > 0) { | ||
| globalActivation = false; user.addActivation(getPerk(), -1); | ||
| } else if (user.getActivations() - plugin.getFeatureState().reserved(owner, "Activations") > 0) { | ||
| user.addActivation(-1); |
There was a problem hiding this comment.
When PerkQue is disabled and a player with at least two activation credits submits the same perk twice before the first asynchronous activation finishes, both calls decrement a credit here, but the second call is subsequently rejected by activationAdmissions in executeConfirmed. That failed call never refunds or durably associates its debit with gameplay, so rapid retries silently discard a purchased activation; establish the per-player/perk admission before changing either activation balance.
AGENTS.md reference: AGENTS.md:L29-L29
Useful? React with 👍 / 👎.
| private void reloadAfterDrain() { | ||
| if (usageEffects != null) usageEffects.invalidate(); | ||
| if (queuePlanner != null) queuePlanner.invalidate(); |
There was a problem hiding this comment.
If a direct activation has published its shared snapshot but is still waiting for UserStorageConfirmation when a reload reaches this method, the reload barrier does not cover that later user-cache flush. This method invalidates queue and automation work but neither advances featureLifecycle nor replaces the PerkHandler, so verifyAdmission still succeeds and the pre-reload Perk can run its effects, rewards, or commands after definitions have been reloaded—even if that perk was disabled or changed.
AGENTS.md reference: AGENTS.md:L33-L33
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13e4d21e8f
ℹ️ 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".
| if (getInteractionOptions().banked() && plugin.getConfigFile().getLimitActivations()) { | ||
| plugin.getPerkHandler().addQue(user, this); return false; |
There was a problem hiding this comment.
When BankedTime.Enabled and LimitActivations are enabled but PerkQue is disabled, this unconditional branch still places every bank grant into QueuePlanner; its check() refuses to start while any perk is active. Consequently an unrelated long-lived or permanent perk can block the bank grant indefinitely, and different players' personal banks are serialized despite the queue being disabled. Route the activation through durable atomic debit without imposing queue scheduling when PerkQue is false.
AGENTS.md reference: AGENTS.md:L89-L89
Useful? React with 👍 / 👎.
| if (user.getActivations(getPerk()) - plugin.getFeatureState().reserved(owner, "PerkActivations_" + getPerk()) > 0) { | ||
| user.addActivation(getPerk(), -1); | ||
| } else if (user.getActivations() - plugin.getFeatureState().reserved(owner, "Activations") > 0) { | ||
| user.addActivation(-1); |
There was a problem hiding this comment.
With limited non-banked activations that start directly, the cached credit is decremented here before executeAdmitted performs its fallible ServerData publication and user-storage confirmation, and no failure path restores it. The newly added admission fence prevents the previously reported concurrent debit, but if snapshot capacity, publication, owner scheduling, or confirmation fails before gameplay, the player is left with a reduced activation balance and no perk; establish durable ownership before changing the credit, or restore it only for failures proven to precede gameplay.
AGENTS.md reference: AGENTS.md:L29-L29
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d097a02dd
ℹ️ 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".
| if (banked && !alreadyConsumed) return plugin.getBankedPerks().grantConfirmed(user, this, length) | ||
| .thenCompose(ignored -> confirmCooldown(user)); |
There was a problem hiding this comment.
For a banked perk with LimitActivations disabled, this publishes the additive bank grant first and only then calls confirmCooldown. If the server crashes or cooldown persistence fails between those operations, the durable bank time remains while the old cooldown remains eligible, so retrying the activation adds the same entitlement again for free. Persist the cooldown/admission before the bank addition, or retain a durable uncertainty fence that prevents retries.
AGENTS.md reference: AGENTS.md:L35-L35
Useful? React with 👍 / 👎.
| if (getInteractionOptions().banked() && plugin.getConfigFile().getLimitActivations() && plugin.getConfigFile().getPerkQue()) { | ||
| plugin.getPerkHandler().addQue(user, this); return false; |
There was a problem hiding this comment.
When BankedTime, LimitActivations, and PerkQue are enabled, this branch unconditionally creates a WAITING entry even when there is no active perk, paused queue, or predecessor. An otherwise direct purchase can therefore remain queued if the player disconnects before prepare runs, rather than using the durable STARTING direct-start path; only enqueue it when the ordinary queue conditions actually require waiting.
AGENTS.md reference: AGENTS.md:L91-L91
Useful? React with 👍 / 👎.
| var barrier = featureState == null ? java.util.concurrent.CompletableFuture.completedFuture(null) : featureState.runtimeStorage(() -> null); | ||
| barrier.whenComplete((ignored, error) -> { |
There was a problem hiding this comment.
Reload now returns as soon as this asynchronous barrier is registered, but the inspected CommandLoader still sends its “reloaded” success message immediately after calling plugin.reload(). Even a normal reload has not happened when that message is shown, and if this barrier fails the reload is explicitly postponed while the administrator has already been told it succeeded; expose completion or move the success response into the successful callback.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37c912baa6
ℹ️ 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".
| if (!runActivationHooks && (effect == Effect.Rewards || effect == Effect.Commands | ||
| || effect == Effect.GlobalCommands)) continue; |
There was a problem hiding this comment.
When an unexpired shutdown record contains CureHunger, CureSpells, or HealAll, restoreActivePerk passes false here, but this filter skips only rewards and commands, so effect.runEffect heals or cures players again on every restart/reload. Fresh evidence after the earlier restoration fix is that these remaining one-shot enum cases are still executed; they should be excluded from the restoration path alongside the other activation-only effects.
AGENTS.md reference: AGENTS.md:L29-L30
Useful? React with 👍 / 👎.
| ticking.remove(owner); sessions.remove(owner); for (BankKey key : List.copyOf(clocks.keySet())) if (key.owner().equals(owner)) suspend(key); | ||
| clocks.keySet().removeIf(key -> key.owner().equals(owner)); |
There was a problem hiding this comment.
When a disconnect, world transition, or failed pulse submits its final elapsed debit while the bounded feature-worker queue is full (or publication fails before changing the file), suspend leaves that elapsed interval reserved only inside the clock, but this line immediately deletes the clock without observing the checkpoint future. A later reconnect creates a fresh clock against the unchanged persisted balance, so the already-used interval becomes usable again; retain/retry the reservation or keep the bank unavailable until that final debit is resolved.
AGENTS.md reference: AGENTS.md:L47-L50
Useful? React with 👍 / 👎.
1 participant
Adds four independently configurable interaction effects, durable personal usage charges, queue management, banked personal time, diagnostics and local statistics. Existing timed perks retain their defaults when the optional settings are absent.
Behavior
AutoBlockRefill: refills a depleted placement hand from matching inventory items, preserving metadata and revalidating the hand/slot. Cancelled placements and no-op refills cost nothing.VeinMining: bounded connected ore traversal without loading chunks, native break/protection/tool/drop handling and recursion prevention. Additional successful blocks are the usage unit; the available budget bounds the batch before breaking.RightClickHarvest: main-hand mature crop harvesting, native cancellation/protection checks, one reserved inventory seed and replanting. Reuses AutoPlant requirements and avoids duplicate harvesting.PrecisionMode: owner-selected MCPerks effects are suppressed while sneaking; suppression spends no charges.Commands and opt-in configuration
Existing
PerkQueremains authoritative. OptionalLocalStatistics.Enabledis in Config.yml. README documents limits, units, permissions, seed policy and complete examples.Dependency and compatibility
Requires AdvancedCore PR #360, including the additive shutdown hooks, final-file completion contract and
ServerData.mutateFinalDataownership API. The dependency version is3.8.2-SNAPSHOT; its version string alone is not an immutable artifact identity. AdvancedCore is shaded into MCPerks, so server owners do not install a separate AdvancedCore plugin.Java 21 minimum is unchanged. MCPerks does not declare Folia support; conditional scheduler paths do not establish full platform support. Existing data and configuration remain readable, with no implicit conversion of legacy duration into banked time or automatic charge refill. Gameplay, storage and arbitrary external commands are not one universal atomic transaction; uncertain side effects remain conservative as documented.
Current native storage follow-up
Exact source commit:
15c87bfe58e97f4ea177be6470cd446412ca1490.The ordered MCPerks file owner remains available until both shutdown preparation and AdvancedCore's actual final-file publication settle. Final mutations record explicit write intent, preserve independently changed YAML siblings, normalize equivalent parsed numeric values, and publish through the existing ordered worker. Capacity rejection and asynchronous failure do not acknowledge unpublished state. Native reload/bulk mutation respects the inherited publication monitor; custom file ownership remains asynchronous.
Current candidate clean package passed all 348 tests, zero failures/errors/skips, against the exact locally built AdvancedCore candidate from PR #360, commit
fd53fd37a6e4de22a99a6fef0aa7b61d5fa58d1band JAR SHA-2568a357bf4c2265cbfb26eaf6734069151d25f43e16f1c222766c8ec7fd647c880.MCPerks JAR SHA-256:
10b2929720b6897110604faafd61f9cf7cbab9253d5600ba77611af9303622dd(9197654 bytes). ZIP integrity, relocated native ownership classes, Java 21 base classes and absence of bundled JUnit/Mockito checked. The unchanged MCPerks native-dependency source retains its scoped independent Sol 6.1 review. A fresh Sol 6.1 Extra High review of the producer legacy-owner compatibility delta also reported No findings, including the current MCPerks adapter dispatch. The exact consumer was rebuilt after the producer change; this is not a new whole-feature review.PR #76 remains draft. The published snapshot previously probed lacked the required APIs. Local exact-artifact success does not prove remote dependency availability. A compatible producer must be published and this consumer rebuilt against that exact remote artifact before its hosted compatibility and readiness gates can pass. Current-head hosted checks are pending.
Earlier native protocol tests belong to source
0273ec0e7710b2caa3cda462abf7cfc4cf1b39d1and JAR SHA-256611a6587d688ed6fa43201157715db40d696106bd86f71ec4706ea15054f4a0b, not this update. Those covered interactions, online/offline bank time, queue ownership, restart and start/cancel timing cases through automation; they were not visual gameplay verification. No current-JAR live Paper, Folia, database or graphical acceptance is claimed. MCPerks does not declare Folia support. No merge, release or deployment is performed.