Skip to content

Let GetTextureDimensions accept second argument#133

Open
Hdt80bro wants to merge 2 commits intomasterfrom
fix-GetTextureDimensions
Open

Let GetTextureDimensions accept second argument#133
Hdt80bro wants to merge 2 commits intomasterfrom
fix-GetTextureDimensions

Conversation

@Hdt80bro
Copy link
Copy Markdown
Collaborator

@Hdt80bro Hdt80bro commented Feb 15, 2026

GetTextureDimensions documents an optional second argument called border that adds the number to both dimensions all around. This is supremely unhelpful, but it's low-hanging fruit.

Addresses #12

Summary by CodeRabbit

  • Chores
    • Updated internal texture dimension handling to support additional parameters, improving system flexibility for texture processing operations.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Feb 15, 2026

📝 Walkthrough

Walkthrough

A new hook snippet is added to the GetTextureDimensions hook file, introducing an address label at 0x00780B29 with a comparison instruction that modifies the function signature from accepting one argument to accepting two arguments.

Changes

Cohort / File(s) Summary
Hook Addition
hooks/GetTextureDimensions.hook
New hook snippet at offset +0x39 with address label 0x00780B29 and comparison instruction, updating function signature to accept 2 arguments instead of 1.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A texture hook now takes two,
Where once there was but one in view,
At 0x780B29, we align,
Two arguments make this compare divine! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description is missing most required sections from the template, including guides checklist, variable naming verification, data structure updates, README changes, and test instructions. Add sections for guide completion, variable naming, data structure updates (moho.h/global.h/Info.txt), README documentation, and test instructions as specified in the template.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: enabling GetTextureDimensions to accept a second argument for the border parameter.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into master

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-GetTextureDimensions

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
hooks/GetTextureDimensions.hook (1)

2-4: The patch logic looks correct; consider a slightly more explicit comment.

The hook correctly bumps the argument-count check from 2 to 3 (allowing the new optional border parameter). The inline comment explains the effect well, but since the commit history already shows confusion around why the literal is 3 for 2 user-facing args, a brief note about the off-by-one (e.g., "arg count includes the implicit self/texture reference" or "less-than comparison: eax < 3 permits 0, 1, or 2 args") would make this self-documenting for future readers.

📝 Suggested comment improvement
 # cfunc_GetTextureDimensions+0x39
 0x00780B29:
-    cmp     eax, 3 # from only accepting 1 arg to 2 (it's used in a less-than operation)
+    cmp     eax, 3 # accept up to 2 args (was 1); eax < 3 because the count includes the implicit first argument

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.

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.

1 participant