Skip to content

feat(savedata): implement save icons and the read-only mount check - #3950

Closed
semirkabir wants to merge 1 commit into
boykopovar:mainfrom
semirkabir:feat/savedata-save-icons
Closed

semirkabir wants to merge 1 commit into
boykopovar:mainfrom
semirkabir:feat/savedata-save-icons

Conversation

@semirkabir

@semirkabir semirkabir commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

What

Addresses the review: the removed sceSaveDataMount3: unknown mount_mode throw is restored (d3c30eb0f), and every // comment outside the technical-debt markers is gone from Export.cpp, SaveData.hpp and tests/GuestSaveDataIcon.cpp.

  • libSceSaveData.native: implement sceSaveDataSaveIcon, sceSaveDataLoadIcon and the savedata-memory icon paths (GetSaveDataMemory2/SetSaveDataMemory2/SetupSaveDataMemory2 with an init_icon). Icons persist at _sd/<dir>/sce_sys/icon0.png, the console layout titles also see through the mount; the memory sidecar lives at _sd_mem/<slot>.icon. Loads follow shadPS4 semantics (buf_size fill, full size in data_size, drop on an empty-icon set).
  • Mounts now record RDONLY at sceSaveDataMount3, so a save on a read-only mount returns BAD_MOUNTED (0x809F000D) instead of silently succeeding.
  • Export-name evidence: every export of the library hash-verifies against the zecoxao/sce_symbols aerolib database (checked with tools/nid_names.py), including the previously name-unverified sceSaveDataCancel stub; recorded in TechnicalDebt.

Tested

Ubuntu 24.04 container, no GPU. CMake configure + build from scratch; all 13 savedata tests pass, including the new guest_savedata_icon (tests/GuestSaveDataIcon.cpp: round trip, truncation, NOT_FOUND before a save, null-argument PARAMETER checks, RDONLY BAD_MOUNTED, memory sidecar init/get/set/drop, Get with a null icon buffer). Also verified against the built export set: all 28 libSceSaveData.native export names recompute their NIDs. Rebased onto current main (bca7a29): the unknown-mount_mode throw is back (d3c30eb) and every test still passes.

No title was run: no PS5 console behavior was tested, and I have no title that reaches these functions yet (the ps5-doom binary imports no libSceSaveData functions, so its log cannot exercise the icon path). If a title I can run shows the save-icon flow, I will run it and record the log lines here; until then this stays one hurdle short of the title-log evidence incoming reviews now ask for.

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 (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • AI-assisted: yes

@github-actions github-actions Bot added build docs prx needs rebase Conflicts with main, rebase needed labels Oct 11, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

Two things before this can go in:

  • libSceSaveData.native/Export.cpp, mount3: the sceSaveDataMount3: unknown mount_mode throw is removed, which the description doesn't mention and the new tests don't need. Please put it back.
  • The // comments in Export.cpp, SaveData.hpp and GuestSaveDataIcon.cpp have to go; only technical-debt markers are allowed.

Which title loads an icon it saved? The description says none was run.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Conflicts with main again in core/libs/prx/libSceSaveData.native/Export.cpp: #3814 added sceSaveDataDirNameSearch2 at the end of the file. Please rebase, and leave the sceSaveDataCancel TechnicalDebt entry as it is on main; the NID note doesn't belong in this PR.

@semirkabir

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (089f49002): the file-end conflict with #3814 is resolved, sceSaveDataDirNameSearch2 and the RvID8BVeg+g unknown stay as they are on main, and the sceSaveDataCancel TechnicalDebt entry is back to main's text without the NID note. All 14 savedata and neighbouring guest tests pass, and conventions are green.

@semirkabir
semirkabir force-pushed the feat/savedata-save-icons branch from 089f490 to 80aa747 Compare October 13, 2026 04:17
@github-actions

Copy link
Copy Markdown

Progress report (e51428b → 80aa747)

📉 System libraries: 87.9% (-0.02%, -1 reverted functions)

⚠️ 1 went back to stubs
Library Name
libSceSaveData.native sceSaveDataDirNameSearch2

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

xXPBSXx commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Tested on Windows 10 (19045), MinGW-w64 GCC 15.2.0 (WinLibs r7), Release: this PR merged onto main c080fc2d5 (no conflict).

  • guest_savedata_icon passes with the PR (0.29 s).
  • With the PR's changes to libSceSaveData.native/Export.cpp and SaveData.hpp taken back to main, it fails (load without an icon returns NOT_FOUND, icon lands at sce_sys/icon0.png inside ...), so the new checks fail without the change.

Not tested: Linux, a console, a title run.

AI-assisted (Claude Code).

@nicolasfalesy

Copy link
Copy Markdown
Contributor

Linux test (Arch, GCC 16.2.1, Release), this PR's head merged alone onto main 7e181fc (clean merge): main's guest_savedata_search fails with sceSaveDataDirNameSearch2 not implemented. The branch still replaces main's implementation in libSceSaveData.native/Export.cpp with a stub (int APS5_VABI sceSaveDataDirNameSearch2() { NotImplemented_nid_no_patch(__func__); ...), which is also what the progress report's "1 went back to stubs" refers to. Keeping main's version of that function should fix it.

@AaravMehta-07

Copy link
Copy Markdown
Contributor

While preparing a separate savedata wildcard regression, I noticed this PR's current diff removes the trailing-% handling in dir_name_match introduced by merged #4504. That would regress patterns such as TRAIL% matching TRAIL.

My candidate keeps the existing %/_ semantics but replaces recursive matching: one search across three legal 31-character save names with %A repeated 15 times followed by B exceeds the 30-second test limit on the baseline. Since both patches touch this helper, could you keep the matcher unchanged when rebasing this icon PR, so the independent wildcard fix can follow without conflict?

This investigation and the proposed wildcard patch are AI-assisted. No commercial-title performance or PS5 hardware behavior is claimed.

EagleBearerYS added a commit to EagleBearerYS/AnyPS5 that referenced this pull request Oct 13, 2026
boykopovar#5173 changes the create condition right after the last error check, and boykopovar#3950 removes the comment after the create block, so clearing the result in either place made the pull requests conflict. The result is still cleared only after every error return and before anything is written to it.

@adriadam10 adriadam10 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Merged onto current main, this branch still reverts code that is already merged:

  • core/libs/prx/libSceSaveData.native/Export.cpp:901: sceSaveDataDirNameSearch2 is turned back into a NotImplemented_nid_no_patch stub. This fails guest_savedata_search (as @nicolasfalesy found) and is the "went back to stubs" line in the progress report. Keep main's implementation (dirNameSearch + trace).
  • Export.cpp:64: the while (*pattern == '%') tail in dir_name_match (from #4504) is removed, so TRAIL% no longer matches TRAIL (@AaravMehta-07). Keep the matcher as on main.
  • Comments that exist on main and have nothing to do with icons are deleted (Export.cpp:34, :69, :93, :771, :802, SaveData.hpp:10). Please drop those hunks; only comments the PR itself adds should go.

The easiest way is to reset those hunks to main (git checkout origin/main -- <file> then re-apply only the icon/RDONLY changes), rebase, and re-run guest_savedata_search along with guest_savedata_icon. The icon and read-only mount changes themselves were not reviewed in depth until the branch stops overwriting main.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Closing: no title is known to reach this. With the queue at 700+ open PRs, please hold off on new PRs that don't name a title needing the change until it's down. If a title stops on this on current main, reopen with the title ID and the log line.

RINNECODER added a commit to RINNECODER/AnyPS5 that referenced this pull request Oct 14, 2026
… nid and others) (#489)

* fix(nid): validate ELF table layouts before patching

(cherry picked from commit abe8fb8)

* test(nid): avoid competing hash test registration

(cherry picked from commit 617e7ac)

* fix(avplayer): resume the presentation clock when the source restarts after a pause

Clock::Reset left the paused flag set by Clock::Pause, so a Start issued
from the paused state (stop and restart of the source) kept the clock
frozen at the start position. Only the frame at that position was
delivered; later frames were held back by the default A/V sync.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
(cherry picked from commit a671dc4)

* fix(libSceNet): accept MSG_DONTWAIT in the send and receive calls

sceNetSend, sceNetSendto, sceNetRecv, sceNetRecvfrom, sceNetRecvmsg and
sceNetSendmsg failed with EOPNOTSUPP for any flag other than MSG_PEEK on
a receive, so a game polling a socket with MSG_DONTWAIT got an error on
every call instead of its data or EWOULDBLOCK.

FreeBSD 11.4 defines the flag in sys/sys/socket.h:

	#define	MSG_DONTWAIT	0x80		/* this message should be nonblocking */

and soreceive_generic() (sys/kern/uipc_socket.c) treats it like a
non-blocking socket for that one call:

		if ((so->so_state & SS_NBIO) ||
		    (flags & (MSG_DONTWAIT|MSG_NBIO))) {
			SOCKBUF_UNLOCK(&so->so_rcv);
			error = EWOULDBLOCK;
			goto release;

shadPS4's sceNet uses the same value (ORBIS_NET_MSG_DONTWAIT 0x80).

On Linux and macOS the flag now maps to the host's MSG_DONTWAIT, alone
or with MSG_PEEK on a receive. Winsock has no per-call non-blocking
flag, so Windows keeps failing with EOPNOTSUPP for it; TechnicalDebt
records that. Other flags still fail with EOPNOTSUPP.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 44d46ee)

* fix(audioout): serialize shared SDL audio initialization

(cherry picked from commit c35572e)

* test(audioout): detect overlapping backend initialization

(cherry picked from commit 5031432)

* feat(net): implement memory-pool netstat diagnostic

(cherry picked from commit 565fafa)

* fix(libSceVideoOut): return RESOURCE_BUSY when a bus is opened twice

(cherry picked from commit c15e9f2)

* fix(savedata): leave the mount result untouched when sceSaveDataMount3 fails

A title checks for its save with a CREATE2 mount it never
unmounts, then saves with a second mount of the same directory. That
mount returns BUSY, and the title keeps using the first mount point for
sceSaveDataPrepare, its save file and sceSaveDataUmount2.
sceSaveDataMount3 cleared the result before its checks, so the save was
written to "/save" in the game folder instead of the save directory, and
the first mount was never released. The result is now written only when
the mount succeeds, as shadPS4 does.

(cherry picked from commit 4ccf888)

* fix(libSceNet): report SO_ERROR from sceNetGetsockopt

(cherry picked from commit 0daf1d5)

* fix(savedata): report a save's size as the mount that created it gave

Every native save reported SAVE_DATA_BLOCKS_MAX blocks, all of them free,
in the sceSaveDataDirNameSearch infos and in sceSaveDataGetMountInfo,
whatever the title created it with and whatever its files held. A save's
size is now the blocks of the sceSaveDataMount3 call that created it,
kept beside the save; a save without a kept size takes that of its next
mount that names one, and otherwise keeps SAVE_DATA_BLOCKS_MAX. Free
blocks are what the save's files leave of it, each file rounded up to
32 KiB blocks, and the blocks and free-blocks sort keys use both.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 4b7a357)

* fix(savedata): return no saves for a search with another title's ID

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit d34b788)

* feat(libScePosixForWebKit): implement getservbyname

Implement getservbyname (H1bArJPagV0) in libScePosixForWebKit. A title imports it from this library only (registry index 408,
call site 0x1042124); once fnmatch (boykopovar#3545) is in, its predicted load
stop is:

    FAIL: unresolved ELF import H1bArJPagV0 from libScePosixForWebKit.prx

The ps5-payload-dev SDK stubs (f5d40f02, sce_stubs/) declare
getservbyname in libScePosixForWebKit.c and not in libkernel.c or
libSceLibcInternal.c. The call site is the service-name branch of a
getaddrinfo inside the title: a non-numeric service is looked up with
protocol "tcp", "udp" or "sctp" and the port is read big-endian at
offset 16 of the result, which fixes the servent layout used here
(name, aliases, 32-bit port, protocol at 0, 8, 16, 24).

The guest has no /etc/services and the console's file is unknown, so the
function searches 37 lines of FreeBSD 11.4 etc/services (8c639945):
ftp-data, ftp, ssh, telnet, smtp, domain, tftp, http, pop3, ntp, imap,
https, rtsp, submission, imaps and pop3s with their aliases and
protocols, in file order. None of their names or aliases appears on an
earlier line of the full file, so these lookups answer as the full file
would. The search follows files_servent in lib/libc/net/getservent.c
(8c639945): exact, case-sensitive match on the name or an alias, a null
protocol takes the first match, the port in network byte order, the
result in per-thread storage like getserv's NSS TLS state. Any other
name returns null without touching errno, as for a name the file lacks.
The repository's getaddrinfo still resolves service names through the
host, so there was no table to share; this is recorded in
TechnicalDebt.md.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
(cherry picked from commit 15bbc19)

* fix(libScePosixForWebKit): keep the getservbyname result trivially destructible

The per-thread servent storage held std::string and std::vector
members, so getservbyname_nid_postfix registered a thread_local
destructor through __cxa_thread_atexit. On Windows that destructor runs
after emulated TLS freed the object, and prx_thread_local_destructors
(boykopovar#4029) rejects libScePosixForWebKit.prx for it.

The result now lives in a thread_local struct of fixed-size char arrays
and an alias pointer array, sized for the longest name (submission),
alias (www-http), protocol (sctp) and alias count in the embedded table;
static_asserts check the sizes against the table and that the struct is
trivially destructible. The guest servent layout and the per-thread
result FreeBSD's getservent.c keeps in its NSS TLS state are unchanged.
Nothing is allocated, so the ENOMEM path is gone.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
(cherry picked from commit 8b9529b)

* test(libSceConvertKeycode): cover sceConvertKeycodeGetImeKeyboardType

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 0afa191)

* feat(ces): implement sceCesUcsProfileInitSJis1997Cp932

(cherry picked from commit e5f0938)

* refactor(savedata): clear the mount result once the mount point is made

boykopovar#5173 changes the create condition right after the last error check, and boykopovar#3950 removes the comment after the create block, so clearing the result in either place made the pull requests conflict. The result is still cleared only after every error return and before anything is written to it.

(cherry picked from commit 6a46563)

* fix(libSceKeyboard): report one key slot when no key is held

A keyboard record on the console never counts fewer than one key: with
nothing held, and with no keyboard connected, the count is 1 and that
slot holds code 0. SharpProspero's keyboard reader, PS5_Quake3e's notes
on records read from a console and the PS5 input research notes all say
so. AnyPS5 reported a count of 0. The count is now at least 1; the key
slots themselves are unchanged.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
(cherry picked from commit 8ea8ea3)

* fix(libSceNpAuth): report an async request as running on its first poll

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit cb8b0ca)

* feat(libSceConvertKeycode): implement sceConvertKeycodeGetCharacter for the US layout

The signature is inferred from a title call: a 16-bit keycode, a second
argument the title passes as 0, the type from
sceConvertKeycodeGetImeKeyboardType and a UTF-16 output. The keycode is a
USB HID keyboard usage and the unshifted US character is returned; a
keycode without a character returns 0. A second argument or keyboard type
other than 0 throws.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 5ca6487)

* feat(libSceNet): implement sceNetResolverStartNtoa6

Resolve a host name to its first IPv6 address through the host resolver
(getaddrinfo with AF_INET6) and write the 16-byte address. The lookup
shares the IPv4 path's resolver bookkeeping (abort preservation, error
state); a name without an IPv6 address fails with RESOLVER_ENODNS like a
failed lookup. Nonzero flags throw.

A title imports it and failed to
load without it (unresolved zl35YNs9jnI from libSceNet.prx).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit df32a44)

* fix(libSceNet): dissolve a datagram association on connect to AF_UNSPEC

sceNetConnect() on a connected datagram socket rejected an address
whose family is AF_UNSPEC (a null address) with EINVAL before touching
the socket, so the socket stayed connected: sceNetGetpeername() still
reported the old peer and datagrams from any other sender were dropped
by the host.

FreeBSD 11.4 disconnects a connected datagram socket first and only then
looks at the address. sys/kern/uipc_socket.c, soconnectat():

	 * If protocol is connection-based, can only connect once.
	 * Otherwise, if connected, try to disconnect first.  This allows
	 * user to disconnect by connecting to, e.g., a null address.
	 */
	if (so->so_state & (SS_ISCONNECTED|SS_ISCONNECTING) &&
	    ((so->so_proto->pr_flags & PR_CONNREQUIRED) ||
	    (error = sodisconnect(so)))) {

udp_disconnect() (sys/netinet/udp_usrreq.c) clears the peer and keeps
the local port. connect(2): "Datagram sockets may dissolve the
association by connecting to an invalid address, such as a null
address." libkernel's connect gets the same change in boykopovar#4498.

A connected datagram socket given an address whose family byte is 0 is
now disconnected on the host first (AF_UNSPEC on Linux, a zeroed
address of the socket's family on Windows), and the call then fails
with EINVAL as before. Linux also drops an ephemeral local port on that
disconnect, so the port is bound again on the wildcard address, which
is what FreeBSD leaves. A host failure during the disconnect is
returned as the call's error. Stream sockets keep the previous path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 65d56b3)

* fix(libSceNet): fail the AF_UNSPEC connect with EAFNOSUPPORT as FreeBSD does

Co-Authored-By: Claude <noreply@anthropic.com>
(cherry picked from commit fce9538)

* fix(libSceOpusDec): decode narrowband SILK packets

FFmpeg resamples narrowband SILK (TOC configurations 0-3) from 8 kHz and
holds back 24 samples of the first narrowband packet after the decoder
starts or after a packet of another bandwidth: a 20 ms packet decodes to
936 samples and the next wider packet to 984. sceOpusDecDecode required
exactly the packet's duration and threw, so ProsperoRadio aborted on a
narrowband Opus station.

The decoder now keeps FFmpeg's output in a queue and returns exactly the
packet's duration, as libopus does: the first narrowband packet starts
with 24 samples of silence and the held-back samples follow in the next
calls. The new test decodes three libopus 6 kb/s narrowband packets and
a wideband packet after them; it aborts on main.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
(cherry picked from commit fb41f10)

* fix(http2): allow completion waits without an event queue

(cherry picked from commit 28bcd6b)

* feat(libSceNgs2.native): implement custom sampler rack id 0x4001 (boykopovar#4334)

(cherry picked from commit 855823e)

* feat(libSceNgs2.native): add custom voice filter support and related tests for 0x13 issue case

(cherry picked from commit ef52b16)

* test(libSceNgs2): fix custom filter regression expectations

(cherry picked from commit 852c808)

* fix(ngs2): resolve voice patches to rack handles

(cherry picked from commit 8b0da18)

* docs: drop title IDs from ported test and debt text

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: MiniRML <134226498+MiniRML@users.noreply.github.com>
Co-authored-by: xXPBSXx <20614131+xXPBSXx@users.noreply.github.com>
Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-authored-by: milescoviello <kitslayer29@gmail.com>
Co-authored-by: PrintOrb <donhou45@gmail.com>
Co-authored-by: krishalivaibhav <krishalivaibhav@gmail.com>
Co-authored-by: babyhuey <7751607+babyhuey@users.noreply.github.com>
Co-authored-by: Yunus Shaikh <75296019+EagleBearerYS@users.noreply.github.com>
Co-authored-by: bubblegg <86165363+bubblegg@users.noreply.github.com>
Co-authored-by: SandstromPL <paritoshlahre@gmail.com>
Co-authored-by: screamm <davidrydgren@gmail.com>
Co-authored-by: melihcan <melihcan.abay@students.fhnw.ch>
Co-authored-by: Kikoarl <7354010+Kikoarl@users.noreply.github.com>
Co-authored-by: Rick <99503318+0xRickx@users.noreply.github.com>
Co-authored-by: Sasha Papo <sasha.papo@gmail.com>
Co-authored-by: codingreapermaps <233954859+codingreapermaps@users.noreply.github.com>
Co-authored-by: ps54all <ps5@4.all>
Co-authored-by: Adrià Franch <adriadam10@gmail.com>
Co-authored-by: eliasbruno124-dev <eliasbruno124@gmail.com>
Co-authored-by: Jnbla6 <badrsha321@gmail.com>
Co-authored-by: MarcZX100 <marczxrr@gmail.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.

6 participants