Skip to content

Comments

WorldPoints inside PoH changed (more than I realized)#1503

Merged
chsami merged 2 commits intochsami:developmentfrom
Krulvis:development
Sep 11, 2025
Merged

WorldPoints inside PoH changed (more than I realized)#1503
chsami merged 2 commits intochsami:developmentfrom
Krulvis:development

Conversation

@Krulvis
Copy link
Contributor

@Krulvis Krulvis commented Sep 10, 2025

All house styles have their own level now

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 10, 2025

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The HouseStyle enum mapping to WorldPoint values was updated. Z-levels changed for BASIC_STONE (to z=1), WHITEWASHED_STONE (z=2), FREMENNIK_STYLE_WOOD (z=3), TROPICAL_WOOD (to z=0), FANCY_STONE (z=1), TWISTED_THEME (z=3), CIVITAS (z=2), and CANIFIS (z=3). A new enum constant UNKNOWN(12, WorldPoint(1986, 5707, 1)) was added. BASIC_WOOD and DEADLY_MANSION remain unchanged. The enum order reflects the insertion of UNKNOWN. No other files were modified.

Pre-merge checks (2 passed, 1 warning)

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The current title is overly informal and includes a parenthetical aside that doesn't contribute meaningful information about the change. It fails to clearly convey that the PR adjusts z-level mappings for multiple HouseStyle constants and adds a new UNKNOWN enum value. As a result, it may not be immediately clear to team members what the primary update entails. Rename the pull request to a concise title that directly summarizes the core changes, for example “Adjust HouseStyle z-level mappings and add UNKNOWN enum constant,” so that reviewers can immediately grasp the intent.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed The description accurately reflects that each house style now has its own level, which aligns with the changes to the z-coordinate values in the HouseStyle enum. Although concise, it stays on topic and relates directly to the modifications in the pull request.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

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 and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8d2dedf and 590a340.

⛔ Files ignored due to path filters (4)
  • runelite-client/src/main/resources/net/runelite/client/plugins/microbot/shortestpath/teleportation_items.tsv is excluded by !**/*.tsv
  • runelite-client/src/main/resources/net/runelite/client/plugins/microbot/shortestpath/teleportation_poh.tsv is excluded by !**/*.tsv
  • runelite-client/src/main/resources/net/runelite/client/plugins/microbot/shortestpath/teleportation_portal_poh.tsv is excluded by !**/*.tsv
  • runelite-client/src/main/resources/net/runelite/client/plugins/microbot/shortestpath/teleportation_spells.tsv is excluded by !**/*.tsv
📒 Files selected for processing (1)
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/poh/data/HouseStyle.java (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Build
🔇 Additional comments (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/poh/data/HouseStyle.java (1)

15-21: Z-level remap for styles 1–8 looks consistent.

Planes now follow a clear 0..3 pattern per anchor (1858/1922). No issues spotted with these constants.

Comment on lines +24 to +26
UNKNOWN(12, new WorldPoint(1986, 5707, 1)),
CIVITAS(13, new WorldPoint(1986, 5707, 2)),
CANIFIS(14, new WorldPoint(1986, 5707, 3)),
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Handle unknown varbit values defensively (avoid null from getStyle).

Adding UNKNOWN(12, …) is good, but any unlisted varbit (e.g., 9 or future additions) still yields null → likely NPEs downstream. Prefer failing fast (or returning Optional) instead of returning null.

Apply this diff to fail fast with context:

@@
     public static HouseStyle getStyle() {
         int varbitValue = Microbot.getVarbitValue(VarbitID.POH_HOUSE_STYLE);
         for (HouseStyle style : values()) {
             if (varbitValue == style.varbitValue) {
                 return style;
             }
         }
-        return null;
+        throw new IllegalStateException("Unknown POH house style varbit: " + varbitValue);
     }

Run these to check for risky usages and missing mappings:


🏁 Script executed:

#!/bin/bash
# Call sites that may assume non-null:
rg -nP -C2 'HouseStyle\.getStyle\(\)'

# Any ordinal-based usages (fragile if enum order changed):
rg -nP -C2 '\.ordinal\(\)'

# References to varbit 9 (if it exists in-game and needs mapping):
rg -nP -C2 '\b9\b' runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/poh/**/*.java

Length of output: 37660


Fail-fast on unknown POH house styles
Change getStyle() to throw rather than return null to eliminate silent NPEs downstream:

@@ public static HouseStyle getStyle() {
-    return null;
+    throw new IllegalStateException("Unknown POH house style varbit: " + varbitValue);
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
UNKNOWN(12, new WorldPoint(1986, 5707, 1)),
CIVITAS(13, new WorldPoint(1986, 5707, 2)),
CANIFIS(14, new WorldPoint(1986, 5707, 3)),
public static HouseStyle getStyle()
{
int varbitValue = /* existing varbit‐reading logic */;
for (HouseStyle style : values())
{
if (style.getVarbitValue() == varbitValue)
{
return style;
}
}
throw new IllegalStateException("Unknown POH house style varbit: " + varbitValue);
}
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/poh/data/HouseStyle.java
around lines 24-26, the enum lookup method getStyle(int) currently returns null
for unknown IDs which causes silent NPEs downstream; change it to throw an
IllegalArgumentException (or a custom unchecked exception) when no matching
HouseStyle is found, include the unknown id in the message for diagnostics,
update the method signature to return HouseStyle (non-null) and ensure any
existing callers are adjusted to handle or allow the unchecked exception.

@chsami chsami merged commit c9b477a into chsami:development Sep 11, 2025
2 checks passed
This was referenced Sep 11, 2025
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