Skip to content

foos implementations - #555

Merged
phinner merged 24 commits into
xpdustry:masterfrom
JasonP01:foosImplementation
Aug 2, 2026
Merged

foos implementations#555
phinner merged 24 commits into
xpdustry:masterfrom
JasonP01:foosImplementation

Conversation

@JasonP01

@JasonP01 JasonP01 commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Add some foos support and some nice fun commands

None of this is currently functional as foos doesnt support these yet

Summary by CodeRabbit

  • New Features
    • Added teleport, status effect, unit change, and player name change commands.
    • Added kick moderation support, including moderator commands and in-game announcements.
    • Improved Foo client login, packet handling, moderation, and player data synchronization.
    • Updated excavation voting with consistent vote and force execution handling.
    • Improved account login event processing.
  • Localization
    • Added English messages for kick announcements, disallowed actions, invalid targets, and unknown arguments.

Note

Add foo client integration with kick punishment, teleport, and moderator commands

  • Introduces FoosClientDetector, replacing SimpleClientDetector, which handles custom packet types: identifying foo clients, pushing player data, processing rate-limited logins, relaying excavation votes, and accepting OVERSEER-authorized moderation packets.
  • Adds FunHandler with /teleport, /statuseffect, /changeunit, and /changename commands for OVERSEERs and moderators.
  • Introduces Punishment.Type.KICK and PunishmentDuration.NONE; when a KICK punishment is created, targeted players are immediately disconnected and a server-wide announcement is broadcast.
  • Extracts ExcavateManager interface from ExcavateCommand so FoosClientDetector can submit excavation votes.
  • Risk: SimpleClientDetector is removed entirely; any behavior relying on it is replaced by FoosClientDetector.

Macroscope summarized a6914cf.

@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The plugin registers FunHandler, binds ClientDetector to FoosClientDetector, adds Foos-client synchronization and packet moderation, introduces teleport and utility commands, integrates excavate voting, and adds kick punishment support with localized messages.

Changes

Mindustry moderation and fun features

Layer / File(s) Summary
Kick punishment support
imperium-common/.../security/*, imperium-backend/.../commands/ModerationCommand.kt, imperium-mindustry/.../security/*, imperium-mindustry/.../translation/Messages.kt, imperium-mindustry/.../bundles/bundle_en.properties
Adds the KICK punishment type, no-duration punishment support, kick commands, kick execution, and kick announcements.
Foos detection and packet moderation
imperium-mindustry/.../game/FoosClientDetector.kt, imperium-mindustry/.../MindustryModule.kt, imperium-mindustry/.../account/AccountListener.kt
Tracks Foo clients, synchronizes player data, handles login and moderation packets, and applies mapped punishments.
Fun packet and command behavior
imperium-mindustry/.../game/FunHandler.kt, imperium-mindustry/.../ImperiumPlugin.kt
Adds conditional teleport packet handling and teleport, status-effect, unit-change, and name-change commands, then registers the handler.
Excavate integration and login helper visibility
imperium-mindustry/.../world/ExcavateCommand.kt, imperium-mindustry/.../MindustryModule.kt, imperium-mindustry/.../account/LoginWindow.kt
Adds the ExcavateManager contract and binding and exposes login message helpers.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant FoosClient
  participant FoosClientDetector
  participant DataStoreService
  participant PunishmentManager
  FoosClient->>FoosClientDetector: Send moderation payload
  FoosClientDetector->>DataStoreService: Read sender rank
  FoosClientDetector->>PunishmentManager: Apply mapped punishment
  PunishmentManager-->>FoosClientDetector: Return punishment identifier
  FoosClient-->>FoosClientDetector: Receive result message
Loading

Suggested reviewers: phinner

Poem

A rabbit checks each packet’s flight,
And hops through commands day and night.
Foos data travels, kicks are logged,
Teleport paths are neatly guarded.
“Hop!” says Bun, “the patch is bright!”

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title identifies the pull request's primary focus on Foos-related implementations, including client detection and supporting functionality.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Test Results

44 tests   44 ✅  12s ⏱️
10 suites   0 💤
10 files     0 ❌

Results for commit 260cf1d.

♻️ This comment has been updated with latest results.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (1)
imperium-mindustry/src/main/resources/com/xpdustry/imperium/mindustry/bundles/bundle_en.properties (1)

304-305: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Add missing trailing periods and standardize spelling.

For consistency with the rest of the translation file, add trailing periods to these new properties. Also, consider standardizing on "spelled" rather than "spelt".

📝 Proposed fix
-imperium.player.invalid.target=The player {0} you attempted to perform that action on is no longer valid
-imperium.command.arg.unknown=Unknown argument {0}. Ensure you spelt it correctly
+imperium.player.invalid.target=The player {0} you attempted to perform that action on is no longer valid.
+imperium.command.arg.unknown=Unknown argument {0}. Ensure you spelled it correctly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@imperium-mindustry/src/main/resources/com/xpdustry/imperium/mindustry/bundles/bundle_en.properties`
around lines 304 - 305, Update the imperium.player.invalid.target and
imperium.command.arg.unknown translation values to end with periods, and change
“spelt” to the file’s standardized “spelled” spelling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FoosClientDetector.kt`:
- Around line 124-139: Update sendPlayerData to launch its database lookup
inside the existing coroutine scope, ensuring the synchronous
store.selectBySessionKey call and subsequent packet construction occur off the
Mindustry main thread while preserving the current resend and player-data
behavior.
- Around line 57-77: Move JSON parsing and the Groups.player lookup out of the
scope.launch block and execute them on the Mindustry main thread before
launching the coroutine; pass the resolved target and parsed targetId into the
coroutine, while keeping blocking database work inside scope.launch. Update the
foosFreeze packet handler without changing its authorization and invalid-target
behavior.
- Around line 101-104: Update resendPlayerData to accept only the
PlayerLoginEvent parameter, then obtain the player from event.player when
calling sendPlayerData. Preserve the existing resend behavior and force flag.

In
`@imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FunHandler.kt`:
- Around line 33-35: Update the exception handling around teleport packet
processing to avoid passing the caught exception to Log.err, preventing stack
traces for malformed network payloads. Keep only a brief warning message, or
remove the log if parsing failures are expected.
- Around line 41-48: Update onTeleportCommand to require an explicit player
argument when the sender is not a player, avoiding sender.player for console
execution; preserve the existing sender-player fallback for player senders.
Rename the singular target variable appropriately and simplify the fallback
using the established onStatusCommand pattern where applicable.
- Around line 21-36: Update the "teleport" packet handler in
Vars.netServer.addPacketHandler to use the sender parameter and reject
unauthorized clients before parsing or applying teleport data. Reuse the
existing rank or permission check used by the project, allowing only permitted
senders to call setUnitPosition while preserving the current handling for
authorized packets.

---

Nitpick comments:
In
`@imperium-mindustry/src/main/resources/com/xpdustry/imperium/mindustry/bundles/bundle_en.properties`:
- Around line 304-305: Update the imperium.player.invalid.target and
imperium.command.arg.unknown translation values to end with periods, and change
“spelt” to the file’s standardized “spelled” spelling.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9b117759-f8f1-4812-8e4e-68f0bf5a95fc

📥 Commits

Reviewing files that changed from the base of the PR and between 4b48eff and c74988e.

📒 Files selected for processing (7)
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/ImperiumPlugin.kt
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/MindustryModule.kt
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/ClientDetector.kt
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FoosClientDetector.kt
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FunHandler.kt
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/translation/Messages.kt
  • imperium-mindustry/src/main/resources/com/xpdustry/imperium/mindustry/bundles/bundle_en.properties
💤 Files with no reviewable changes (1)
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/ClientDetector.kt

@JasonP01
JasonP01 marked this pull request as draft July 20, 2026 02:53
@JasonP01
JasonP01 marked this pull request as ready for review July 20, 2026 14:13

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FoosClientDetector.kt`:
- Around line 148-156: Update getPunishmentType to return a nullable
Punishment.Type and return null for unrecognized type values instead of
defaulting to BAN. In the moderation packet handling that calls
getPunishmentType, detect null, reject the packet, and send an explanatory
message to the sender before any punishment is applied.

In
`@imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FunHandler.kt`:
- Around line 96-103: The onNameChangeCommand method applies untrusted name
input without validation. Before calling target.name, trim the supplied name,
enforce the supported length bounds, and strip or reject Mindustry color/format
tags so arbitrary formatting and impersonation are prevented; preserve the
existing command response for valid names and handle invalid input without
changing the player name.
- Around line 34-56: Correct the authorization condition in onImperiumInit’s
teleport packet handler so only ranks at or above Rank.OVERSEER can teleport the
selected player, matching the declared Rank ordering. Preserve the existing
player lookup and FooClient navTp fallback behavior.

In
`@imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/security/ModerationCommand.kt`:
- Around line 71-78: Update the server-side PunishmentListener create-event
handler to add a Punishment.Type.KICK branch that invokes the Mindustry kick
API, while preserving the existing BAN disconnect behavior. The kick command
entry points require no direct changes:
imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/security/ModerationCommand.kt:71-78,
imperium-backend/src/main/kotlin/com/xpdustry/imperium/backend/commands/ModerationCommand.kt:103-110,
and
imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FoosClientDetector.kt:84-94
are corrected by the punishment-handler change.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 926b236d-f517-4f70-8ac0-5f9c6de303d4

📥 Commits

Reviewing files that changed from the base of the PR and between c74988e and 60b4292.

📒 Files selected for processing (9)
  • imperium-backend/src/main/kotlin/com/xpdustry/imperium/backend/commands/ModerationCommand.kt
  • imperium-common/src/main/kotlin/com/xpdustry/imperium/common/security/Punishment.kt
  • imperium-common/src/main/kotlin/com/xpdustry/imperium/common/security/PunishmentDuration.kt
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/ImperiumPlugin.kt
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FoosClientDetector.kt
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FunHandler.kt
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/security/ModerationCommand.kt
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/translation/Messages.kt
  • imperium-mindustry/src/main/resources/com/xpdustry/imperium/mindustry/bundles/bundle_en.properties
🚧 Files skipped from review as they are similar to previous changes (3)
  • imperium-mindustry/src/main/resources/com/xpdustry/imperium/mindustry/bundles/bundle_en.properties
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/ImperiumPlugin.kt
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/translation/Messages.kt

@JasonP01

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FoosClientDetector.kt (1)

129-129: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use sendAnnouncement for consistent GUI feedback.

This line uses sendMessage to display the invalid credentials error, whereas loginPlayer uses sendAnnouncement for the exact same error (line 202). Consider using sendAnnouncement here as well for a consistent user experience.

♻️ Proposed fix
-                    if (username.isBlank() || password.isBlank()) return@launch player.asAudience.sendMessage(gui_login_failure_invalid_credentials())
+                    if (username.isBlank() || password.isBlank()) return@launch player.asAudience.sendAnnouncement(gui_login_failure_invalid_credentials())
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FoosClientDetector.kt`
at line 129, Replace sendMessage with sendAnnouncement in the
invalid-credentials branch of the login flow, preserving the existing
gui_login_failure_invalid_credentials() message and early return. Align this
path with loginPlayer’s corresponding feedback behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FoosClientDetector.kt`:
- Around line 114-119: Wrap the JSON parsing and vote handling inside the
"excavateVote" packet handler with try-catch, matching the existing
exception-handling pattern used by the login packet handler. Catch
malformed-packet parsing failures from Jval.read(data) and handle them without
allowing the exception to escape the main-thread packet callback.

---

Nitpick comments:
In
`@imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FoosClientDetector.kt`:
- Line 129: Replace sendMessage with sendAnnouncement in the invalid-credentials
branch of the login flow, preserving the existing
gui_login_failure_invalid_credentials() message and early return. Align this
path with loginPlayer’s corresponding feedback behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e7b8b86f-4b78-4abc-a71a-ab0ffcf6e179

📥 Commits

Reviewing files that changed from the base of the PR and between 7f131ba and b033c34.

📒 Files selected for processing (4)
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/MindustryModule.kt
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/account/LoginWindow.kt
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FoosClientDetector.kt
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/world/ExcavateCommand.kt
🚧 Files skipped from review as they are similar to previous changes (1)
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/MindustryModule.kt

@JasonP01
JasonP01 requested a review from phinner July 21, 2026 13:41
@JasonP01

Copy link
Copy Markdown
Contributor Author

foos client side done, you can test the server and client support by compiling and running this foos branch https://github.com/JasonP01/foos/tree/dev

@JasonP01

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@JasonP01

Copy link
Copy Markdown
Contributor Author

@coderabbitai resume

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Reviews resumed.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FoosClientDetector.kt (4)

107-110: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Correct the packet name in the error message.

This handler processes foosModeration, but both the log and player-facing message say foosFreeze, which makes malformed moderation packets difficult to diagnose.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FoosClientDetector.kt`
around lines 107 - 110, Update the exception handler around the foosModeration
packet processing to replace the incorrect foosFreeze label in both Log.err and
playerObject.sendMessage with foosModeration, preserving the existing error
handling behavior.

168-177: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Avoid sending the fallback rank during login synchronization.

DataStoreService.onPlayerLogin loads its cache asynchronously, while this coroutine immediately reads store.selectBySessionKey. A successful login can therefore send Rank.EVERYONE before the account data is cached, with no guaranteed resend. Use the successful account/rank directly or await cache population before serializing playerdata.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FoosClientDetector.kt`
around lines 168 - 177, The sendPlayerData method must not serialize
Rank.EVERYONE merely because the asynchronous cache is not populated yet. Ensure
login synchronization waits for DataStoreService.onPlayerLogin cache population
or obtains the successfully loaded account/rank directly before constructing the
JSON, while preserving the existing rank field for valid cached accounts.

168-183: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Return to the Mindustry thread before accessing Player and sending the packet.

sendPlayerData(...) looks up the stored rank asynchronously, but player.name/player.id and Call.clientPacketReliable(...) are still read and sent from the background scope. Keep the database lookup on the background thread, then post the remaining Player access and Call.clientPacketReliable(...) call to Core.app.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FoosClientDetector.kt`
around lines 168 - 183, Update sendPlayerData so the asynchronous scope performs
only the store.selectBySessionKey rank lookup, then posts construction of the
packet data and Call.clientPacketReliable to Core.app. Ensure player.name,
player.id, and the Player connection are accessed only inside the
Mindustry-thread callback while preserving the existing rank fallback and packet
contents.

96-105: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Catch and report exceptions from the punishment coroutine.

The nested scope.launch { … } completes after the outer packet handler moves on, so failures in users.getByIdentity(...), punishments.punish(...), or the reply(...) callback escape into the shared coroutine scope and only fall back to the unhandled coroutine exception handler. Wrap executePunishment(...) in the launched coroutine and send an explicit failure message via reply(...).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FoosClientDetector.kt`
around lines 96 - 105, Wrap the executePunishment call inside the nested
scope.launch coroutine with exception handling covering lookup, punishment, and
reply failures. On failure, invoke the existing reply callback with an explicit
failure message, while preserving the current punishment arguments and normal
success behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FoosClientDetector.kt`:
- Around line 66-68: Remove the client-supplied `fooCheck` packet as an
authorization boundary in `FoosClientDetector`; do not set `fooClients[player]`
solely from that packet. Replace the trust decision with a server-issued
challenge/token validated before marking a player as a Foo client, or remove the
`fooClients` authorization path and update `FunHandler`’s `teleport`/`navTp`
handling accordingly.

---

Outside diff comments:
In
`@imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FoosClientDetector.kt`:
- Around line 107-110: Update the exception handler around the foosModeration
packet processing to replace the incorrect foosFreeze label in both Log.err and
playerObject.sendMessage with foosModeration, preserving the existing error
handling behavior.
- Around line 168-177: The sendPlayerData method must not serialize
Rank.EVERYONE merely because the asynchronous cache is not populated yet. Ensure
login synchronization waits for DataStoreService.onPlayerLogin cache population
or obtains the successfully loaded account/rank directly before constructing the
JSON, while preserving the existing rank field for valid cached accounts.
- Around line 168-183: Update sendPlayerData so the asynchronous scope performs
only the store.selectBySessionKey rank lookup, then posts construction of the
packet data and Call.clientPacketReliable to Core.app. Ensure player.name,
player.id, and the Player connection are accessed only inside the
Mindustry-thread callback while preserving the existing rank fallback and packet
contents.
- Around line 96-105: Wrap the executePunishment call inside the nested
scope.launch coroutine with exception handling covering lookup, punishment, and
reply failures. On failure, invoke the existing reply callback with an explicit
failure message, while preserving the current punishment arguments and normal
success behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7c61fb6a-73ff-4704-a645-903b03b8e801

📥 Commits

Reviewing files that changed from the base of the PR and between 727882d and 29986ab.

📒 Files selected for processing (4)
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/ImperiumPlugin.kt
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/account/AccountListener.kt
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FoosClientDetector.kt
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/security/ModerationCommand.kt
💤 Files with no reviewable changes (1)
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/ImperiumPlugin.kt
🚧 Files skipped from review as they are similar to previous changes (1)
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/security/ModerationCommand.kt

@phinner

phinner commented Aug 2, 2026

Copy link
Copy Markdown
Member

So... are you waiting for me to merge this or foos first?

@JasonP01

JasonP01 commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

You

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FunHandler.kt (2)

80-90: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Guard against non-finite or negative status durations.

length.toFloatOrNull() parses "NaN", "Infinity", and "-Infinity" as valid Float values, and it also accepts negative numeric strings. A moderator who types length=NaN or a negative value passes a non-finite or negative duration directly to target.unit().apply(statusEffect, time), which can produce an effect that never expires as intended or behaves inconsistently.

Reject non-finite and negative values explicitly, similar to the isFinite() checks already used in decodeTeleportPacket.

Proposed fix
         val time =
             if (length == "infinite") Float.POSITIVE_INFINITY
-            else length.toFloatOrNull() ?: return sender.reply(command_arg_unknown(length))
+            else {
+                val parsed = length.toFloatOrNull() ?: return sender.reply(command_arg_unknown(length))
+                if (!parsed.isFinite() || parsed < 0) return sender.reply(command_arg_unknown(length))
+                parsed
+            }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FunHandler.kt`
around lines 80 - 90, Update onStatusCommand’s duration parsing so the special
"infinite" value remains supported, while parsed numeric durations are accepted
only when finite and non-negative. Reject NaN, infinities, and negative values
through the existing command-argument error path before calling
target.unit().apply(statusEffect, time), following the validation pattern used
by decodeTeleportPacket.

92-107: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Remove the old unit before switching the player’s controlled unit.

tunit is copied into cunit, then target.unit(cunit) only changes the controller; it does not despawn the old unit. If tunit was controlling another player, that player loses their unit from the world instead of switching control. Store the previous controller before switching and restore it on cunit when any old units remain, or remove the old unit if it is no longer needed.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FunHandler.kt`
around lines 92 - 107, Update onChangeUnitCommand to preserve the previous
controller before calling target.unit(cunit), then remove the old tunit from the
world and restore its controller onto cunit when applicable; otherwise remove
tunit as unused. Ensure the new unit is added only after the old unit has been
detached or removed, while retaining the existing state-copy behavior.
🧹 Nitpick comments (1)
imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FunHandler.kt (1)

74-115: 📐 Maintainability & Code Quality | 🔵 Trivial

Consider localizing user-facing reply strings.

Several command replies use hardcoded English strings ("Console must provide a player" at Line 75, "Added ${statusEffect.name} to ${target.plainName()}" at Line 89, "Set ${target.plainName()}'s unit to ${unit.name}" at Line 106, "Open tab list hehe" at Line 115), while other parts of the same commands use localized message functions (e.g. command_arg_unknown(status)). This mix is inconsistent with the bundle-based translation pattern (bundle_en.properties) used elsewhere in the Mindustry moderation cohort.

Moving these strings into the translation bundle would keep the command output consistent with the rest of the plugin's localization approach.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FunHandler.kt`
around lines 74 - 115, Replace the hardcoded user-facing replies in
onTeleportCommand, onStatusCommand, onChangeUnitCommand, and onNameChangeCommand
with the existing bundle-based localization functions. Add corresponding
translation keys and parameterized messages for the player, status effect, and
unit names, while preserving the current reply behavior and unknown-argument
handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FunHandler.kt`:
- Around line 123-126: Update blockIsCore to handle a null result from
Vars.world.tile(x, y) before accessing block() or team(). Return false for
out-of-bounds coordinates while preserving the existing true condition for
matching CoreBlock tiles and teams.

---

Outside diff comments:
In
`@imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FunHandler.kt`:
- Around line 80-90: Update onStatusCommand’s duration parsing so the special
"infinite" value remains supported, while parsed numeric durations are accepted
only when finite and non-negative. Reject NaN, infinities, and negative values
through the existing command-argument error path before calling
target.unit().apply(statusEffect, time), following the validation pattern used
by decodeTeleportPacket.
- Around line 92-107: Update onChangeUnitCommand to preserve the previous
controller before calling target.unit(cunit), then remove the old tunit from the
world and restore its controller onto cunit when applicable; otherwise remove
tunit as unused. Ensure the new unit is added only after the old unit has been
detached or removed, while retaining the existing state-copy behavior.

---

Nitpick comments:
In
`@imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FunHandler.kt`:
- Around line 74-115: Replace the hardcoded user-facing replies in
onTeleportCommand, onStatusCommand, onChangeUnitCommand, and onNameChangeCommand
with the existing bundle-based localization functions. Add corresponding
translation keys and parameterized messages for the player, status effect, and
unit names, while preserving the current reply behavior and unknown-argument
handling.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: fa55e30e-cf8d-4996-b169-d1971da43bf6

📥 Commits

Reviewing files that changed from the base of the PR and between 29986ab and 260cf1d.

📒 Files selected for processing (3)
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/ImperiumPlugin.kt
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FoosClientDetector.kt
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FunHandler.kt
💤 Files with no reviewable changes (1)
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/ImperiumPlugin.kt
🚧 Files skipped from review as they are similar to previous changes (1)
  • imperium-mindustry/src/main/kotlin/com/xpdustry/imperium/mindustry/game/FoosClientDetector.kt

…stry/game/FunHandler.kt

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@phinner
phinner merged commit 9954525 into xpdustry:master Aug 2, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants