Repository navigation
feat(libSceNpCppWebApi): cover SessionManager PlayerSessionsApi and InGameCatalog ContainerApi imports - #8959
Merged
oneandonlydean merged 6 commits intoOct 23, 2026
Conversation
…nGameCatalog ContainerApi imports
github-actions
Bot
added
the
unblocks title
Progress report (f2eee36 → 45bc432)📈 System libraries: 44.8% (+0.24%, +115 implemented, +87 declared functions) ✅ 115 implemented
⚪ 75 behaviourally empty (constant return or throw)
🆕 87 declared as stubs
|
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
MarcZX10
reviewed
Oct 21, 2026
MarcZX10
suggested changes
Oct 21, 2026
MarcZX10
left a comment
MarcZX10
left a comment
Contributor
There was a problem hiding this comment.
Requesting one correction before approval: PlayerSessionsApi::setPlayerSessionProperties returns success (0) while the PR describes offline request entry points as returning SCE_NP_WEBAPI_ERROR_NOT_SIGNED_IN. Please make this mutation consistent with the other PlayerSessionsApi request methods. I left an inline note on the implementation. The open overlap with #4278 also still needs the superseded/merge-order clarification from my earlier comment.
Contributor
Author
|
Fixed in 5e4ebed — On the overlap flag (#8264, #8561, #8843): the export sets are disjoint at the NID level — this PR's 198 imports were computed from the title's relinker registry minus everything already covered. #8561 covers the 64 UserProfile/ProfanityFilter/CommunicationRestrictionStatus imports (deliberately excluded here), #8264 covers 37 unrelated aerolib exports (none in this set), and #8843 is signature-alignment on existing stubs. The flagged collisions are textual only (same debt-file region, same Export.cpp tail) — whichever lands second rebases a few lines; no functional supersession. Happy to rebase on whichever goes first if a specific order is preferred. Re: #4278 — I can't find an earlier comment on this PR referencing it (it resolves to a shader perf PR); if you meant another ticket, point me at it and I'll address it. |
MarcZX10
reviewed
Oct 21, 2026
Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Contributor
Author
|
Done in 7b5594a — TestOfflineResponses now asserts setPlayerSessionProperties returns SCE_NP_WEBAPI_ERROR_NOT_SIGNED_IN alongside getPlayerSessions, so a future success-return regression on a mutating verb is caught. Also fixed the hasResponse extern (V129 -> V130PostPlayerSessionsResponseBody) which had drifted from the exported symbol. Compiled and ran locally: all 8 sub-tests pass. |
…and getContainer Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Merged
12 tasks done
akkadbakkad89
force-pushed
the
feat/np-webapi-session-ingc
branch
from
October 21, 2026 10:42
5612578 to
d3e68ab
Compare
MarcZX10
approved these changes
Oct 21, 2026
MarcZX10
left a comment
MarcZX10
left a comment
Contributor
There was a problem hiding this comment.
Reviewed the current head, including the latest ParameterToGetContainer lifecycle and getContainer change. The earlier offline error finding is fixed, the follow-up adds a regression assertion for setPlayerSessionProperties, and both review threads are resolved. I found no remaining blocking correctness issue in the reviewed diff.
Local validation on this head: c++ -std=c++20 -fsyntax-only passed for Export.cpp and GuestNpCppWebApiString.cpp; I linked and ran the focused test executable, and it passed. The full CMake test configuration could not be generated in this environment because the SDL2 submodule sources are absent and the host lacks the required SDL XRandR development support. Hosted eligibility, conventions, labels, and report checks are green; the build jobs were skipped by path rules.
12 tasks done
Contributor
|
I opened #9612 to preserve aliased inputs in the existing |
Collaborator
|
@akkadbakkad89 I read the whole change line by line. The offline model matches the rest of the file, and the new test cases pass in a standalone build. There are two mechanical problems in the
I checked this with a small program that calls the exports. With |
…the vector count offset
akkadbakkad89
force-pushed
the
feat/np-webapi-session-ingc
branch
from
October 23, 2026 02:38
d3e68ab to
45bc432
Compare
Contributor
Author
|
@oneandonlydean all three fixed in
The rebase resolution kept every #8561 UserProfile block and our blocks verbatim (union of both sides; each side's exported functions are disjoint, as you said). On the count-check test row: agreed it's worth having — the aliasing/ AI-assisted: yes (Devin). |
MarcZX10
suggested changes
Oct 23, 2026
MarcZX10
left a comment
MarcZX10
left a comment
Contributor
There was a problem hiding this comment.
The latest implementation corrects the three issues identified in the thread: the six copy constructors now retain their source pointer, the four Vector<IntrusivePtr<...>> operations use the 0x30 reference-count field, and empty-string equality matches c_str(). I checked the exact 19-line source correction, local diff whitespace and conventions; both pass. I found no new source defect in those corrections.
Two items still prevent approval:
- The new copy-constructor/refcount fixes have no regression assertion in the current diff. The existing three focused cases cover
String,Vector::pushBack, and offline responses, but do not copy one of these affectedIntrusivePtrobjects and verify both its pointer and reference count at offset0x00or0x30. Please add a focused check (coordinate its insertion with #9612's test changes) or provide an exact-head standalone regression run that proves those cases. tools/check_main_sync.py --base upstream/main --head <head>reports 6 commits ahead / 53 behind currentupstream/main(merge basef4b2f7907), though the merge is clean. CONTRIBUTING requires branches kept current. Please merge current main and rerun the focused string/API tests and conventions on the resulting head.
The prior approval was on d3e68ab, before the copy/refcount corrections, so it does not cover this head. The Windows/Linux test results from earlier heads are not presented as reruns of these latest changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
198 more
libSceNpCppWebApiexports, all imported by PPSA20397 (Assassin's Creed Shadows, recorded in the title's relinker registry; every name recomputes to the imported NID viatools/nid_names.py):SessionManager::V1::PlayerSessionsApiverbs,ParameterTo*lifecycle, request/response model accessorsInGameCatalog::V5::ContainerApi+ its parameters and response model accessorsCommonhelpers the title binds:IntrusivePtrctors/dtor/deref/assign/copy over the new pointee types,VectorC1/D1/pushBack/empty/size/operator[],Iterator/ConstIteratordtors,Transaction/TransactionBaselifecycle,String::empty/operator==Semantics follow the file's established offline model: request entry points and transaction IO fail
SCE_NP_WEBAPI_ERROR_NOT_SIGNED_IN;hasResponsehonestly answers 0; parameter bookkeeping succeeds without keeping state.Vector::empty/size/operator[]read the host-createdVectorBody(honest for bodies ourpushBackgrows; they throw on title-provided bodies whose element layout is unknown). Response accessors, iterators,Vector::begin/end/copyFromand transactionabort/getId/operator=throw: a failed request never produces a response, and aborting or renumbering a transaction that never ran has no defined outcome. New stubs are documented in the library's debt file.Tested
Linux x86_64, GCC 13 (g++
-fsyntax-onlyclean forExport.cpp).guest_np_cpp_web_api_string(the library's existing test) extended with three cases and run against the new code — all pass:String::empty/String::operator==semantics,Vector<IntrusivePtr<...JoinableUser>>ctor+pushBack+dtor lifecycle (body refcount, element refcount increment, element deleter on release), and the offline responses (Transaction::hasResponse= 0,PlayerSessionsApi::getPlayerSessions=0x80552907).Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)Overlap checked two ways:
tools/pr_overlap.py --branch feat/np-webapi-session-ingcreports no overlap, and a NID-level comparison against #8561 (UserProfile/ProfanityFilter/CommunicationRestrictionStatus) shows it covers 64 other imports of the same title — disjoint from this set. The title itself still stops at the unrelated AGC stencil-replay gate before reaching these calls; this PR is pure import-coverage work on the recorded registry.