Fix memory usage and chunkcache#18
Conversation
Removed deployment step to Testers Repo from workflow.
|
@UplandJacob If you could test the |
Still seems to be a bit crazy @Trainboy15
maybe resetting cache more often or not saving so many things to it? |
|
I agree, it prob shouldent use 24GB, but it seems at least a bit better than 36GB |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
@bobhenl If you could test it now that would be great |
|
where can be the build downloaded btw, I don't see it in actions |
btw is it checking just eagler players? https://spark.lucko.me/pvGBrzMdbI
https://spark.lucko.me/CfTf6fEcPk
|
|
Not sure 100%. will look into it |
This reverts commit 1ca2cda.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThis PR adds test infrastructure, improves plugin robustness through null-safety checks and lifecycle cleanup, optimizes chunk processing with fastutil collections and refactored payload serialization, and introduces a test suite using MockBukkit to validate core plugin behaviors. ChangesTesting Infrastructure and Plugin Improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 @.github/workflows/test.yml:
- Around line 15-16: The checkout step currently uses actions/checkout@v4
without disabling token persistence; update the Checkout repository step
(actions/checkout) to set persist-credentials: false so the GitHub token is not
written to local git config during this job. Locate the step that uses
actions/checkout@v4 and add the persist-credentials: false option under that
step's with: block.
- Around line 15-16: Replace floating version tags for GitHub Actions with full
commit SHAs: update each uses: entry (e.g., uses: actions/checkout@v4 and the
other uses: lines flagged at 19 and 33) to reference the corresponding action's
full commit SHA (for example actions/checkout@<full-sha>) by looking up the
canonical commit for the tagged release in the action's GitHub repo and
substituting the tag with that SHA so the workflow is pinned to an immutable
commit.
In `@src/main/java/tf/tuff/TuffX.java`:
- Around line 103-105: TuffX currently only calls
PacketEvents.getAPI().terminate() and unregisters incoming channels via
getServer().getMessenger().unregisterIncomingPluginChannel(this); update
TuffX.shutdown/disable logic to explicitly unregister any plugin channels that
other classes register by calling
getServer().getMessenger().unregisterOutgoingPluginChannel(...) and
getServer().getMessenger().unregisterIncomingPluginChannel(...) for the exact
channel names used in Y0Plugin (the channels registered in Y0Plugin.register...
at the Y0Plugin class) and in TuffActions (the channels registered in
TuffActions at its register calls), ensuring every
registerOutgoing/registerIncoming has a matching unregister to prevent
“registration already exists” on reload.
In `@src/test/TuffXTest.java`:
- Around line 31-65: Tests reference plugin.latestAvailableVersion but TuffX has
no such member, causing compile failures; add a package-visible String field
named latestAvailableVersion to class TuffX (initialize to null) so tests can
read/write it, and update any existing update-check logic (e.g., the method that
detects versions) to assign to this field when an available version is found;
alternatively, expose equivalent package-visible getter/setter on TuffX with the
name latestAvailableVersion so the tests can access the value.
- Around line 1-10: The test class TuffXTest is outside Gradle’s default test
source set; move the file TuffXTest (which declares package tf.tuff) into the
standard test directory so its path matches the package
(src/test/java/tf/tuff/TuffXTest.java) or alternatively update Gradle’s
sourceSets configuration to include its current location; ensure the package
declaration remains tf.tuff and that the class name TuffXTest and imports are
unchanged so JUnit (test { useJUnitPlatform() }) will run it in CI.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d161060d-7acd-4b1c-9979-16b6cd9cd31d
⛔ Files ignored due to path filters (1)
builds/TuffXPlus-1.0.1-beta.jaris excluded by!**/*.jar
📒 Files selected for processing (6)
.github/workflows/test.ymlbuild.gradlesrc/main/java/tf/tuff/TuffX.javasrc/main/java/tf/tuff/viablocks/CustomBlockListener.javasrc/main/java/tf/tuff/y0/Y0Plugin.javasrc/test/TuffXTest.java
| - name: Checkout repository | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
Disable credential persistence on checkout.
This job only builds, tests, and uploads artifacts, so keeping the GitHub token in the local git config is unnecessary. Set persist-credentials: false on actions/checkout to narrow the workflow’s credential exposure.
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 15-16: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 16-16: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/test.yml around lines 15 - 16, The checkout step currently
uses actions/checkout@v4 without disabling token persistence; update the
Checkout repository step (actions/checkout) to set persist-credentials: false so
the GitHub token is not written to local git config during this job. Locate the
step that uses actions/checkout@v4 and add the persist-credentials: false option
under that step's with: block.
Pin every GitHub Action to a full commit SHA.
These uses: entries are still floating on major tags, so the workflow can pick up upstream changes without a PR here. That weakens the CI trust boundary and also matches the current static-analysis failure.
Also applies to: 19-19, 33-33
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 15-16: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 16-16: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/test.yml around lines 15 - 16, Replace floating version
tags for GitHub Actions with full commit SHAs: update each uses: entry (e.g.,
uses: actions/checkout@v4 and the other uses: lines flagged at 19 and 33) to
reference the corresponding action's full commit SHA (for example
actions/checkout@<full-sha>) by looking up the canonical commit for the tagged
release in the action's GitHub repo and substituting the tag with that SHA so
the workflow is pinned to an immutable commit.
| PacketEvents.getAPI().terminate(); | ||
|
|
||
| getServer().getMessenger().unregisterIncomingPluginChannel(this); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Plugin message registrations:"
rg -n --type=java 'register(Incoming|Outgoing)PluginChannel|unregister(Incoming|Outgoing)PluginChannel' src
echo
echo "Expected: every registerOutgoingPluginChannel(...) has a matching unregisterOutgoingPluginChannel(...) on the disable path."Repository: TuffNetwork/TuffXPlus
Length of output: 2239
Fix plugin-channel teardown: TuffX only unregisters incoming; missing outgoing/incoming unregisters are in Y0Plugin and TuffActions.
src/main/java/tf/tuff/TuffX.java:105calls onlygetServer().getMessenger().unregisterIncomingPluginChannel(this)andTuffXhas noregisterOutgoingPluginChannel(...)usage, so “mirror outgoing cleanup here” doesn’t apply.- Ensure matching
unregisterOutgoingPluginChannel(...)/unregisterIncomingPluginChannel(...)for the channels registered in:src/main/java/tf/tuff/y0/Y0Plugin.java:232-233src/main/java/tf/tuff/tuffactions/TuffActions.java:68-69(missing any corresponding unregisters can trigger “registration already exists” on reload).
🤖 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 `@src/main/java/tf/tuff/TuffX.java` around lines 103 - 105, TuffX currently
only calls PacketEvents.getAPI().terminate() and unregisters incoming channels
via getServer().getMessenger().unregisterIncomingPluginChannel(this); update
TuffX.shutdown/disable logic to explicitly unregister any plugin channels that
other classes register by calling
getServer().getMessenger().unregisterOutgoingPluginChannel(...) and
getServer().getMessenger().unregisterIncomingPluginChannel(...) for the exact
channel names used in Y0Plugin (the channels registered in Y0Plugin.register...
at the Y0Plugin class) and in TuffActions (the channels registered in
TuffActions at its register calls), ensuring every
registerOutgoing/registerIncoming has a matching unregister to prevent
“registration already exists” on reload.
| package tf.tuff; | ||
|
|
||
| import be.seeseemelk.mockbukkit.MockBukkit; | ||
| import be.seeseemelk.mockbukkit.ServerMock; | ||
| import be.seeseemelk.mockbukkit.entity.PlayerMock; | ||
| import org.junit.jupiter.api.*; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.*; | ||
|
|
||
| class TuffXTest { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Test file location:"
fd -p 'TuffXTest.java'
echo
echo "Gradle source-set configuration:"
rg -n 'sourceSets|useJUnitPlatform|test \{' build.gradle settings.gradle gradle.properties 2>/dev/null || true
echo
echo "Expected: either the test file lives under src/test/java/... or build.gradle defines a custom test source set that includes src/test."Repository: TuffNetwork/TuffXPlus
Length of output: 340
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== build.gradle lines 70-120 =="
sed -n '70,120p' build.gradle
echo
echo "== Any sourceSets configuration in repo (limited to build.gradle/gradle/*.gradle) =="
rg -n "sourceSets" -S . || true
echo
echo "== List of Java test files under src/test =="
fd -e java . src/test 2>/dev/null | head -n 200Repository: TuffNetwork/TuffXPlus
Length of output: 696
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Test file location:"
fd -p 'TuffXTest.java'
echo
echo "Gradle source-set configuration:"
rg -n 'sourceSets|sourceDirs|test \{' build.gradle settings.gradle gradle.properties 2>/dev/null || true
echo
echo "Expected: either the test file lives under src/test/java/... or build.gradle defines a custom test source set/sourceDirs including src/test."Repository: TuffNetwork/TuffXPlus
Length of output: 308
Move TuffXTest into Gradle’s active test source set (src/test/java).
src/test/TuffXTest.java is not under src/test/java, and build.gradle only configures test { useJUnitPlatform() } with no sourceSets override—so this test can be skipped entirely in CI. Also, since it declares package tf.tuff;, it should be located at src/test/java/tf/tuff/TuffXTest.java.
🤖 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 `@src/test/TuffXTest.java` around lines 1 - 10, The test class TuffXTest is
outside Gradle’s default test source set; move the file TuffXTest (which
declares package tf.tuff) into the standard test directory so its path matches
the package (src/test/java/tf/tuff/TuffXTest.java) or alternatively update
Gradle’s sourceSets configuration to include its current location; ensure the
package declaration remains tf.tuff and that the class name TuffXTest and
imports are unchanged so JUnit (test { useJUnitPlatform() }) will run it in CI.
3 participants
IDK if it works put it compiled so...
Also u might want to add back the deploy to testers repo, but i had to remove it
Summary by CodeRabbit
New Features
Bug Fixes
Tests