Skip to content

PCSX2 injection support for vanilla JP KH1, vanilla USA KH1 and JP ReCoM#1255

Merged
shananas merged 8 commits intoOpenKH:masterfrom
kikeprime:master
Apr 7, 2026
Merged

PCSX2 injection support for vanilla JP KH1, vanilla USA KH1 and JP ReCoM#1255
shananas merged 8 commits intoOpenKH:masterfrom
kikeprime:master

Conversation

@kikeprime
Copy link
Copy Markdown
Contributor

@kikeprime kikeprime commented Mar 4, 2026

I added the necessary changes to the Mod Managers code so the PCSX2 injection feature is compatible with the original Japanese PS2 release of ReCoM.
The 2 offsets had to be shifted with +0x90 while the negative instruction parameters in the 2 hook functions with +0x42C0.
I didn't want to touch the structure of the file so for now I duplicated the 2 hook functions but I propose an offset parameter based on the USA release since that was added first.

Summary by CodeRabbit

  • New Features

    • Expanded game detection to include additional KH1 and Re:CoM product variants.
    • Extended injector support to handle JP and vanilla variants for relevant file-loading flows.
  • Bug Fixes

    • Adjusted ISO extraction logic for Re:Chain of Memories to correct sector sizing during file extraction.
  • UI Updates

    • Setup wizard now lists JP alongside US and Final Mix for supported game selections.

Mentioning the new JP ReCoM support.
Accept JP ReCoM ISO.
Added JP ReCoM support. Some instruction parameters had to be changed in the hook functions so I duplicated them. A more elegant solution is welcome.
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 4, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 697f6a6d-1ee9-49af-b210-bee182823d56

📥 Commits

Reviewing files that changed from the base of the PR and between 724d13e and 7f7482c.

📒 Files selected for processing (1)
  • OpenKh.Tools.ModsManager/Services/GameDataExtractionService.cs

📝 Walkthrough

Walkthrough

Adds JP-region detectors for KH1/Recom, extends PCSX2 injector offsets/hooks with JP and vanilla variants and conditional injection, adjusts Recom ISO sector calculation, and updates SetupWizard supported-game labels.

Changes

Cohort / File(s) Summary
Game Detection
OpenKh.Tools.ModsManager/Services/GameService.cs
Added GameDetectorModel entries for SYSTEM.CNF;1 product IDs: SLPS_251.05;1 and SLUS_203.70;1 (kh1), and SLPM_666.76;1 (Recom).
PCSX2 Injection / Offsets
OpenKh.Tools.ModsManager/Services/Pcsx2Injector.cs
Added new Offsets properties (LoadFileTaskVanilla, LoadFileAsyncJp, GetFileSizeRecomJp), private hook instruction arrays for JP/vanilla variants, conditional injection calls in WritePatch, and a new Offsets entry for SLPM_666.76;1.
ISO extraction logic
OpenKh.Tools.ModsManager/Services/GameDataExtractionService.cs
Changed Recom extraction sector calculation to use ReadInt16() + 1 as the sector/length source when building rdi_stream, altering parsed directory entries.
UI
OpenKh.Tools.ModsManager/Views/SetupWizardWindow.xaml
Updated supported-games labels to include JP for Kingdom Hearts I and Re:Chain of Memories.

Sequence Diagram(s)

(none — changes are metadata, offsets and a small extraction tweak; flow remains localized and not requiring a multi-component sequence diagram.)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • kenjiuno
  • shananas

Poem

🐰 I hopped through bytes and offset streams,
Added JP traces to SYSTEM.CNF dreams.
Hooks now listen in vanilla and JP,
Recom’s sectors shifted — a tidy new way.
Carrots, detectors, and tiny code beams.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding PCSX2 injection support for three specific game variants (vanilla JP KH1, vanilla USA KH1, and JP ReCoM), which is the primary objective of the PR across all modified files.

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

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

Copy link
Copy Markdown

@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.

🧹 Nitpick comments (1)
OpenKh.Tools.ModsManager/Services/Pcsx2Injector.cs (1)

487-546: Consider parameterizing JP/US hook generation to avoid drift.

These two JP arrays are near-duplicates of existing hooks with shifted immediates. A small builder method would keep behavior identical and reduce maintenance risk when hooks evolve.

♻️ Refactor sketch
- private static readonly uint[] LoadFileAsyncHook = new uint[] { ... };
- private static readonly uint[] LoadFileAsyncHookJp = new uint[] { ... };
+ private static uint[] BuildLoadFileAsyncHook(short oA, short oB, short oC, short oD) => new uint[]
+ {
+     LUI(T6, HookStack),
+     LW(T2, V0, oA),
+     LW(T3, V0, oB),
+     ADDI(T4, V0, oC),
+     ...
+     LW(A2, V0, oD),
+     ...
+ };
+ private static readonly uint[] LoadFileAsyncHook = BuildLoadFileAsyncHook(-0x2A10, -0x2A24, -0x29F8, -0x29F4);
+ private static readonly uint[] LoadFileAsyncHookJp = BuildLoadFileAsyncHook(0x18B0, 0x189C, 0x18C8, 0x18CC);

- private static readonly uint[] GetFileSizeRecomHook = new uint[] { ... };
- private static readonly uint[] GetFileSizeRecomHookJp = new uint[] { ... };
+ private static uint[] BuildGetFileSizeRecomHook(short fileDirOffset) => new uint[]
+ {
+     LUI(T6, HookStack),
+     LUI(V1, 0x5C),
+     ADDI(T4, V1, fileDirOffset),
+     ...
+ };
+ private static readonly uint[] GetFileSizeRecomHook = BuildGetFileSizeRecomHook(-0x29F8);
+ private static readonly uint[] GetFileSizeRecomHookJp = BuildGetFileSizeRecomHook(0x18C8);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@OpenKh.Tools.ModsManager/Services/Pcsx2Injector.cs` around lines 487 - 546,
The two JP hook arrays (LoadFileAsyncHookJp and GetFileSizeRecomHookJp) are
near-duplicates of US hooks with only immediate/address shifts; create a small
builder method (e.g., BuildHookSequence or BuildJpHook) that accepts the varying
immediates/offsets (V1 high words like 0x5B/0x5C, the AddI/AddIU immediates such
as 0x18C8/0x18CC, and any differing constant like Operation.GetFileSizeRecom)
and returns the uint[] of instructions using the same instruction helpers (LUI,
LW, SW, BEQ, BNE, ADDI, ADDIU, JR, NOP, etc.); replace the hardcoded arrays
LoadFileAsyncHookJp and GetFileSizeRecomHookJp with calls to that builder
passing the specific constants (maintain use of HookStack,
Param1/Param2/ParamOperator/ParamReturn, and registers T0..T6, V0/V1, S1/S2, RA)
so behavior stays identical while preventing drift when the common hook template
changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@OpenKh.Tools.ModsManager/Services/Pcsx2Injector.cs`:
- Around line 487-546: The two JP hook arrays (LoadFileAsyncHookJp and
GetFileSizeRecomHookJp) are near-duplicates of US hooks with only
immediate/address shifts; create a small builder method (e.g., BuildHookSequence
or BuildJpHook) that accepts the varying immediates/offsets (V1 high words like
0x5B/0x5C, the AddI/AddIU immediates such as 0x18C8/0x18CC, and any differing
constant like Operation.GetFileSizeRecom) and returns the uint[] of instructions
using the same instruction helpers (LUI, LW, SW, BEQ, BNE, ADDI, ADDIU, JR, NOP,
etc.); replace the hardcoded arrays LoadFileAsyncHookJp and
GetFileSizeRecomHookJp with calls to that builder passing the specific constants
(maintain use of HookStack, Param1/Param2/ParamOperator/ParamReturn, and
registers T0..T6, V0/V1, S1/S2, RA) so behavior stays identical while preventing
drift when the common hook template changes.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f06c1840-0840-438b-96d1-5ac5e33bbebf

📥 Commits

Reviewing files that changed from the base of the PR and between c54593e and f64f203.

📒 Files selected for processing (3)
  • OpenKh.Tools.ModsManager/Services/GameService.cs
  • OpenKh.Tools.ModsManager/Services/Pcsx2Injector.cs
  • OpenKh.Tools.ModsManager/Views/SetupWizardWindow.xaml

Both vanilla JP and USA KH1 needs a smaller skip jump than KH1FM. I did the same duplicate method as for JP ReCoM.
Vanilla JP and USA versions of KH1 are now valid KH1 ISOs for the ISO selector.
Copy link
Copy Markdown

@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.

🧹 Nitpick comments (2)
OpenKh.Tools.ModsManager/Services/Pcsx2Injector.cs (2)

938-945: Consider extracting repeated hook-injection boilerplate.

The three new conditional blocks repeat the same Log.Info + WritePatch(... JAL(WriteHook(...)), ADDIU(...)) pattern. A small helper would reduce copy/paste risk as more variants are added.

Also applies to: 955-961, 998-1004

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@OpenKh.Tools.ModsManager/Services/Pcsx2Injector.cs` around lines 938 - 945,
Extract the repeated injection boilerplate into a small helper method (e.g.,
InjectHook) and replace the three repeated blocks with calls to it; the helper
should take the stream, the target offset (like offsets.LoadFileTaskVanilla), a
hook writer/delegate (like LoadFileTaskHookVanilla passed to WriteHook), the
operation enum value (Operation.LoadFileTask) and a name for logging, then
perform the Log.Info and the WritePatch(stream, offset, ADDIU(...),
JAL(WriteHook(...)), ADDIU(...)) sequence using WriteHook and WritePatch so
callers simply call InjectHook(stream, offsets.LoadFileTaskVanilla,
LoadFileTaskHookVanilla, Operation.LoadFileTask,
nameof(offsets.LoadFileTaskVanilla)); update the other occurrences (lines
referencing offsets.* and their respective Hook and Operation) to use this
helper.

427-455: Parameterize LoadFileTask hook construction to avoid code drift.

LoadFileTaskHook and LoadFileTaskHookVanilla are identical except the skip immediate at Line 451 (0x64 vs 0x98 in the other hook). This is a good candidate for a small hook-builder helper.

♻️ Suggested refactor
- private static readonly uint[] LoadFileTaskHook = new uint[]
- {
-   ...
-   ADDIU(RA, RA, 0x98),
-   ...
- };
-
- private static readonly uint[] LoadFileTaskHookVanilla = new uint[]
- {
-   ...
-   ADDIU(RA, RA, 0x64),
-   ...
- };
+ private static uint[] BuildLoadFileTaskHook(short skipImmediate) => new uint[]
+ {
+   LUI(T6, HookStack),
+   SW(S1, T6, Param1),
+   SW(S0, T6, Param2),
+   SW(V0, T6, Param3),
+   SW(T5, T6, ParamOperator),
+   LW(T5, T6, ParamOperator),
+   BNE(T5, (byte)Operation.HookExit, -2),
+   LW(V1, T6, ParamReturn),
+   BEQ(V1, Zero, 3),
+   MOVE(S2, V0),
+   BEQ(Zero, Zero, 2),
+   ADDIU(RA, RA, skipImmediate),
+   LI(V0, -1),
+   JR(RA),
+   NOP(),
+ };
+
+ private static readonly uint[] LoadFileTaskHook = BuildLoadFileTaskHook(0x98);
+ private static readonly uint[] LoadFileTaskHookVanilla = BuildLoadFileTaskHook(0x64);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@OpenKh.Tools.ModsManager/Services/Pcsx2Injector.cs` around lines 427 - 455,
The two hook arrays LoadFileTaskHook and LoadFileTaskHookVanilla differ only by
the skip immediate (ADDIU(RA, RA, 0x64) vs 0x98); refactor by extracting a
hook-builder (e.g., BuildLoadFileTaskHook or CreateLoadFileTaskHook) that
returns the uint[] and takes the skipImmediate as a parameter, then replace both
LoadFileTaskHook and LoadFileTaskHookVanilla with calls to that builder (pass
0x64 and 0x98 respectively); ensure the builder reproduces the exact sequence of
instructions (LUI(T6, HookStack), SW(...), LW(...), BNE(...), LW(V1,...),
BEQ(...), MOVE(...), BEQ(...), ADDIU(RA, RA, skipImmediate), LI(V0, -1), JR(RA),
NOP()) and retains the same identifiers (T6, S1, S0, V0, T5, V1, S2, RA) so all
references remain valid.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@OpenKh.Tools.ModsManager/Services/Pcsx2Injector.cs`:
- Around line 938-945: Extract the repeated injection boilerplate into a small
helper method (e.g., InjectHook) and replace the three repeated blocks with
calls to it; the helper should take the stream, the target offset (like
offsets.LoadFileTaskVanilla), a hook writer/delegate (like
LoadFileTaskHookVanilla passed to WriteHook), the operation enum value
(Operation.LoadFileTask) and a name for logging, then perform the Log.Info and
the WritePatch(stream, offset, ADDIU(...), JAL(WriteHook(...)), ADDIU(...))
sequence using WriteHook and WritePatch so callers simply call
InjectHook(stream, offsets.LoadFileTaskVanilla, LoadFileTaskHookVanilla,
Operation.LoadFileTask, nameof(offsets.LoadFileTaskVanilla)); update the other
occurrences (lines referencing offsets.* and their respective Hook and
Operation) to use this helper.
- Around line 427-455: The two hook arrays LoadFileTaskHook and
LoadFileTaskHookVanilla differ only by the skip immediate (ADDIU(RA, RA, 0x64)
vs 0x98); refactor by extracting a hook-builder (e.g., BuildLoadFileTaskHook or
CreateLoadFileTaskHook) that returns the uint[] and takes the skipImmediate as a
parameter, then replace both LoadFileTaskHook and LoadFileTaskHookVanilla with
calls to that builder (pass 0x64 and 0x98 respectively); ensure the builder
reproduces the exact sequence of instructions (LUI(T6, HookStack), SW(...),
LW(...), BNE(...), LW(V1,...), BEQ(...), MOVE(...), BEQ(...), ADDIU(RA, RA,
skipImmediate), LI(V0, -1), JR(RA), NOP()) and retains the same identifiers (T6,
S1, S0, V0, T5, V1, S2, RA) so all references remain valid.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e5745a92-2026-4e9c-8a8d-77b79dc33084

📥 Commits

Reviewing files that changed from the base of the PR and between f64f203 and d174e2c.

📒 Files selected for processing (1)
  • OpenKh.Tools.ModsManager/Services/Pcsx2Injector.cs

Mention vanilla USA and JP KH1 support.
@kikeprime
Copy link
Copy Markdown
Contributor Author

In the 3 commits I made today I added support for the vanilla USA and JP versions of KH1.
The 0x98 long jump had to be shortened to 0x64 because FM added an additional if check (according to IDA Pro's decomp view) so the vanilla function is shorter. The original jump skipped to much code.
Vanilla JP and USA have the same function just elsewhere so 1 hook function is sufficient for them. I tested both versions just in case.
I used the same duplicate hook function method as for ReCoM for the exact same reason.

@kikeprime kikeprime changed the title JP PS2 ReCoM support for PCSX2 injection PCSX2 injection support for vanilla JP KH1, vanilla USA KH1 and JP ReCoM Mar 6, 2026
@Some1fromthedark
Copy link
Copy Markdown
Contributor

I did some testing with the JP RECOM and JP & US KH1 ISO's.
Here are the results of my tests:

File Extraction Test:

  • JP KH1: Successful ✔️
  • US KH1: Successful ✔️
  • JP RECOM: Failed ❌

File Injection Test:

  • JP KH1: Successful ✔️
  • US KH1: Successful ✔️
  • JP RECOM: Successful ✔️

File Fallback Test:

  • JP KH1: Successful ✔️
  • US KH1: Successful ✔️
  • JP RECOM: Successful ✔️

Everything seemed to work except that I was unable to extract the JP RECOM. If you could address that issue, I will give my approval.

@Some1fromthedark
Copy link
Copy Markdown
Contributor

Some1fromthedark commented Apr 7, 2026

I found what causes the issue with the extraction. There is an off by one error when determining the size of the memory buffer using the sector count for the JP version. This can be resolved by changing line 93 of OpenKh.Tools.ModsManager/Services/GameDataExtractionService.cs to

var rdi_stream = IsoUtility.GetSectors(stream, 0x244, stream.SetPosition(0x244 * 0x800 + 8).ReadInt16() + 1);

I tested with the USA ROM and it doesn't seem to break it but I only cancelled the extraction after a short while. some1fromthedark's fix.
@kikeprime
Copy link
Copy Markdown
Contributor Author

I added your fix and it doesn't seem to break USA extraction.

Copy link
Copy Markdown

@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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@OpenKh.Tools.ModsManager/Services/GameDataExtractionService.cs`:
- Line 93: The code reading the RDI sector count uses a single-byte read at the
sector start; update the call that creates rdi_stream (the IsoUtility.GetSectors
call) to read the 16-bit sector-count field at offset +0x8 from the sector
start, add 1 to that 16-bit value, and pass that resulting count to GetSectors.
Locate the expression that uses stream.SetPosition(0x244 * 0x800).ReadByte() and
replace it with logic that seeks to (0x244 * 0x800) + 0x8, reads a 16-bit
unsigned integer (proper endianness as used elsewhere in this project), adds 1,
and uses that value for the third parameter to IsoUtility.GetSectors so
rdi_stream is sized correctly.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 838a5ec2-3ff1-4c8b-b506-f7ea74523343

📥 Commits

Reviewing files that changed from the base of the PR and between d174e2c and 724d13e.

📒 Files selected for processing (3)
  • OpenKh.Tools.ModsManager/Services/GameDataExtractionService.cs
  • OpenKh.Tools.ModsManager/Services/GameService.cs
  • OpenKh.Tools.ModsManager/Views/SetupWizardWindow.xaml
✅ Files skipped from review due to trivial changes (1)
  • OpenKh.Tools.ModsManager/Views/SetupWizardWindow.xaml
🚧 Files skipped from review as they are similar to previous changes (1)
  • OpenKh.Tools.ModsManager/Services/GameService.cs

Sorry, that was on me for not copying the line. Weirdly enough that worked too. This works with both ROMs.
Copy link
Copy Markdown
Contributor

@Some1fromthedark Some1fromthedark left a comment

Choose a reason for hiding this comment

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

All of the tests I ran in my previous comment are now passing. LGTM.

@shananas shananas merged commit 96a032b into OpenKH:master Apr 7, 2026
4 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.

3 participants