Broadcast proxy votes immediately in FORWARD mode - #1541
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51896bc158
ℹ️ 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".
| boolean immediateProxyBroadcast = proxyBroadcastDecider.usesImmediateForwarding(); | ||
| if (immediateProxyBroadcast) { | ||
| Set<String> targets = proxyBroadcastDecider.resolveTargets(playerOnline, playerServer); | ||
| sendProxyBroadcast(targets, uuid, player, service, time, text == null ? "" : text.toString()); |
There was a problem hiding this comment.
When OfflineMode is FORWARD but the voter is online and the scope targets multiple servers, this sends a standalone broadcast to every target and later suppresses broadcasting from the normal vote envelopes. Unlike the normal Bungee vote path, which preserves the proxy-wide event.isWasOnline() value, the standalone handler in BungeeHandler uses user.isOnline() on each receiving backend; therefore EVERY_VOTE_ONLINE_ONLY drops the announcement on every target except the backend currently hosting the voter. Immediate standalone forwarding should apply only when playerOnline is false.
Useful? React with 👍 / 👎.
| if (proxyBroadcastDecider.usesImmediateForwarding()) { | ||
| broadcastHere = false; |
There was a problem hiding this comment.
When upgrading or reloading with FORWARD already configured—or when changing from QUEUE to FORWARD while votes remain cached—these cached entries predate the new standalone broadcast and were never announced. Unconditionally setting broadcastHere to false when the player logs in silently discards their pending announcements before removing the cached votes; suppression needs to distinguish votes that were actually broadcast immediately.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25a108d62b
ℹ️ 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".
| getVoteCacheHandler().addServerVote(s, | ||
| new OfflineBungeeVote(voteId, player, uuid, service, time, realVote, text.toString())); | ||
| new OfflineBungeeVote(voteId, player, uuid, service, time, realVote, | ||
| text.toString(), immediateProxyBroadcast)); |
There was a problem hiding this comment.
When PLUGINMESSAGING is used and a targeted backend currently has no connected players, sendPluginMessageServer skips sending the standalone message, but this cached vote is still persisted with broadcastForwarded=true. When someone later joins and checkCachedVotes releases the vote, that flag suppresses the normal envelope's broadcast, so the announcement is permanently lost. Only record the broadcast as forwarded for targets where delivery was possible, or leave it pending in this case.
Useful? React with 👍 / 👎.
| globalMessageProxyHandler.sendMessage(targetServer, delay, | ||
| VotingPluginWire.voteBroadcast(uuid, player, service, time, text)); |
There was a problem hiding this comment.
The new offline-only gate does not put the sampled playerOnline=false state into this standalone envelope, so BungeeHandler instead evaluates user.isOnline() when the delayed message arrives. If the voter reconnects during the scheduler or broker delay, a backend configured with EVERY_VOTE_ONLINE_ONLY broadcasts a vote that was received while the player was offline. This delivery-time state check is fresh evidence that the earlier online-gating fix is incomplete; the envelope needs to carry the original online state.
Useful? React with 👍 / 👎.
| private void addBroadcastForwardedColumnIfMissingOnce() { | ||
| final String key = getDbType() + ":" + getTableName() + ":broadcastForwarded"; | ||
| if (!MIGRATED_BROADCAST_FORWARDED.add(key)) { | ||
| return; |
There was a problem hiding this comment.
The static guard is keyed only by database type and table name, although the supported reloadall flow tears down MySQL and reconnects using the newly loaded configuration. If an administrator switches the vote-cache connection to another existing database with the same prefix during that reload, this key is already present, so the new database never receives broadcastForwarded; subsequent inserts and reads reference the absent column and cached votes can be lost. The equivalent guard in ProxyOnlineVoteCacheTable has the same defect, so both keys need to distinguish the actual database/schema (and should only be retained after a successful migration).
Useful? React with 👍 / 👎.
| boolean immediateProxyBroadcast = proxyBroadcastDecider.usesImmediateForwarding(playerOnline); | ||
| if (immediateProxyBroadcast) { | ||
| Set<String> targets = proxyBroadcastDecider.resolveTargets(playerOnline, playerServer); | ||
| sendProxyBroadcast(targets, uuid, player, service, time, text == null ? "" : text.toString()); |
There was a problem hiding this comment.
On Bungee, normal vote events call this method with timeQueue=true, and an active GlobalData time change returns at line 1654 after placing the vote in timeChangeQueue. Because the new immediate-forwarding block is below that return, an offline vote configured with OfflineMode=FORWARD is not broadcast until processQueue eventually replays it, which can delay the supposedly immediate announcement for the entire rollover operation. The forwarding state needs to be handled before this queueing return, while ensuring the replay does not broadcast it again.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 611cf633dc
ℹ️ 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".
| new OfflineBungeeVote(voteId, player, uuid, service, time, realVote, | ||
| text.toString(), broadcastForwardedServers.contains(s))); |
There was a problem hiding this comment.
When SendVotesToAllServers and WaitForUserOnline are enabled with Scope.Mode: PLAYER_SERVER, an offline vote successfully forwarded to a designated server such as the lobby is cached for every other server with broadcastForwarded=false. If the voter later joins one of those servers, checkCachedVotes recomputes PLAYER_SERVER using the new online location and broadcasts the already-announced vote again. Track that standalone routing was handled independently of whether this particular reward server was a forwarding target.
Useful? React with 👍 / 👎.
| boolean broadcastForwarded = standaloneProxyBroadcast && !proxyBroadcastTargets.isEmpty() | ||
| && broadcastForwardedServers.containsAll(proxyBroadcastTargets); | ||
| getVoteCacheHandler().addOnlineVote(uuid, | ||
| new OfflineBungeeVote(voteId, player, uuid, service, time, realVote, text.toString())); | ||
| new OfflineBungeeVote(voteId, player, uuid, service, time, realVote, text.toString(), | ||
| broadcastForwarded)); |
There was a problem hiding this comment.
With SendVotesToAllServers: false, plugin messaging, and multiple offline-forward targets, a partial delivery is collapsed to broadcastForwarded=false unless every target succeeded. On login, checkOnlineVotes sends only one vote envelope to the player's current server and never retries the failed offline targets; it can also duplicate the announcement if that current server was among the successful targets. Persist the successful or pending target set for online-cache entries rather than reducing it to one aggregate boolean.
Useful? React with 👍 / 👎.
| Set<String> targets = proxyBroadcastDecider.resolveTargets(false, null); | ||
| broadcastForwardedServers.addAll( | ||
| sendProxyBroadcast(targets, uuid, player, service, time, "", false)); |
There was a problem hiding this comment.
During an active GlobalData time change, an offline FORWARD vote is broadcast here before the vote-delay check runs. If the vote is a duplicate that checkVoteDelay rejects when processQueue replays it, users have already seen an announcement for a vote that receives no normal processing or reward. The acceptance checks need to occur before this forwarding, or the announcement must remain pending until the queued vote is accepted.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a80c53edef
ℹ️ 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".
| broadcastForwardedServers.addAll( | ||
| sendProxyBroadcast(broadcastTargets, uuid, player, service, time, "", false)); |
There was a problem hiding this comment.
During an active GlobalData rollover with an offline voter and SendVotesToAllServers: false, two same-service votes can both pass this pre-forward delay check because the first vote exists only in timeChangeQueue; getLastVotesTime scans the database and the online/server caches but not that queue. Both votes are therefore announced here, while replaying the first adds it to the online cache and causes the second to be rejected without a reward. Fresh evidence beyond the earlier delay-order report is the missing timeChangeQueue lookup, so an accepted queued vote must reserve its delay slot before its standalone broadcast is sent.
Useful? React with 👍 / 👎.
| if (toSend) { | ||
| boolean broadcastHere = true; | ||
| if (getConfig().getProxyBroadcastEnabled()) { | ||
| boolean broadcastHere = cache.needsBroadcastOn(server); |
There was a problem hiding this comment.
With PLUGINMESSAGING, SendVotesToAllServers, and WaitForUserOnline enabled, a FORWARD target with no connected carrier initially remains pending as intended, but when another player joins that target, checkCachedVotes rejects the entry at the toSend gate because the voter is still offline, so this new needsBroadcastOn path is never reached. The announcement then waits until the voter personally joins that backend, defeating FORWARD even though a carrier is available; retry the standalone pending target independently of the reward-delivery gate.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18cb5269da
ℹ️ 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".
| globalMessageProxyHandler.sendMessage(targetServer, delay, envelope); | ||
| forwarded.add(targetServer); |
There was a problem hiding this comment.
With a non-plugin transport, a failed send is still recorded as forwarded: for example, the MYSQL branch catches sendToBackend's SQLException inside GlobalMessageProxyHandler.sendMessage, after which this code unconditionally adds the target. A queued or cached vote will then persist that target as delivered and suppress its later normal-envelope broadcast. Fresh evidence beyond the earlier plugin-messaging case is that every other transport still uses a void wrapper that cannot report failure; only add the target when the selected transport confirms that it accepted the message.
Useful? React with 👍 / 👎.
| if (cache.isProxyBroadcastHandled()) { | ||
| Set<String> pendingTargets = new LinkedHashSet<>(cache.getBroadcastTargets()); | ||
| pendingTargets.removeAll(cache.getBroadcastForwardedServers()); | ||
| cache.getBroadcastForwardedServers().addAll(sendProxyBroadcast(pendingTargets, | ||
| cache.getUuid(), cache.getPlayerName(), cache.getService(), cache.getTime(), | ||
| cache.getText(), false)); |
There was a problem hiding this comment.
With PLUGINMESSAGING and SendVotesToAllServers: false, an offline vote is stored only in the voter-keyed online cache, so if an immediate target has no carrier this retry block cannot run when some other player later joins that target: checkOnlineVotes is invoked with the joining player's UUID, while checkCachedVotes scans only server caches. The pending broadcast therefore remains queued until the voter logs in or is online during a periodic check, contrary to FORWARD's voter-independent delivery. Fresh evidence beyond the earlier per-target persistence report is that the newly persisted pending state has no target-driven retry path for online-cache entries.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db01da12c7
ℹ️ 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".
| redisHandler.publishEnvelope(getConfig().getRedisPrefix() + "VotingPlugin_" + server, envelope); | ||
| // RedisHandler currently swallows publish failures and exposes no acknowledgement, | ||
| // so the standalone delivery must remain pending. | ||
| return false; |
There was a problem hiding this comment.
With REDIS and offline FORWARD enabled, publishEnvelope can successfully deliver the standalone announcement, but this unconditional false leaves the target marked pending. In SendVotesToAllServers mode, the subsequent normal vote envelope therefore also sets broadcastHere=true, producing an immediate duplicate; voter-keyed cached votes are retained and republished on later logins as well. The identical unconditional failure result after client.sendEnvelope causes the same socket behavior. Fresh evidence after the earlier transport-success report is that the new implementation deliberately reports failure even after invoking a successful send.
Useful? React with 👍 / 👎.
| // Totals processing (primary server OR no multiproxy) | ||
| if (getConfig().getPrimaryServer() || !getConfig().getMultiProxySupport()) { | ||
| if (getConfig().getBungeeManageTotals()) { | ||
| data = getProxyMySQL().getExactQuery(new Column("uuid", new DataValueString(uuid))); |
There was a problem hiding this comment.
When a vote arrives just as a GlobalData rollover finishes, data is loaded here before checkForFinishedTimeChanges(). That call can invoke onTimeChangedFinished, which wipes the period-total column and replays queued votes; if the second isTimeChangedHappened() check is then false, this vote continues using the pre-wipe snapshot and writes the old total back over both the reset and any replayed increments. Fresh evidence beyond the earlier queue-order reports is this stale database snapshot crossing the synchronous rollover-completion callback; complete the rollover before loading data, or reload it afterward.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8abc3ecc6
ℹ️ 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 (deliveredReward && getConfig().getMultiProxySupport() | ||
| && getConfig().getMultiProxyOneGlobalReward()) { | ||
| multiProxyHandler.sendClearVote(uuid, player); |
There was a problem hiding this comment.
When MultiProxyOneGlobalReward is enabled and a voter-keyed vote still has failed FORWARD targets on any proxy, delivering the reward here broadcasts ClearVote. MultiProxyHandler handles that message through clearVote(uuid), whose implementation calls removeOnlineVotes(uuid), deleting the retained incomplete entries from every proxy; consequently, a target that lacked a plugin-message carrier during the initial attempt is never retried. Clear the reward eligibility without removing cache entries whose proxy broadcast is still incomplete.
Useful? React with 👍 / 👎.
| try (Jedis jedis = new Jedis(new HostAndPort(getConfig().getRedisHost(), getConfig().getRedisPort()), | ||
| config.build())) { |
There was a problem hiding this comment.
With BungeeMethod.REDIS, every call routed through globalMessageProxyHandler now constructs a fresh blocking Jedis connection, including ordinary vote, update, and status envelopes—not just standalone broadcasts. During a Redis outage, the per-target loops execute these calls serially with a two-second connection timeout while vote is synchronized, so a network with many backends can stall all vote processing for tens of seconds per vote. Use a reusable/pool-backed publisher that can return the Redis PUBLISH result instead of reconnecting for every envelope.
Useful? React with 👍 / 👎.
| + qi("broadcastForwardedServers") + " = ?, " + qi("rewardDelivered") + " = ? WHERE " | ||
| + qi("uuid") + " = ? AND " + qi("service") + " = ? AND " + qi("time") + " = ?;"; |
There was a problem hiding this comment.
When two cached votes for the same UUID and service receive the same millisecond timestamp, this predicate updates both rows with one vote's forwarding and reward state even though each row has a distinct voteid. This can occur with back-to-back votes when vote-delay enforcement is disabled, and after a reload one vote may consequently suppress a broadcast as already forwarded. The server-cache SQL updater and JSON update paths use the same non-unique tuple, so use voteid whenever it is available and retain the tuple only as a legacy fallback.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0b8a935ef
ℹ️ 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".
| try (Socket socket = new Socket()) { | ||
| socket.connect(new InetSocketAddress(host, port), 2000); |
There was a problem hiding this comment.
With BungeeMethod=SOCKETS, globalMessageProxyHandler routes ordinary vote, update, and status envelopes here as well as standalone broadcasts, so every envelope now creates and synchronously connects a fresh socket. Because vote() is synchronized and sends to backend servers serially, each unreachable target can incur this two-second timeout for every attempted envelope, stalling all vote processing across a multi-server network; reuse the already initialized clientHandles or another persistent/pool-backed sender while obtaining delivery confirmation.
Useful? React with 👍 / 👎.
| if (publisherPool == null || System.currentTimeMillis() < redisPublisherRetryAfter) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
With BungeeMethod=REDIS, one transient publish exception sets this global cooldown, after which every envelope for every target returns false for two seconds without attempting Redis. The caller in globalMessageProxyHandler ignores that result for ordinary vote and update envelopes, so in a SendVotesToAllServers loop a single stale pooled connection can cause the remaining backend rewards to be silently skipped without being cached; limit backoff to retryable standalone broadcasts or make normal delivery failures enter the vote cache rather than dropping them.
Useful? React with 👍 / 👎.
| if (cache.getBroadcastForwardedServers().addAll(forwarded)) { | ||
| cache.setBroadcastForwarded(cache.isProxyBroadcastComplete()); | ||
| getVoteCacheHandler().updateOnlineVote(cachedUuid, cache); |
There was a problem hiding this comment.
When MultiProxyOneGlobalReward clears a vote whose standalone broadcast is still incomplete, clearOnlineVoteRewards correctly retains it with rewardDelivered=true; however, when this retry later delivers the last target, the code only persists the now-complete entry. If the voter remains offline, checkOnlineVotes never removes it, so it remains in memory and SQL/JSON until the optional age purge and is scanned for every subsequent login by retryPendingOnlineBroadcasts; remove the entry once both rewardDelivered and isProxyBroadcastComplete() are true.
Useful? React with 👍 / 👎.
| getVoteCacheHandler().getTimeChangeQueue().add(delayedVote); | ||
| if (proxyBroadcastHandled) { | ||
| Set<String> forwarded = sendProxyBroadcast(broadcastTargets, uuid, player, service, time, "", | ||
| false); |
There was a problem hiding this comment.
With an active GlobalData rollover and offline FORWARD mode, this adds the accepted vote only to the in-memory queue and then immediately publishes its announcement. Timed queue rows are serialized only by VoteCacheHandler.saveVoteCache(), which is called during graceful disable rather than when the vote is queued, so a proxy crash after this send but before rollover replay leaves users with an announced vote that disappears on restart and never updates totals or rewards; durably store the queued vote and its delivery state before publishing the standalone broadcast.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b4f9c2565d
ℹ️ 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".
| vote(vote.getName(), vote.getService(), true, false, vote.getTime(), null, null, vote); | ||
| getVoteCacheHandler().removeTimeVote(vote); |
There was a problem hiding this comment.
When replay exits early—for example because getProxyMySQL() is unavailable—or catches an exception internally, vote(...) returns void, so this unconditional removal deletes the durable timed entry despite no totals or rewards being processed. Because FORWARD may already have announced the vote before the rollover finished, a transient replay failure permanently loses the corresponding vote; make replay report success and remove the entry only after successful processing.
Useful? React with 👍 / 👎.
| timeChangeQueue.remove(vote); | ||
| if (useMySQL) { | ||
| timedVoteCacheTable.removeVote(vote); |
There was a problem hiding this comment.
With MySQL cache storage, a transient failure in ProxyTimedVoteCacheTable.removeVote is caught and only logged, but the vote has already been removed from timeChangeQueue. The proxy therefore stops retrying the deletion while the SQL row remains; after the next restart, load() restores and replays that row, incrementing totals and delivering rewards a second time. Remove the in-memory entry only after the durable deletion succeeds, or retain it for a deletion retry.
Useful? React with 👍 / 👎.
| Set<String> forwarded = sendProxyBroadcast(Collections.singleton(target), uuid, player, | ||
| service, time, "", false); |
There was a problem hiding this comment.
When an offline FORWARD vote arrives during a GlobalData rollover, this publishes the standalone broadcast with an empty totals payload before the queued replay updates the backend user's totals. BroadcastHandler evaluates PlaceholderAPI placeholders against that user, while the standalone receiver ignores the envelope's totals field, so broadcast formats containing vote-total placeholders render the pre-vote values. Supply the projected post-rollover snapshot to the receiver or defer rendering until those totals are available.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1bd7f56d14
ℹ️ 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".
| placeholders.put("VotingPlugin_total", Integer.toString(monthTotal)); | ||
| placeholders.put("VotingPlugin_total_alltime", Integer.toString(allTimeTotal)); | ||
| placeholders.put("VotingPlugin_total_monthly", Integer.toString(monthTotal)); |
There was a problem hiding this comment.
When UseMonthDateTotalsAsPrimaryTotal is enabled, %VotingPlugin_total% and %VotingPlugin_total_monthly% normally resolve through VotingPluginUser.getTotal(Monthly), which reads the dated monthly column, but the new pre-rendering replaces both with the legacy monthTotal. Whenever those columns differ—especially around monthly rollover—FORWARD broadcasts display a different total from every normal placeholder evaluation; use dateMonthTotal for these PAPI placeholders when it is the configured primary value.
Useful? React with 👍 / 👎.
| if (!vote(vote.getName(), vote.getService(), true, false, vote.getTime(), null, null, vote)) { | ||
| scheduleTimeVoteRetry(); | ||
| return; | ||
| } | ||
| processedTimeVotesPendingRemoval.add(vote); |
There was a problem hiding this comment.
If the proxy stops after this vote(...) call succeeds but before removeTimeVote deletes the durable row, the row is loaded and processed again after restart, duplicating totals, vote-party progress, and rewards. The new processedTimeVotesPendingRemoval guard is fresh evidence beyond the prior deletion-retry issue because it exists only in memory and cannot close this normal crash window; record completion durably or make replay idempotent by voteId before applying side effects.
Useful? React with 👍 / 👎.
| checkCachedVotes(serverName); | ||
| retryPendingOnlineBroadcasts(serverName); | ||
| checkOnlineVotes(playerName, uuid, serverName); |
There was a problem hiding this comment.
When SendVotesToAllServers is false and an offline FORWARD broadcast initially fails over Redis, MySQL, MQTT, or sockets, its pending state lives only in the voter-keyed online cache, but this retry method is invoked only from the login path. Unlike plugin messaging, recovery of those transports produces no carrier-login event, and the periodic tasks call checkOnlineVotes only for currently online voters, so a recovered transport may never receive the announcement while the voter remains offline and no player newly logs into the target; include these pending entries in a periodic transport retry.
Useful? React with 👍 / 👎.
| VoteTimeQueue delayedVote = new VoteTimeQueue(voteId, player, service, time, | ||
| proxyBroadcastHandled, broadcastTargets, broadcastForwardedServers); |
There was a problem hiding this comment.
When a non-primary proxy receives a multi-proxy vote during an active GlobalData rollover, text contains the primary proxy's totals snapshot, but this queued record stores only routing state. processQueue later replays it with text=null; because a non-primary proxy has processesTotals=false, no replacement snapshot is created, and the subsequent text.toString() throws. The row is then retained and retried forever, incrementing vote-party state on each attempt without delivering the reward, so persist and restore the incoming snapshot with the queued vote.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f41e9d953
ℹ️ 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 (totals != null) { | ||
| pendingTotals.put(uuid, totals); | ||
| } |
There was a problem hiding this comment.
When a forwarded offline vote starts a BATCH_WINDOW_PER_PLAYER batch and the same player then receives a normal vote before the flush, the second call supplies totals=null, but this conditional leaves the first vote's snapshot in pendingTotals. flushBatch consequently pre-renders the combined announcement with the older snapshot instead of letting PlaceholderAPI read the totals updated by the later vote, so total and points placeholders undercount. Remove the pending snapshot when a newer batch entry has no override, or otherwise track which source represents the latest totals.
Useful? React with 👍 / 👎.
| placeholders.put("VotingPlugin_total_monthly", Integer.toString(primaryMonthTotal)); | ||
| placeholders.put("VotingPlugin_total_weekly", Integer.toString(weeklyTotal)); | ||
| placeholders.put("VotingPlugin_total_daily", Integer.toString(dailyTotal)); | ||
| placeholders.put("VotingPlugin_points", Integer.toString(points)); | ||
| placeholders.put("VotingPlugin_points_format", numberFormat.format(points)); |
There was a problem hiding this comment.
On a FORWARD target that has not yet received the normal vote envelope, formats containing %VotingPlugin_BungeeVotePartyVotesCurrent%, %VotingPlugin_BungeeVotePartyVotesNeeded%, or %VotingPlugin_BungeeVotePartyVotesRequired% still render the backend's old state. The standalone listener parses a snapshot containing votePartyCurrent and votePartyRequired, but this replacement map omits them and, unlike handleWireVote, the listener does not update BungeeHandler's vote-party state before PlaceholderAPI runs. Include these snapshot fields in standalone rendering without globally mutating state for an early announcement.
Useful? React with 👍 / 👎.
| if (!vote(vote.getName(), vote.getService(), true, false, vote.getTime(), queuedTotals, null, vote)) { | ||
| scheduleTimeVoteRetry(); | ||
| return; |
There was a problem hiding this comment.
If the head rollover entry can no longer be replayed—for example its UUID cannot be resolved after restart, a changed Bedrock-prefix policy rejects its name, or the replay-time delay check rejects it—vote(...) returns false deterministically, but this path retains the entry and returns before examining any later votes. The scheduled retry then repeats the same head entry every five seconds forever, so one unrecoverable row blocks totals and rewards for every valid rollover vote behind it. Distinguish transient processing failures from terminal validation rejections and quarantine or remove only the latter before continuing the queue.
Useful? React with 👍 / 👎.
| if (proxyBroadcastHandled) { | ||
| for (String target : broadcastTargets) { | ||
| Set<String> forwarded = sendProxyBroadcast(Collections.singleton(target), uuid, player, | ||
| service, time, projectedTotals == null ? "" : projectedTotals.toString(), false); |
There was a problem hiding this comment.
When an offline FORWARD vote arrives during a GlobalData rollover and a target initially cannot accept the standalone message—for example a plugin-message backend has no carrier—the failed target is persisted in delayedVote, but no login or periodic retry scans timeChangeQueue; both retry methods only inspect the online cache, while checkCachedVotes only inspects server caches. Even if a carrier appears or a broker recovers immediately, the announcement therefore remains pending until processQueue runs after the rollover finishes. Include timed entries in the target-driven and periodic retry paths so a failed first attempt does not restore the rollover-length delay.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a454872430
ℹ️ 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 new VoteTotalsSnapshot(allTimeTotal, monthTotal, weeklyTotal, dailyTotal, points, votePartyVotes, | ||
| currentVotePartyVotesRequired, dateMonthTotal); |
There was a problem hiding this comment.
When GlobalData rollover is active and the vote party is enabled, the queue branch creates this snapshot before addVoteParty() and then returns, while acceptedQueuedVotes is applied only to player totals. An offline FORWARD broadcast containing the vote-party placeholders therefore shows the pre-vote current/required values—and multiple queued votes all show the same state—even though ordinary votes increment and potentially trigger the party before creating their snapshot. Project the queued vote-party increments and threshold transitions into these fields as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5808c02986
ℹ️ 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".
| voteCheckTask = server.getScheduler().buildTask(this, () -> { | ||
| if (getVotingPluginProxy().getGlobalDataHandler() == null | ||
| || !getVotingPluginProxy().getGlobalDataHandler().isTimeChangedHappened()) { | ||
| getVotingPluginProxy().retryPendingOnlineBroadcasts(); |
There was a problem hiding this comment.
On Velocity, an offline FORWARD vote queued during a GlobalData rollover is not retried periodically if its initial standalone delivery fails, because this newly added call remains inside the !isTimeChangedHappened() guard; the retry method now scans timed votes, but it cannot run until the rollover ends unless a player login triggers the separate target-specific path. Fresh evidence after the earlier timed-retry report is this Velocity-only scheduling guard, which can still delay the supposedly immediate broadcast for the full rollover when no one logs into the target.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89b041c4cd
ℹ️ 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".
| service, time, projectedTotals == null ? "" : projectedTotals.toString(), false); | ||
| if (delayedVote.getBroadcastForwardedServers().addAll(forwarded)) { | ||
| broadcastForwardedServers.addAll(forwarded); | ||
| getVoteCacheHandler().updateTimeVote(delayedVote); |
There was a problem hiding this comment.
When a standalone broadcast succeeds during an active rollover but updateTimeVote fails because of a transient SQL/JSON storage error, this return value is ignored and the in-memory target remains marked delivered. No later retry attempts to persist that state, so if the proxy restarts before the rollover completes, the durable row is loaded with the target still pending and republishes an already delivered announcement. Handle the failed update by retaining a persistence retry before considering the target durably forwarded.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd51831051
ℹ️ 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".
| broadcastForwardedServers.addAll(sendProxyBroadcast(remainingTargets, uuid, player, service, time, | ||
| text == null ? "" : text.toString(), false)); |
There was a problem hiding this comment.
When BungeeManageTotals is false and the backend's WaitUntilVoteDelay rejects a real vote, the proxy skips checkVoteDelay because managesTotals is false but still sends this standalone FORWARD announcement. The subsequent normal envelope returns from PlayerVoteListener before broadcasting or rewarding, so users see an announcement for a rejected vote; defer the standalone message until the backend confirms acceptance in this configuration.
Useful? React with 👍 / 👎.
| if (cache.getBroadcastForwardedServers().addAll(forwarded)) { | ||
| cache.setBroadcastForwarded(cache.isProxyBroadcastComplete()); | ||
| getVoteCacheHandler().updateServerVote(server, cache); |
There was a problem hiding this comment.
With SQL cache storage and WaitForUserOnline keeping the reward cached, a transient failure in updateServerVote is swallowed by ProxyVoteCacheTable.updateProxyBroadcastState, while this in-memory entry remains marked as forwarded. If the proxy restarts before reward delivery, the durable row is loaded with the target still pending and republishes an already successful announcement; make this update report success and retain dirty state for retry, as the timed-vote path already does.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f1210d975
ℹ️ 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 (cache.isProxyBroadcastHandled() && !cache.isProxyBroadcastComplete()) { | ||
| retained.add(cache); |
There was a problem hiding this comment.
With a voter-keyed cache and Redis, if an earlier standalone failure has set redisPublisherRetryAfter and the voter logs in during that cooldown, the retry above returns false, but the normal vote envelope is published through globalMessageProxyHandler without the cooldown and can successfully broadcast with broadcastHere=true. This condition nevertheless retains the vote as broadcast-incomplete, so the periodic retry later publishes the standalone announcement again. Fresh evidence beyond the earlier cooldown report is that normal publishing now bypasses the cooldown while its successful fallback broadcast is not reflected in the pending-target state.
Useful? React with 👍 / 👎.
| QueuedVoteResult result = vote(vote.getName(), vote.getService(), true, false, vote.getTime(), queuedTotals, | ||
| null, vote); |
There was a problem hiding this comment.
When an online-mode player's name mapping changes or is unavailable during rollover replay, passing null here forces a fresh name-to-UUID lookup even though the accepted vote's UUID is now persisted in VoteTimeQueue. If that lookup fails—for example after a restart and rename with external UUID lookup disabled—the vote is classified as terminal and its durable row is removed without totals or rewards. Fresh evidence after the earlier queue-blocking report is that the new durable UUID field is loaded but still ignored during replay; pass vote.getUuid() instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13bd7b5ebd
ℹ️ 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".
| Set<String> pendingTargets = new LinkedHashSet<>(cache.getBroadcastTargets()); | ||
| pendingTargets.removeAll(cache.getBroadcastForwardedServers()); | ||
| cache.getBroadcastForwardedServers().addAll(sendProxyBroadcast(pendingTargets, | ||
| cache.getUuid(), cache.getPlayerName(), cache.getService(), cache.getTime(), | ||
| cache.getText(), false)); |
There was a problem hiding this comment.
When a FORWARD target is added to BlockedServers after the vote was cached but before the voter logs in, this login path sends the entire persisted pending-target set without applying the current block list. The periodic retry path filters blocked targets, but checkOnlineVotes can still publish the announcement to a backend explicitly configured not to receive votes; filter or reconcile pending targets before calling sendProxyBroadcast here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a22f6e59b
ℹ️ 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".
| removeOnlineVotes(uuid); | ||
| for (OfflineBungeeVote vote : retained) { | ||
| addOnlineVote(uuid, vote); |
There was a problem hiding this comment.
When MultiProxyOneGlobalReward retains an entry with pending FORWARD targets, this deletes every persisted vote for the UUID and then reinserts the retained entries one by one, but removeOnlineVotes swallows SQL deletion failures and addOnlineVote likewise cannot report insertion failure. A transient failure can therefore either duplicate retained rows (failed delete followed by successful inserts), causing the pending broadcast to be sent multiple times after restart, or lose them (successful delete followed by failed insert). Replace the rows transactionally or propagate persistence success before changing the in-memory cache.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3b0efe717
ℹ️ 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".
| broadcastForwardedServers.addAll(sendProxyBroadcast(remainingTargets, uuid, player, service, time, | ||
| text == null ? "" : text.toString(), false)); |
There was a problem hiding this comment.
When an offline FORWARD vote must subsequently enter the server or online cache, this publishes the announcement before addServerVote/addOnlineVote attempts durable storage. With MySQL cache storage, both insert methods swallow SQL failures and leave only the in-memory entry, so a transient cache-database failure followed by a proxy restart loses the reward even though users already saw the successful vote announcement. Make the initial cache insert report success and persist the reward and delivery state before sending the standalone broadcast.
Useful? React with 👍 / 👎.
| boolean processesTotals = getConfig().getPrimaryServer() || !getConfig().getMultiProxySupport(); | ||
| boolean managesTotals = processesTotals && getConfig().getBungeeManageTotals(); | ||
| boolean canValidateStandaloneBroadcast = canForwardStandaloneBroadcast(managesTotals); |
There was a problem hiding this comment.
With MultiProxySupport enabled, every non-primary proxy sets processesTotals=false, which makes this validation gate false even when the vote was accepted by the totals-managing primary and arrived with its totals snapshot. If the voter is offline on that secondary, none of its local FORWARD targets receive a standalone broadcast; the vote is instead placed in the ordinary online cache and waits for a later login, while only the primary proxy's local backends were announced immediately. Preserve the primary's accepted/validated context so secondary proxies can perform their own configured FORWARD delivery.
Useful? React with 👍 / 👎.
1 participant
Summary
ProxyBroadcast.OfflineModeisFORWARDRoot cause
WaitForUserOnline: truecoupled reward delivery to proxy broadcast timing. Aggregate forwarded state then lost the distinction between original targets, successful targets, pending targets, non-target reward servers, and reward delivery.The GlobalData path also loaded acceptance/totals state across rollover callbacks, allowing announcements without a reserved delay slot and stale pre-reset totals to be written back after rollover completion.
User impact
With
OfflineMode: FORWARD, an accepted offline vote is announced immediately to every reachable backend in the configured scope while its reward may remain queued until the voter joins.Successful targets are not retried, failed targets remain pending and retry when transport becomes available, non-target cached rewards cannot rebroadcast the vote, and reward delivery happens at most once. Legacy cache entries retain their previous pending-broadcast behavior.
Validation
AI disclosure
This PR was created with assistance from OpenAI Codex.