fix: harden ground item action reflection#1807
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughGround-item action handling was updated across reflection and interaction paths. Possibly related PRs
🚥 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. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/api/tileitem/models/Rs2TileItemModel.java`:
- Around line 227-239: The default click path in Rs2TileItemModel currently
assumes the first ground action is at index 0, so click() can fail when
groundActions has leading null entries. Update the click("") handling to resolve
the first non-null usable action by scanning the array (matching the same lookup
behavior used for explicit actions) instead of hard-coding groundActions[0], and
keep the existing index tracking so the selected action is applied correctly.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/reflection/Rs2Reflection.java`:
- Around line 88-95: The cache-first path in Rs2Reflection still reuses cached
fields before verifying the inspected class, so a class switch can return null
or empty results without triggering rediscovery. Update the cache keying logic
around extractWithCache and the ground item action lookup to compare the current
item/class against the cached class before using cachedOuterField and
cachedListField, and force resetGroundItemActionCache plus re-discovery whenever
the inspected class changes rather than waiting for reflection to throw.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c9d3d9e4-e72c-4867-9883-192b9a87e685
📒 Files selected for processing (6)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/api/tileitem/models/Rs2TileItemModel.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/grounditem/Rs2GroundItem.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/reflection/Rs2Reflection.javarunelite-client/src/test/java/groundactionfixture/GroundItemActionFixture.javarunelite-client/src/test/java/net/runelite/client/plugins/microbot/util/grounditem/GroundItemMenuActionResolutionTest.javarunelite-client/src/test/java/net/runelite/client/plugins/microbot/util/reflection/Rs2ReflectionGroundItemActionsTest.java
|
Addressed the CodeRabbit ground item findings in the latest push.
|
Summary
Defensive hardening for Microbot ground item action reflection and action resolution.
This PR does not claim that normal ground item pickup is broadly broken upstream. The intent is narrower: avoid brittle reflection/action-resolution behavior when client internals include misleading static/synthetic fields or when the requested ground action cannot be resolved cleanly.
Changes
Listimplementations rather than only concreteArrayListfields.Listimplementations, cache behavior, and unresolved action handling.Validation
git diff --check upstream/development...HEAD./gradlew.bat :client:compileJava --console=plain