Skip to content

fix(libScePlayGo): report missing chunk metadata - #5494

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
MarcZX10:fix/playgo-assume-unlisted-incomplete-dump
Oct 14, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
MarcZX10:fix/playgo-assume-unlisted-incomplete-dump

Conversation

@MarcZX10

@MarcZX10 MarcZX10 commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

What

When a package has neither playgo-chunkdefs.xml, sce_sys/playgo-chunk.dat, nor sce_sys/playgo-scenario.json, the dump cannot establish whether nonzero chunk IDs exist. Keep chunk 0 available, but throw a diagnostic naming the missing metadata when a title queries another ID. Complete package tables still return SCE_PLAYGO_ERROR_BAD_CHUNK_ID for IDs outside the table. Update the technical-debt note and regression coverage for both paths.

This is related to #5487, but it does not make the incomplete High on Life dump load successfully. It gives a clear stop instead of guessing that undocumented chunks are installed.

Tested

  • Linux x86-64, GCC 12.2: compiled the real Export.cpp and GuestPlayGo.cpp with a temporary ResolvePath_nid_no_patch harness and ran guest_playgo_tests successfully.
  • python3 tools/check_conventions.py --base upstream/main passed.
  • git diff --check passed.
  • Full project configure remains unavailable in this environment because SDL2 development files are absent (SDL2Config.cmake).

Checklist

  • Based on current main; no duplicate implementation identified
  • One topic per PR
  • No unrelated files or generated artifacts added
  • Generic behavior, not specific to one title
  • AI-assisted: yes
  • Depends on: none

@CnRJay CnRJay left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The description is clear about the trade-off, but this is the kind of change upstream has turned down. An environment switch that makes PlayGo answer "installed" for chunk IDs the package doesn't describe is an approximation: with it on, the answers no longer correspond to the package, and titles that enumerate until the first rejected ID never stop. Reviewers have asked for approximations, even opt-in or logged ones, to stay on testing branches, and for upstream to either be exact or throw (see the CMASK discussion on #1237).

The underlying case is a dump with no playgo-* files, which means the dump is incomplete rather than the title being unusual. Two exact alternatives:

  • make the missing metadata a clear stop (RejectUnreadablePlayGoFiles is already there) with a message telling the user the dump lacks sce_sys/playgo-chunk.dat;
  • or derive the chunk list from what the package actually contains, if the chunk layout can be recovered from the pkg or the app0 file layout.

Users who need the switch for a particular broken dump can carry it on a fork branch.

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Thanks for the review. The associated #5487 report now has a title-side A/B: with the opt-in, High on Life asks scePlayGoGetLocus for IDs 0–999 and stops itself; without it, IDs ≥1 return BAD_CHUNK_ID and block the save load. That is evidence for this opt-in workaround, not for console semantics. I agree the opt-in still lets an enumeration-until-error title run past its normal stop, which is why the default remains strict and the behavior is prominently documented. If the project policy excludes even explicit user-selected compatibility workarounds from upstream, I can rework this as a clear missing-metadata error instead; I wanted to make sure the High on Life evidence is considered before choosing between those outcomes.

@AlexOfTheForest

Copy link
Copy Markdown
Contributor

Scope matches #5487: the variable is only read when there's no playgo-chunk.dat, no package chunk table and no playgo-scenario.json, and the default stays strict.

One thing: other APS5_* switches in core/libs/prx are enabled just by being set (getenv(...) != nullptr, e.g. APS5_NO_TIMELINE, APS5_COARSE_TIMED_WAITS). This one needs exactly "1", so =true or =yes is silently ignored and the user still gets the save error. Either follow the presence convention or reject values other than 0/1 with an error, so a typo doesn't look like the setting doing nothing.

Also, with the setting on, scePlayGoGetLocus accepts every id while scePlayGoGetChunkId still returns only chunk 0. USAGE.md mentions it; a line in the debt entry would help too, since a title that cross-checks both will see them disagree.

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Thanks for the review. I pushed 36a4e690a: the setting now accepts only 0 or 1; another explicitly set value throws an error naming the required values at the first relevant PlayGo query. The regression covers the unset default, explicit 0, permissive 1, and rejects both true and yes. The existing test also confirms that scePlayGoGetChunkId lists only chunk 0 under the opt-in; the technical-debt entry already documents that distinction.\n\nOn Linux, I compiled the real Export.cpp and GuestPlayGo.cpp and ran the focused test executable successfully using a temporary stub only for path resolution. check_conventions.py and git diff --check pass. Full CMake configuration was unavailable because SDL2 development files are absent in this environment.

@MarcZX10 MarcZX10 changed the title fix(libScePlayGo): add opt-in fallback for unknown chunks fix(libScePlayGo): report missing chunk metadata Oct 14, 2026
@MarcZX10
MarcZX10 force-pushed the fix/playgo-assume-unlisted-incomplete-dump branch from 36a4e69 to a44689c Compare October 14, 2026 12:33
@MarcZX10

Copy link
Copy Markdown
Contributor Author

Thanks for spelling out the upstream constraint. I removed the approximation entirely. With no chunk table or scenario metadata, chunk 0 remains available and a query for any other ID now throws a diagnostic naming both missing metadata files and the queried ID; a complete chunk table still returns BAD_CHUNK_ID for an ID outside that table. I added regression coverage for both cases. The focused Linux GuestPlayGo test executable and convention checks pass; full CMake configure remains unavailable here because SDL2 development files are missing.

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request Conflicting files (lines on main) Same exports Same new files
#6033 docs/dev/debt/libScePlayGo.md L5

@adriadam10 adriadam10 self-assigned this Oct 14, 2026
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked the diff, guest_playgo (passes with the change, fails without it) and the partial ctest (33 tests) on Linux; the debt note matches the code. Merging.

@Tamalero

Copy link
Copy Markdown
Contributor

@adriadam10 @MarcZX100, before this merges: the current head (a44689c48) no longer has the opt-in. It now throws when a dump without PlayGo metadata asks about a nonzero chunk. That turns two known titles from "running" into "aborting at startup":

I haven't run this head on High on Life; the above is from the diff and the call pattern logged on #5487. If a clear stop is the goal, I'd keep BAD_CHUNK_ID as the default answer (it is what #4491 settled on for Sifu) and log the missing metadata once, rather than throw. The earlier opt-in (APS5_PLAYGO_ASSUME_UNLISTED_CHUNKS_INSTALLED) worked for High on Life, as reported on #5487.

Tested and written by Claude (AI assistant) on a contributor's machine.

@adriadam10
adriadam10 merged commit ddba90e into boykopovar:main Oct 14, 2026
8 of 10 checks passed
MarcZX10 pushed a commit to MarcZX10/AnyPS5 that referenced this pull request Oct 15, 2026
…ithout PlayGo metadata

Since boykopovar#5494 a dump with no playgo-chunk.dat and no playgo-scenario.json
throws on the first nonzero chunk id. High on Life (PPSA15388) asks
scePlayGoGetLocus about ids 0-999 at startup and now aborts on id 1;
Sifu (PPSA03001) walks ids until the first error (boykopovar#4491).

APS5_PLAYGO_UNLISTED_CHUNKS=installed reports such ids as installed and
=absent returns SCE_PLAYGO_ERROR_BAD_CHUNK_ID. Without it the stop stays,
and its message now names the setting. Any other value throws.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MarcZX10 pushed a commit to MarcZX10/AnyPS5 that referenced this pull request Oct 15, 2026
…ithout PlayGo metadata

Since boykopovar#5494 a dump with no playgo-chunk.dat and no playgo-scenario.json
throws on the first nonzero chunk id. High on Life (PPSA15388) asks
scePlayGoGetLocus about ids 0-999 at startup and now aborts on id 1;
Sifu (PPSA03001) walks ids until the first error (boykopovar#4491).

APS5_PLAYGO_UNLISTED_CHUNKS=installed reports such ids as installed and
=absent returns SCE_PLAYGO_ERROR_BAD_CHUNK_ID. Without it the stop stays,
and its message now names the setting. Any other value throws.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Tamalero added a commit to Tamalero/AnyPS5 that referenced this pull request Oct 16, 2026
…ithout PlayGo metadata

Since boykopovar#5494 a dump with no playgo-chunk.dat and no playgo-scenario.json
throws on the first nonzero chunk id. High on Life (PPSA15388) asks
scePlayGoGetLocus about ids 0-999 at startup and now aborts on id 1;
Sifu (PPSA03001) walks ids until the first error (boykopovar#4491).

APS5_PLAYGO_UNLISTED_CHUNKS=installed reports such ids as installed and
=absent returns SCE_PLAYGO_ERROR_BAD_CHUNK_ID. Without it the stop stays,
and its message now names the setting. Any other value throws.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

5 participants