Skip to content

feat(libSceNgs2): implement the Geom reset functions - #3995

Merged
oneandonlydean merged 1 commit into
boykopovar:mainfrom
delarun:fix/ngs2-dead-cells
Oct 12, 2026
Merged

oneandonlydean merged 1 commit into
boykopovar:mainfrom
delarun:fix/ngs2-dead-cells

Conversation

@delarun

@delarun delarun commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

What

Implements sceNgs2GeomResetListenerParam and sceNgs2GeomResetSourceParam, taken from #1021 and limited to the two reset functions, as its closing comment suggested. The Geom structures get their real layouts. sceNgs2GeomCalcListener and sceNgs2GeomApply stay unimplemented.

The reset defaults are recorded in docs/dev/TechnicalDebt.md.

Tested

Linux (CachyOS), Dead Cells (PPSA15552) with #3793 on top: it used to abort with sceNgs2GeomResetSourceParam not implemented (reported in #1021) and now gets past that call. It stops later on an unrelated NGS2 patch, left for a separate PR. tools/check_conventions.py passes.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt
  • No notes, investigation .md files or images added to the repository
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • AI-assisted: yes

@MarcZX10

Copy link
Copy Markdown
Contributor

The conventions job currently fails because the title is not Conventional Commits (Fix/ngs2 dead cells). A title such as fix(libSceNgs2): implement geometry reset calls should satisfy the title rule.

@github-actions github-actions Bot added build docs prx needs rebase Conflicts with main, rebase needed labels Oct 11, 2026
@delarun delarun changed the title Fix/ngs2 dead cells fix(libSceNgs2): implement geometry reset calls Oct 11, 2026
@delarun

delarun commented Oct 11, 2026

Copy link
Copy Markdown
Contributor Author

The conventions job currently fails because the title is not Conventional Commits (Fix/ngs2 dead cells). A title such as fix(libSceNgs2): implement geometry reset calls should satisfy the title rule.

Thanks, changed it
also rebased to main

@TideOPL

TideOPL commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

Thanks for picking up the two Geom resets from #1021. A few notes:

  • Conventions check: it fails only because of the PR title (Pull request title is not Conventional Commits: Fix/ngs2 dead cells). Something like feat(libSceNgs2): implement the Geom reset functions would pass.
  • Rack handle mapped to its voice: the sceNgs2VoiceControl patch change, where a rack handle with exactly one voice is taken to mean that voice, is driven by one title. It's recorded in TechnicalDebt, which is good, but a title that passes the wrong handle now patches silently instead of failing. Unless there's evidence that rack and voice handles coincide on the console, I'd keep the much better error message and leave the alias out of this PR. It's also a second topic, so it could be a PR of its own.
  • Minor: Ngs2FindRack(patch.dest_handle) is called twice, and the second rack shadows the first.
  • Minor: the TechnicalDebt line lists "pitch ratio 1" among the reset defaults, but neither reset function touches Ngs2GeomAttribute.

Tested by reviewer
Windows 11 (26200), WinLibs GCC 15.2.0 r7, Release, PR head 384c3c9 merged onto main c5da823. All 9 ngs2_* tests pass, including the new ngs2_geom. Not run: Dead Cells (I don't have it), Linux.

AI-assisted: yes. Written with AI assistance (Claude Code), with the tests run on my machine; I reviewed it.

@delarun delarun changed the title fix(libSceNgs2): implement geometry reset calls feat(libSceNgs2): implement the Geom reset functions Oct 11, 2026
@delarun

delarun commented Oct 11, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for picking up the two Geom resets from #1021. A few notes:

  • Conventions check: it fails only because of the PR title (Pull request title is not Conventional Commits: Fix/ngs2 dead cells). Something like feat(libSceNgs2): implement the Geom reset functions would pass.
  • Rack handle mapped to its voice: the sceNgs2VoiceControl patch change, where a rack handle with exactly one voice is taken to mean that voice, is driven by one title. It's recorded in TechnicalDebt, which is good, but a title that passes the wrong handle now patches silently instead of failing. Unless there's evidence that rack and voice handles coincide on the console, I'd keep the much better error message and leave the alias out of this PR. It's also a second topic, so it could be a PR of its own.
  • Minor: Ngs2FindRack(patch.dest_handle) is called twice, and the second rack shadows the first.
  • Minor: the TechnicalDebt line lists "pitch ratio 1" among the reset defaults, but neither reset function touches Ngs2GeomAttribute.

Tested by reviewer Windows 11 (26200), WinLibs GCC 15.2.0 r7, Release, PR head 384c3c9 merged onto main c5da823. All 9 ngs2_* tests pass, including the new ngs2_geom. Not run: Dead Cells (I don't have it), Linux.

AI-assisted: yes. Written with AI assistance (Claude Code), with the tests run on my machine; I reviewed it.

Thanks for the review and for running the tests on Windows.

  • Title: changed to feat(libSceNgs2): implement the Geom reset functions.
  • Rack handle alias: removed. You're right that I only have one title's behaviour and no evidence that rack and voice handles coincide on the console. The more detailed error stays, so Dead Cells now stops there with patch from rack 0x4001 port 0 to rack 0x3000 with 1 voices. If I find evidence I'll open a separate PR for it.
  • Double Ngs2FindRack call and the shadowed rack: gone with the alias.
  • TechnicalDebt: dropped "pitch ratio 1" from the reset defaults and removed the alias entry.

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 11, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

Please rebase onto main instead of merging it (d7f1989 is a merge commit) and drop the sceNgs2VoiceControl message change in Voice.cpp: it is a second topic and it conflicts with #3793. The two resets don't need #3793, so Depends on can be none.

Implement sceNgs2GeomResetListenerParam and sceNgs2GeomResetSourceParam,
taken from boykopovar#1021 and limited to the two reset functions. The Geom
structures get their real layouts. sceNgs2GeomCalcListener and
sceNgs2GeomApply stay unimplemented.

Co-Authored-By: Dextroid17 <217507631+Dextroid17@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@delarun
delarun force-pushed the fix/ngs2-dead-cells branch from c298fee to cac398c Compare October 12, 2026 11:43
@delarun

delarun commented Oct 12, 2026

Copy link
Copy Markdown
Contributor Author

Please rebase onto main instead of merging it (d7f1989 is a merge commit) and drop the sceNgs2VoiceControl message change in Voice.cpp: it is a second topic and it conflicts with #3793. The two resets don't need #3793, so Depends on can be none.

Done: rebased onto current main as a single commit, no merge commits. The sceNgs2VoiceControl patch message change is dropped from this PR; I'll bring it up separately once #3793 has landed. Depends on is now none, and the description is updated.

@oneandonlydean oneandonlydean self-assigned this Oct 12, 2026
@oneandonlydean
oneandonlydean merged commit 8fa5cc4 into boykopovar:main Oct 12, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants