Repository navigation
feat(libSceNpCommerce): implement sceNpCommerceDialogOpen2 - #3607
Conversation
adriadam10
left a comment
There was a problem hiding this comment.
Thanks for the PR. Points to address:
core/libs/prx/libSceNpCommerce/Export.cpp:82:sceNpCommerceDialogOpen2now forwards tosceNpCommerceDialogOpen, which returnsCOMMON_DIALOG_ERROR_NOT_INITIALIZED/ARG_NULL. The only reference we found (prosper,src/hle/np/np.cpp:1224,tests/hle/np/test_np_sublibs.cpp:347) callssceNpCommerceDialogOpen2with all-zero arguments and expects 0. Please say which source the param type and the validation come from, or follow prosper; if neither is known, the unverified part should throw.- No test is added. Please add a guest test (registered in ctest) pinning the returned values, including the status afterwards.
docs/dev/TechnicalDebt.mdconflicts withmain; please rebase. Keep the new line away from the end of the section.- The PR body does not follow the template (
conventionsis red:What,Tested,Checklistempty). Please fill them in; "Tested" should list what you ran.
68d68a5 to
556ed4f
Compare
|
done |
|
Independent pre-review on Linux (full CI runs only happen on manual trigger, so recording one): CachyOS, kernel 7.2.9, GCC 16.2.1, Ninja, Release,
|
|
Which title calls |
adriadam10
left a comment
There was a problem hiding this comment.
Thanks for the update. Still open from the previous review, plus the question above:
core/libs/prx/libSceNpCommerce/Export.cpp:84: theCOMMON_DIALOG_ERROR_NOT_INITIALIZEDcheck has no source. The only reference we found (prosper,src/hle/np/np.cpp:500,s_commerce_openregistered forsceNpCommerceDialogOpen2too) ignores every argument, validates nothing and returns 0. Please cite where the validation and theconst void*parameter come from, or follow prosper; whatever is not known should throw.core/libs/tests/GuestNpCommerceDialog.cpp:44: the test pins the unsourcedNOT_INITIALIZEDresult, so it only checks the PR's own choice.- @oneandonlydean's question is unanswered: which title calls
sceNpCommerceDialogOpen2(title ID, and the log line where it stops on main)?docs/dev/TechnicalDebt.mdonly says PPSA23566 imports it. - The branch conflicts with
main(core/libs/CMakeLists.txt,docs/dev/TechnicalDebt.md); please rebase.
|
There is now concrete title evidence in the newer overlapping PR #5416: Call of Duty: Black Ops (PPSA34501), on the |
|
I tested this PR with Call of Duty: Black Ops (PPSA34501) on Windows 11. Using my |
|
Thanks for the Windows 11 test. It confirms that Call of Duty: Black Ops (PPSA34501) reaches sceNpCommerceDialogOpen2 at Zombies → Online and continues past the exception with this implementation, which answers the requested title and repro evidence. The same call is implemented in open PR #5416, so the two PRs now have matching title-level validation and need coordination around their overlapping change. |
|
@itsbo5 here is a rebase of this PR that follows prosper, as @adriadam10's review asked. cc @oneandonlydean, since you're assigned. What the sources show:
Changes on top of 556ed4f, rebased onto
The PR body's What and Tested still describe the Tested with a WSL Ubuntu GCC Release build at the branch head:
The branch is nikowolf123/AnyPS5 Maintainer edits are enabled on your branch, so a maintainer could also push it there. @alexpournaras could you rerun Zombies > Online on this head? Compared with the 556ed4f change you ran, one thing differs: before Diff against main (763ad20), without the one-line removal in libSceAudioOut.mddiff --git a/core/libs/prx/libSceNpCommerce/CMakeLists.txt b/core/libs/prx/libSceNpCommerce/CMakeLists.txt
new file mode 100644
index 000000000..ec34bf123
--- /dev/null
+++ b/core/libs/prx/libSceNpCommerce/CMakeLists.txt
@@ -0,0 +1,15 @@
+add_library(libSceNpCommerce SHARED EXCLUDE_FROM_ALL Export.cpp)
+target_include_directories(libSceNpCommerce PRIVATE ${LIBS_INCLUDE_DIR})
+configure_windows_unwind(libSceNpCommerce)
+
+if(BUILD_TESTING)
+ add_test_executable(guest_np_commerce_dialog_tests ${LIBS_INCLUDE_DIR}/tests/GuestNpCommerceDialog.cpp)
+ target_include_directories(guest_np_commerce_dialog_tests PRIVATE ${LIBS_INCLUDE_DIR})
+ target_link_libraries(guest_np_commerce_dialog_tests PRIVATE libSceNpCommerce libc)
+ configure_windows_unwind(guest_np_commerce_dialog_tests)
+ add_test(NAME guest_np_commerce_dialog COMMAND guest_np_commerce_dialog_tests)
+ if(WIN32)
+ set_tests_properties(guest_np_commerce_dialog PROPERTIES ENVIRONMENT_MODIFICATION
+ "PATH=path_list_prepend:$<TARGET_FILE_DIR:libSceNpCommerce>")
+ endif()
+endif()
diff --git a/core/libs/prx/libSceNpCommerce/Export.cpp b/core/libs/prx/libSceNpCommerce/Export.cpp
index 525178f39..39fecd0fc 100644
--- a/core/libs/prx/libSceNpCommerce/Export.cpp
+++ b/core/libs/prx/libSceNpCommerce/Export.cpp
@@ -80,8 +80,8 @@ int APS5_VABI sceNpCommerceSetPsStoreIconLayout(int layout) {
}
int APS5_VABI sceNpCommerceDialogOpen2(void) {
- NotImplemented_nid_no_patch(__func__);
- return 0;
+ g_status = COMMON_DIALOG_STATUS_FINISHED;
+ return 0;
}
}
diff --git a/core/libs/tests/GuestNpCommerceDialog.cpp b/core/libs/tests/GuestNpCommerceDialog.cpp
new file mode 100644
index 000000000..7b7239959
--- /dev/null
+++ b/core/libs/tests/GuestNpCommerceDialog.cpp
@@ -0,0 +1,64 @@
+#include "SceTypes.hpp"
+
+#include <cstdint>
+#include <cstdlib>
+#include <cstring>
+
+extern "C" {
+int APS5_VABI sceNpCommerceDialogInitialize();
+int APS5_VABI sceNpCommerceDialogOpen(const void* param);
+int APS5_VABI sceNpCommerceDialogOpen2(void);
+int APS5_VABI sceNpCommerceDialogUpdateStatus(void);
+int APS5_VABI sceNpCommerceDialogGetStatus(void);
+int APS5_VABI sceNpCommerceDialogGetResult(void* result);
+int APS5_VABI sceNpCommerceDialogClose(void);
+int APS5_VABI sceNpCommerceDialogTerminate();
+}
+
+namespace {
+
+void Require(bool value) { if (!value) std::abort(); }
+
+constexpr int statusNone = 0;
+constexpr int statusInitialized = 1;
+constexpr int statusFinished = 3;
+constexpr std::int32_t resultUserCanceled = 1;
+
+void RequireCanceledResult() {
+ std::uint8_t result[0x20];
+ std::memset(result, 0x33, sizeof(result));
+ Require(sceNpCommerceDialogGetResult(result) == 0);
+ std::int32_t value = 0;
+ std::memcpy(&value, result, sizeof(value));
+ Require(value == resultUserCanceled);
+ Require(result[4] == 0);
+ Require(result[5] == 0x33);
+}
+
+}
+
+int main() {
+ Require(sceNpCommerceDialogGetStatus() == statusNone);
+ Require(sceNpCommerceDialogInitialize() == 0);
+ Require(sceNpCommerceDialogGetStatus() == statusInitialized);
+
+ Require(sceNpCommerceDialogOpen2() == 0);
+ Require(sceNpCommerceDialogUpdateStatus() == statusFinished);
+ Require(sceNpCommerceDialogGetStatus() == statusFinished);
+ RequireCanceledResult();
+
+ Require(sceNpCommerceDialogClose() == 0);
+ Require(sceNpCommerceDialogGetStatus() == statusFinished);
+ Require(sceNpCommerceDialogTerminate() == 0);
+ Require(sceNpCommerceDialogGetStatus() == statusNone);
+
+ alignas(8) std::uint8_t param[0x40] = {};
+ Require(sceNpCommerceDialogInitialize() == 0);
+ Require(sceNpCommerceDialogOpen(param) == 0);
+ Require(sceNpCommerceDialogOpen2() == 0);
+ Require(sceNpCommerceDialogUpdateStatus() == statusFinished);
+ RequireCanceledResult();
+ Require(sceNpCommerceDialogTerminate() == 0);
+ Require(sceNpCommerceDialogGetStatus() == statusNone);
+ return 0;
+}
diff --git a/docs/dev/debt/libSceNpCommerce.md b/docs/dev/debt/libSceNpCommerce.md
index a3a899c13..7a9d0ec64 100644
--- a/docs/dev/debt/libSceNpCommerce.md
+++ b/docs/dev/debt/libSceNpCommerce.md
@@ -3,3 +3,7 @@
### Silent stubs
- [libSceNpCommerce](../../../core/libs/prx/libSceNpCommerce/Export.cpp) - the PS Store icon show/hide calls and `sceNpCommerceSetPsStoreIconLayout` do nothing
+
+### Unknown function info
+
+- [sceNpCommerceDialogOpen2](../../../core/libs/prx/libSceNpCommerce/Export.cpp) (libSceNpCommerce) - declared without parameters; follows [prosper](https://github.com/mattias800/prosper/blob/73413cb445b0364bb49c4d2478e9ee1614b9f7d9/prosper/src/hle/np/np.cpp#L500-L504), which serves it with its `sceNpCommerceDialogOpen` handler: every argument is ignored, nothing is validated, it returns 0 and the dialog finishes at once, so `sceNpCommerceDialogGetResult` reports it cancelled. The parameters and the console's error results are unknown. Call of Duty: Black Ops (PPSA34501) calls it at Zombies > OnlineAI-assisted: written with Claude Code. I checked prosper's |
556ed4f to
5f0bb25
Compare
|
Pushed @nikowolf123's rebase here, onto current main (the |
|
@nikowolf123 I only copied the relevant change ( |
Met: rebased onto main following prosper (no NOT_INITIALIZED check), with a test.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
5f0bb25 to
68bddf8
Compare
What
Implements
sceNpCommerceDialogOpen2followingsceNpCommerceDialogOpen.Tested
Linux build;
guest_np_commerce_dialog(ctest) covers not-initialized, null and zero parameters, status and result after the call.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt.mdfiles or images added to the repository (attach them to this PR)