refactor: phase 0 & 1

2026-04-26 20:02:15 +03:00
parent 844093e3e7
commit b5c506cf03
36 changed files with 690 additions and 137 deletions
@@ -0,0 +1,28 @@
+---
+BasedOnStyle: LLVM
+IndentWidth: 4
+ColumnLimit: 110
+PointerAlignment: Left
+AlignAfterOpenBracket: Align
+AllowShortFunctionsOnASingleLine: Empty
+AllowShortIfStatementsOnASingleLine: Never
+AllowShortLambdasOnASingleLine: Empty
+BreakBeforeBraces: Attach
+SortIncludes: true
+IncludeBlocks: Regroup
+IncludeCategories:
+  # Project headers: nxst/
+  - Regex:           '^(<|")(nxst/)'
+    Priority:        3
+    SortPriority:    3
+  # Third-party: Plutonium, libnx, SDL, switch.h
+  - Regex:           '^(<|")(pu/|switch\.h|libnx|SDL|freetype|harfbuzz|zlib)'
+    Priority:        2
+    SortPriority:    2
+  # System / C++ standard library
+  - Regex:           '^<'
+    Priority:        1
+    SortPriority:    1
+SpacesBeforeTrailingComments: 2
+Cpp11BracedListStyle: true
+Standard: c++17
@@ -0,0 +1,47 @@
+---
+Checks: >
+  bugprone-*,
+  readability-*,
+  modernize-*,
+  cppcoreguidelines-pro-type-cstyle-cast,
+  -fuchsia-*,
+  -google-*,
+  -llvm-*,
+  -readability-magic-numbers,
+  -readability-named-parameter,
+  -modernize-use-trailing-return-type,
+  -modernize-use-nodiscard,
+  -modernize-avoid-c-arrays,
+  -bugprone-easily-swappable-parameters
+
+WarningsAsErrors: ''
+
+HeaderFilterRegex: 'include/nxst/.*'
+
+CheckOptions:
+  - key:   readability-identifier-naming.ClassCase
+    value: CamelCase
+  - key:   readability-identifier-naming.StructCase
+    value: CamelCase
+  - key:   readability-identifier-naming.FunctionCase
+    value: camelBack
+  - key:   readability-identifier-naming.MethodCase
+    value: camelBack
+  - key:   readability-identifier-naming.VariableCase
+    value: lower_case
+  - key:   readability-identifier-naming.ParameterCase
+    value: lower_case
+  - key:   readability-identifier-naming.MemberCase
+    value: lower_case
+  - key:   readability-identifier-naming.NamespaceCase
+    value: lower_case
+  - key:   readability-identifier-naming.ConstexprVariablePrefix
+    value: 'k'
+  - key:   readability-identifier-naming.ConstexprVariableCase
+    value: CamelCase
+  - key:   readability-identifier-naming.EnumCase
+    value: CamelCase
+  - key:   readability-identifier-naming.EnumConstantCase
+    value: CamelCase
+  - key:   readability-braces-around-statements.ShortStatementLines
+    value: '2'
@@ -0,0 +1,22 @@
+root = true
+
+[*]
+charset = utf-8
+end_of_line = lf
+insert_final_newline = true
+trim_trailing_whitespace = true
+
+[*.{cpp,hpp,h,c}]
+indent_style = space
+indent_size = 4
+
+[Makefile]
+indent_style = tab
+
+[*.{yml,yaml}]
+indent_style = space
+indent_size = 2
+
+[*.{json,md}]
+indent_style = space
+indent_size = 2
@@ -0,0 +1,14 @@
+# Normalize line endings
+* text=auto eol=lf
+
+# Vendor: do not count toward language stats
+lib/Plutonium/** linguist-vendored=true
+deps/**          linguist-vendored=true
+
+# Binary assets
+*.png  binary
+*.jpg  binary
+*.nro  binary
+*.nso  binary
+*.pfs0 binary
+*.elf  binary
@@ -0,0 +1,434 @@
+# NXST Architectural Refactor Plan
+
+## Current Phase Tracker
+
+| Phase | Title | Status | Effort | Notes |
+|-------|-------|--------|--------|-------|
+| 0 | Tooling & ground rules | ✅ Done | S (~2h) | `.clang-format`, `.clang-tidy`, `.editorconfig`, `.gitattributes` |
+| 1 | Bug fixes & dead code | ✅ Done | S (~3h) | logger rewrite, `mkdir 0777`, RU comment, dead code |
+| 2 | File renames + `#pragma once` | ✅ Done | S (~2h) | snake_case filenames, unify guards |
+| 3 | Directory restructure | ☐ Not started | M (~1d) | `src/` + `include/nxst/` layered tree |
+| 4 | Make → CMake migration | ☐ Not started | M (~1d) | devkitpro `Switch.cmake` toolchain |
+| 5 | TransferService extraction | ☐ Not started | L (~2d) | kill globals, sever UI ↔ net coupling |
+| 6 | `Result<T>` + RAII | ☐ Not started | M (~1d) | tagged union, OS handle wrappers, split `restore()` |
+| 7 | Documentation + license | ☐ Not started | S (~half-day) | README, ARCHITECTURE, PROTOCOL, CHANGELOG, GPLv3 LICENSE |
+| 8 | CI | ☐ Not started | S (~2h) | GitHub Actions, `.nro` artifact, format check, layering check |
+
+**Active phase:** Phase 3 — Directory restructure.
+**Last updated:** 2026-04-26.
+
+Mark a phase `🟡 In progress` when starting and `✅ Done` when verified on hardware. Keep this table the source of truth.
+
+---
+
+## Context
+
+NXST is a Nintendo Switch homebrew save-transfer app (~3.2K LOC across 13 .cpp + 24 .hpp files, plus the vendored Plutonium UI submodule). The current codebase works but reads as a prototype:
+
+- Flat `source/` + mostly flat `include/` with mixed `PascalCase` and `lowercase` filenames.
+- Mixed identifier style (`recv_all` next to `isServerTransferDone`).
+- 11 headers use `#pragma once`, 9 use legacy `#ifndef X_HPP` guards.
+- **Severe layering violation**: UI layouts (`TitlesLayout::runTransfer`, `runReceive`) call into raw socket code and `io::restore` directly.
+- Globals: 6 in `server.cpp`, 4 in `client.cpp`, 2 in `io.cpp`, plus `g_currentUId` everywhere.
+- **Logger is silently broken** (`include/logger.hpp:54`): `printf(StringUtils::format(...).c_str(), args...)` already substitutes the format, then passes leftover variadic args to a string with no remaining `%` specifiers — every error log loses its arguments.
+- **Permission bug** in `source/io.cpp:119`: `mkdir(path.c_str(), 777)` — decimal 777 is octal 1411 (no read for owner). Has been wrong since day one.
+- One monster function: `io::restore` is 112 lines mixing mount, create, clear, copy, commit.
+- Russian comment in `io.cpp:231` (mixed languages).
+- Dead commented-out code in `util.cpp:50-58` and `logger.hpp:51-54`.
+- No `.clang-format`, no `.clang-tidy`, no `.editorconfig`, no `.gitattributes`, no CI, no `README.md`, no `ARCHITECTURE.md`, no `LICENSE` file (despite `io.cpp` carrying GPLv3 from Checkpoint).
+
+Goal: transform NXST into a layered, conventionally-named, tooled-up project that builds on Switch via CMake and is readable by a stranger in under ten minutes. Solo developer; bar is "professional indie", not enterprise.
+
+User decisions (already made):
+- **Full 8-phase plan**.
+- **Migrate Make → CMake** with devkitpro toolchain file.
+- **camelCase for functions, PascalCase for classes**, snake_case for files and locals, `kPascalCase` for constants.
+- **No tests** (Switch homebrew rarely has them; pure-logic surface is small).
+
+---
+
+## Target Directory Tree
+
+```
+NXST/
+├── .clang-format               # LLVM-derived, 4-space, 110 col
+├── .clang-tidy                 # bugprone-*, readability-*, modernize-*
+├── .editorconfig               # LF, UTF-8, trim WS, final newline
+├── .gitattributes              # eol=lf; lib/Plutonium linguist-vendored
+├── .github/workflows/build.yml # CI: devkitpro container, cmake, .nro upload
+├── docs/
+│   ├── ARCHITECTURE.md
+│   ├── PROTOCOL.md
+│   └── screenshots/
+├── cmake/
+│   └── modules/                # FindPlutonium.cmake (if needed)
+├── include/nxst/
+│   ├── app/                    # MainApplication, AppContext
+│   ├── domain/                 # Title, Account, TransferState, Result
+│   ├── infra/
+│   │   ├── fs/                 # filesystem, directory, RAII handles
+│   │   ├── net/                # socket, sendAll/recvAll
+│   │   └── sys/                # logger, threading, switch services
+│   ├── service/                # TransferService
+│   └── ui/                     # layouts, widgets, theme
+├── src/                        # mirrors include/nxst/
+│   ├── app/
+│   ├── domain/
+│   ├── infra/{fs,net,sys}/
+│   ├── service/
+│   ├── ui/
+│   └── main.cpp
+├── tools/format.sh             # one-line clang-format helper
+├── lib/Plutonium/              # submodule, untouched
+├── deps/asprintf/              # untouched
+├── CMakeLists.txt              # top-level
+├── README.md
+├── ARCHITECTURE.md             # → docs/ARCHITECTURE.md
+├── CHANGELOG.md
+├── LICENSE                     # GPLv3 (forced by Checkpoint inheritance)
+└── icon.png
+```
+
+---
+
+## Naming Convention Spec
+
+| Axis | Rule | Examples |
+|---|---|---|
+| Files | `snake_case.{cpp,hpp}` | `transfer_service.cpp`, `socket.hpp` |
+| Classes / structs | `PascalCase` | `TransferService`, `AccountProfile`, `Socket` |
+| Functions / methods | `camelCase` | `sendAll()`, `recvAll()`, `replaceUsername()` |
+| Locals / params | `snake_case` | `int sock_fd`, `std::string file_name` |
+| Members | `snake_case`, no prefix | `bytes_done`, `client_sock` |
+| Constants / constexpr | `kPascalCase` | `kTcpPort`, `kBufSize`, `kMulticastGroup` |
+| Enums | `enum class Foo { Bar, Baz }` | `enum class TransferKind { Send, Receive }` |
+| Macros (avoid) | `NXST_UPPER_SNAKE` | `NXST_UNREACHABLE` |
+| Namespaces | lowercase, short | `nxst`, `nxst::net`, `nxst::ui` |
+
+---
+
+## Layering (dependency direction)
+
+```
+                ┌────────────────────────┐
+                │   Presentation (ui/)   │
+                │   layouts, widgets     │
+                └───────────┬────────────┘
+                            │
+                ┌───────────▼────────────┐
+                │     Service (svc/)     │
+                │   TransferService      │
+                └───────────┬────────────┘
+                            │
+       ┌────────────────────┼────────────────────┐
+       │                    │                    │
+┌──────▼──────┐    ┌────────▼──────┐    ┌────────▼──────┐
+│  Domain     │    │   Infra/net   │    │   Infra/fs    │  Infra/sys
+│  Title,     │    │   Socket,     │    │   Directory,  │  Logger,
+│  Account,   │    │   sendAll,    │    │   FsHandle,   │  Thread,
+│  Result<T>  │    │   recvAll     │    │   FileHandle  │  Account svc
+└─────────────┘    └───────────────┘    └───────────────┘
+                            │
+                  libnx, SDL, Plutonium
+```
+
+**Hard rules.** `domain/` depends on nothing in the project. `infra/` depends only on `domain/`. `service/` depends on `domain/` + `infra/`. `ui/` depends only on `service/` + `domain/` — **must not include `<arpa/inet.h>`, `<sys/socket.h>`, `pthread.h`, or call `recv`/`send` directly**. `app/` wires everything.
+
+CI enforcement: a 20-line shell step greps `src/ui/**` for forbidden headers.
+
+---
+
+## Phase 0 — Tooling and ground rules (S, ~2h)
+
+**Goal.** Lay down conventions before touching code.
+
+**Files created.**
+- `.clang-format`: BasedOnStyle LLVM, IndentWidth 4, ColumnLimit 110, PointerAlignment Left, IncludeBlocks Regroup with three categories (system, third-party, project).
+- `.clang-tidy`: enable `bugprone-*`, `readability-*` (minus `magic-numbers`), `modernize-*` (minus `use-trailing-return-type`, `use-nodiscard`), `cppcoreguidelines-pro-type-cstyle-cast`. Disable `fuchsia-*`, `google-*`, `llvm-header-guard`.
+- `.editorconfig`: LF, UTF-8, 4-space, trim trailing whitespace, final newline. Tab indent for `Makefile` (only while it still exists).
+- `.gitattributes`: `* text=auto eol=lf`, `lib/Plutonium/** linguist-vendored=true`, `deps/** linguist-vendored=true`. Stops GitHub mis-classifying the repo language.
+- `tools/format.sh`: `clang-format -i` over `src/` + `include/`.
+
+**Risk.** None — no compiled changes.
+
+**Buildable?** Unaffected.
+
+---
+
+## Phase 1 — Bug fixes and dead code (S, ~3h)
+
+**Goal.** Fix concrete bugs before any structural change can mask them.
+
+**Tasks.**
+1. Replace broken `Logger` (`include/logger.hpp`) with free-function API in `include/nxst/infra/sys/log.hpp`:
+   ```cpp
+   namespace nxst::log {
+       enum class Level { Debug, Info, Warn, Error };
+       void write(Level, const char* fmt, ...) __attribute__((format(printf, 2, 3)));
+       void debug(const char* fmt, ...) __attribute__((format(printf, 1, 2)));
+       void info (const char* fmt, ...) __attribute__((format(printf, 1, 2)));
+       void warn (const char* fmt, ...) __attribute__((format(printf, 1, 2)));
+       void error(const char* fmt, ...) __attribute__((format(printf, 1, 2)));
+   }
+   ```
+   Implementation: `vsnprintf` into stack buffer, prefix with `[YYYY-MM-DD HH:MM:SS][LEVEL]`, append to `/switch/NXST/log.log` and stderr. TU-local `std::mutex`. No singleton class. Drop the GPLv3 Checkpoint header (the new file is original).
+   - `format(printf, ...)` attribute gives compile-time format-string checking — that is the actual reason this rewrite matters.
+   - Leave thin shim `LOG_INFO(...)`/`LOG_ERROR(...)` macros so existing call sites keep compiling during phase migration.
+2. Fix `source/io.cpp:119` — `mkdir(path.c_str(), 777)` → `mkdir(path.c_str(), 0777)`.
+3. Translate Russian comment at `source/io.cpp:231` to English.
+4. Delete dead commented-out code in `source/util.cpp:50-58` and old `include/logger.hpp:51-54` block.
+
+**Risk.** Logger API change ripples to <30 call sites. Macros bridge the gap.
+
+**Buildable?** Yes after each file.
+
+---
+
+## Phase 2 — File renames + `#pragma once` (S, ~2h)
+
+**Goal.** Land the filename convention before the directory move so git history sees rename + move as separate commits.
+
+**Tasks.**
+- Rename to `snake_case` via `git mv` (use two-step on case-insensitive macOS): `Main.cpp`→`main.cpp`, `MainApplication.{cpp,hpp}`→`main_application.{cpp,hpp}`, `TitlesLayout.{cpp,hpp}`→`titles_layout.{cpp,hpp}`, `UsersLayout.{cpp,hpp}`→`users_layout.{cpp,hpp}`, plus matching headers under `include/`.
+- Convert all `#ifndef X_HPP / #define X_HPP / #endif` guards to `#pragma once`. Affected: `logger.hpp`, `account.hpp`, and the 7 other headers using legacy guards.
+- Update affected `#include` lines.
+- `Makefile` `SOURCES` globs `*.cpp`, so no Makefile change needed for renames.
+
+**Risk.** macOS case-insensitive FS hides case-only renames — use `git mv Foo.cpp tmp.cpp && git mv tmp.cpp foo.cpp`.
+
+**Buildable?** Yes after each pair.
+
+---
+
+## Phase 3 — Directory restructure (M, ~1 day)
+
+**Goal.** Move flat `source/` and `include/` into the layered tree. **No code changes**, just relocation + include-path updates.
+
+**Tasks.**
+1. Create the `src/{app,domain,infra/{net,fs,sys},service,ui}` and `include/nxst/{...}` skeleton.
+2. Move files:
+   - `protocol.hpp`, `transfer_state.hpp`, `account.hpp` (the `AccountUid` struct + ordering only), `title.hpp/cpp`, `common.hpp/cpp`, `util.hpp/cpp` → `domain/`.
+   - `client.{hpp,cpp}` → `infra/net/transfer_sender.{hpp,cpp}`, `server.{hpp,cpp}` → `infra/net/transfer_receiver.{hpp,cpp}`. Renaming is intentional: `CLAUDE.md` already documents that "server = receiver, client = sender" is inverted from typical usage; renaming kills the confusion permanently.
+   - `net/socket.hpp` → `infra/net/socket.hpp`.
+   - `io.hpp/cpp`, `directory.hpp/cpp`, `filesystem.hpp/cpp` → `infra/fs/`.
+   - new `log.hpp/cpp`, switch-service init pieces from `account.cpp` → `infra/sys/`.
+   - `main_application.{hpp,cpp}`, `main.{hpp,cpp}` → `app/`.
+   - `titles_layout.*`, `users_layout.*`, `transfer_overlay.hpp`, `theme.hpp`, `ui/*.hpp`, `header_bar.hpp` → `ui/`.
+3. Update `Makefile` (still in use — CMake migration is Phase 4):
+   ```make
+   SOURCES  := src src/app src/domain src/infra/net src/infra/fs src/infra/sys src/service src/ui lib/Plutonium/source
+   INCLUDES := include lib/Plutonium/include
+   ```
+   Remove the now-defunct `include/net` from `INCLUDES`.
+4. Update **every** `#include` to `#include <nxst/...>` form: `#include <nxst/domain/protocol.hpp>`, `#include <nxst/infra/net/socket.hpp>`. The single root `include/` keeps the prefix mandatory and grepable.
+
+**Risk.** Big disruptive phase. Do on a feature branch, build after every directory's worth of moves. `service/` directory stays empty for now — scaffolding only.
+
+**Buildable?** Yes if you move-and-rebuild incrementally.
+
+---
+
+## Phase 4 — Migrate Make → CMake (M, ~1 day)
+
+**Goal.** Replace `Makefile` with `CMakeLists.txt` using devkitpro's bundled `Switch.cmake` toolchain file. Project compiles via `cmake -B build -DCMAKE_TOOLCHAIN_FILE=$DEVKITPRO/cmake/Switch.cmake && cmake --build build`.
+
+**Tasks.**
+1. Create top-level `CMakeLists.txt`:
+   - `cmake_minimum_required(VERSION 3.20)`, `project(NXST CXX)`.
+   - C++17, `-fno-rtti -fno-exceptions` (matches current `CXXFLAGS`).
+   - ARM flags `-march=armv8-a+crc+crypto -mtune=cortex-a57 -mtp=soft -fPIE` mirror current `ARCH`.
+   - `find_package(PkgConfig REQUIRED)` + `pkg_check_modules(SWITCH_LIBS REQUIRED SDL2_ttf SDL2_gfx SDL2_image SDL2_mixer freetype2 harfbuzz minizip libpng libjpeg libwebp glesv2 egl glapi zlib)` against `aarch64-none-elf-pkg-config` (already wired up per memory ID 101).
+   - Manual link: `pu` (Plutonium), `drm_nouveau`, plus the trailing `-lharfbuzz -lfreetype -lz` static-link-order workaround (memory IDs 97, 98, 100 — required after recent libnx update).
+   - Add Plutonium as `add_subdirectory(lib/Plutonium)` if its CMake exists, else as an INTERFACE library wrapping its prebuilt `.a`. Inspection during this phase decides.
+   - Use devkitpro's `nx_create_nro` / `nx_generate_nacp` helpers from the toolchain file to emit `NXST.nro` with icon + NACP metadata (`APP_TITLE=NXST`, `APP_AUTHOR=DragonSpirit`, `APP_VERSION=04.26.2026`).
+2. Create `cmake/modules/` if any custom Find module proves necessary (likely not).
+3. Add convenience targets: `cmake --build build --target send` to invoke `nxlink NXST.nro`.
+4. Delete old `Makefile` only after CMake build produces a working `.nro` matching the Make output.
+5. Update `.gitignore`: replace `build/` Make artifacts with CMake's `build/` directory rules and `CMakeCache.txt`, `CMakeFiles/`, `compile_commands.json`.
+
+**Risk.** Highest in the plan. Mitigation:
+- Keep `Makefile` alongside CMake during the phase. Verify `.nro` from both is identical (or at minimum boots and runs on hardware).
+- If the devkitpro `Switch.cmake` toolchain proves brittle for Plutonium's transitive headers, fall back to keeping Make and document why CMake was deferred.
+- The pkg-config tweaks captured in memory IDs 96–101 are non-obvious — re-apply them in the CMake config explicitly, not via `pkg_check_modules` defaults.
+
+**Buildable?** Yes — both build systems coexist until CMake is verified.
+
+---
+
+## Phase 5 — TransferService extraction, kill globals (L, ~2 days)
+
+**Goal.** Sever the UI → net coupling. Single biggest "this looks like real software" change.
+
+**Tasks.**
+1. Create `src/service/transfer_service.{hpp,cpp}`. Class owns:
+   - one `TransferState` per direction (sender, receiver),
+   - all listen / accept / broadcast threads (currently 6 globals in `server.cpp`, 4 in `client.cpp`),
+   - public API: `start(TransferKind, AccountUid, std::vector<TitleId>)`, `cancel()`, `progress()`, `statusText()`, `isDone()`, `failureReason()`, `setOnComplete(std::function<void(TransferResult)>)`.
+2. Move file globals (`g_server_state`, `g_client_state`, all socket/pthread atomics) into `TransferService` private members. pthread C trampolines: `static void* threadEntry(void* self)` immediately calls `*static_cast<TransferService*>(self)`.
+3. Refactor `titles_layout.cpp` `runTransfer`/`runReceive` to call `app_ctx.transfer.start(...)` / `cancel()` instead of free `transfer_files()` / `start_listening()`. Inject via `AppContext&` (already partially present per `include/ui/UiContext.hpp`).
+4. Move `g_currentUId` out of global scope into `AppContext::current_user`. Pass `AppContext&` down to layouts at construction. There is exactly one `AppContext` and its lifetime equals the app — safe and cheap.
+5. UI's `runReceive` currently calls `io::restore` directly after server work completes. Move into the service's completion callback so UI never touches FS directly.
+
+**Strategy.** Keep old `transfer_files()` / `start_listening()` free functions as thin wrappers around `TransferService` for the duration of the phase. Convert UI call sites one at a time. Delete wrappers only after both sites are on the service.
+
+**Buildable?** Yes via the wrappers.
+
+---
+
+## Phase 6 — `Result<T>` and RAII (M, ~1 day)
+
+**Goal.** Replace bool/out-param/silent-failure patterns with `Result<T>`; wrap raw OS handles in RAII.
+
+**Tasks.**
+1. `include/nxst/domain/result.hpp` — minimal `Result<T, E=std::string>` with tagged union (no `std::variant` because `-fno-exceptions` rules out monostate-on-throw):
+   ```cpp
+   template <class T, class E = std::string>
+   class Result {
+       bool ok_;
+       union { T val_; E err_; };
+   public:
+       static Result success(T v);
+       static Result failure(E e);
+       bool isOk() const noexcept;
+       const T& value() const;     // UB if !isOk
+       const E& error() const;     // UB if isOk
+       template <class F> auto map(F&&) const;
+   };
+   ```
+   Resist importing `tl::expected`. 60 lines of in-house code beats a vendored dep at this scale.
+2. RAII wrappers in `include/nxst/infra/`:
+   - `FdHandle` — owns `int fd`, closes on dtor, move-only.
+   - `FsFileSystemHandle` — owns `FsFileSystem`, calls `fsFsClose`. Plugs the "save"-mount leak from memory ID 59.
+   - `AccountProfileHandle` — owns `AccountProfile`, calls `accountProfileClose`. Plugs the bug from memory ID 64.
+   - `FileHandle` — owns `FILE*`, `fclose` on dtor.
+3. Replace `io.cpp:57` `new u8[BUFFER_SIZE] / delete[]` and `io.cpp:234` `malloc/free` with `std::vector<u8>` (already the pattern in `client.cpp`/`server.cpp`).
+4. Split `io::restore` (the 112-line monster, `io.cpp:221-332`) into `restoreSaveForTitle`, `extractAndCommit`, `clearStaleSaveFiles`. Each returns `Result<void>`.
+5. Convert `recvAll` / `sendAll` / socket setup paths in `infra/net` to return `Result<void>` so error paths compose under `-fno-exceptions`.
+
+**Strategy.** Adopt incrementally — start at IO boundaries, leave `bool` returns untouched in modules not yet refactored.
+
+**Buildable?** Yes per file.
+
+---
+
+## Phase 7 — Documentation + license (S, ~half day)
+
+**Goal.** A stranger can build, run, and understand this in under 15 minutes.
+
+**Files created.**
+- `README.md`: what NXST does, screenshot, install (drop `.nro` into `/switch/`), build (devkitpro + `cmake --build`), credit Plutonium and Checkpoint, link to `docs/`.
+- `docs/ARCHITECTURE.md`: layering diagram from this plan, threading model (one accept thread, one broadcast thread, one transfer thread per direction; cancellation via `shutdown(2)` + atomic flag), pointers to key files.
+- `docs/PROTOCOL.md`: promote the 9-line wire-format comment from `protocol.hpp` into a real spec — byte-level diagrams, multicast discovery on `239.0.0.1:8081`, TCP transfer on `:8080`, EOF sentinel = `filename_len == 0`, `kBufSize=65536`, `kMaxFilename=4096`.
+- `CHANGELOG.md`: Keep-a-Changelog format. Version `0.x` until protocol stabilizes.
+- `LICENSE`: GPLv3 — forced by `io.cpp` carrying Checkpoint's GPLv3 header. Document credit + license inheritance in README.
+
+**Risk.** None.
+
+**Buildable?** Unaffected.
+
+---
+
+## Phase 8 — CI (S, ~2h)
+
+**Goal.** Every push verifies `.nro` builds. One green badge tells visitors the repo is alive.
+
+**Tasks.**
+- `.github/workflows/build.yml`:
+  ```yaml
+  on: [push, pull_request]
+  jobs:
+    nro:
+      runs-on: ubuntu-latest
+      container: devkitpro/devkita64:latest
+      steps:
+        - uses: actions/checkout@v4
+          with: { submodules: recursive }
+        - run: |
+            cmake -B build \
+              -DCMAKE_TOOLCHAIN_FILE=$DEVKITPRO/cmake/Switch.cmake \
+              -DCMAKE_BUILD_TYPE=Release
+            cmake --build build -j$(nproc)
+        - uses: actions/upload-artifact@v4
+          with:
+            name: NXST-${{ github.sha }}
+            path: build/NXST.nro
+    format:
+      runs-on: ubuntu-latest
+      steps:
+        - uses: actions/checkout@v4
+        - run: |
+            sudo apt-get install -y clang-format
+            find src include -name '*.cpp' -o -name '*.hpp' \
+              | xargs clang-format --dry-run --Werror
+    layering:
+      runs-on: ubuntu-latest
+      steps:
+        - uses: actions/checkout@v4
+        - run: |
+            ! grep -rE '^#include\s*[<"](arpa/|sys/socket|pthread\.h)' src/ui/
+  ```
+- Optional release job on tag `v*`: same CMake build, attach `.nro` to GitHub Release.
+
+**Risk.** First run typically fails on submodule init or container path — iterate on a branch.
+
+**Buildable?** Pass or fix.
+
+---
+
+## Critical files to modify
+
+- `Makefile` — Phase 3 SOURCES/INCLUDES update; deleted in Phase 4 after CMake parity.
+- `include/logger.hpp` — Phase 1 replace with `nxst::log` free-function API.
+- `source/io.cpp` — Phase 1 mkdir + comment fix; Phase 6 split `restore()` + RAII; Phase 5 stop being called from UI.
+- `source/server.cpp` — Phase 3 rename to `transfer_receiver.cpp`; Phase 5 globals migrate into `TransferService`.
+- `source/client.cpp` — Phase 3 rename to `transfer_sender.cpp`; Phase 5 globals migrate into `TransferService`.
+- `source/TitlesLayout.cpp` — Phase 2 rename; Phase 5 sever direct calls into network and `io::restore`.
+- `source/Main.cpp` — Phase 2 rename; Phase 5 construct `AppContext` with `TransferService`.
+- `source/util.cpp` — Phase 1 delete dead block.
+
+New files (not yet present):
+- `.clang-format`, `.clang-tidy`, `.editorconfig`, `.gitattributes` — Phase 0.
+- `CMakeLists.txt` — Phase 4.
+- `include/nxst/domain/result.hpp` — Phase 6.
+- `include/nxst/service/transfer_service.hpp` + `src/service/transfer_service.cpp` — Phase 5.
+- `README.md`, `docs/ARCHITECTURE.md`, `docs/PROTOCOL.md`, `CHANGELOG.md`, `LICENSE` — Phase 7.
+- `.github/workflows/build.yml` — Phase 8.
+
+---
+
+## What is deliberately NOT in this plan
+
+1. **C++20.** Devkitpro's `gnu++17` is fine; nothing in the codebase wants concepts/ranges/coroutines, and coroutines under `-fno-exceptions` are a maze. Stay on `gnu++17`.
+2. **Abstracting Plutonium.** It is *the* UI framework; you will not port it. UI calls Plutonium directly.
+3. **`tl::expected` / `std::expected`.** 60-line in-house `Result<T>` is enough.
+4. **DI containers, factories, abstract logger interfaces.** Solo developer, one service, one logger sink. Add complexity only when a second sink actually appears.
+5. **Integration tests against the network.** No bug class is meaningfully prevented; cost is high; value comes from running on real hardware.
+6. **Migrating from pthread to `std::thread`.** Libnx's pthread is the supported path. A single `Thread` RAII wrapper inside `infra/sys/` is the most you should add.
+7. **PImpl broadly.** At 2.3 KLOC compile time is sub-second; PImpl just adds indirection.
+8. **Re-vendoring or modifying Plutonium.** Submodule, untouched. `linguist-vendored` keeps it out of GitHub language stats.
+9. **UPPER_SNAKE constants.** Visually clash with macros; `kPascalCase` is the modern norm.
+
+---
+
+## Recommended phasing for a solo developer
+
+- **Weekend 1:** Phase 0 + 1 + 2. Tooled, bugs fixed, files renamed, builds. Already feels different.
+- **Weekend 2:** Phase 3. Big move, one sitting on a branch.
+- **Weekend 3:** Phase 4 (CMake). Build system swap. Highest individual phase risk — schedule when fresh.
+- **Weekend 4 (longest):** Phase 5 (TransferService). UI no longer touches sockets. The win.
+- **Weekend 5:** Phase 6. `Result<T>`, RAII, `restore()` split.
+- **Evening:** Phase 7. README + ARCHITECTURE + LICENSE.
+- **Evening:** Phase 8. CI.
+
+Stopping after Weekend 4 still leaves the project visibly more serious than it started. Each weekend produces a defensible "ship it" state.
+
+---
+
+## Verification (end-to-end test plan)
+
+After each phase:
+1. `cmake --build build` (Phase 4+) or `make` produces `NXST.nro` with no warnings.
+2. Copy `NXST.nro` to Switch SD card `/switch/NXST/`, launch via hbmenu, verify the title list and user list render (regression tests for memory IDs 73, 78).
+3. Run a save transfer between two Switches: select a title on the sender, "Receive" on the receiver, verify the file lands and `io::restore` replaces it on the receiver. Verify cancel works mid-transfer (regression for memory IDs 60, 103, 104).
+4. Verify the log file at `/switch/NXST/log.log` contains formatted entries with timestamps and levels (regression for the broken-Logger bug).
+5. Phase 8 only: confirm a green CI run on a PR; download the `NXST.nro` artifact; install it on Switch; run the same flow.
@@ -24,9 +24,7 @@
 *         reasonable ways as different from the original version.
 */

-#ifndef ACCOUNT_HPP
-#define ACCOUNT_HPP
-
+#pragma once
 #include <map>
 #include <string.h>
 #include <string>
@@ -73,4 +71,3 @@ namespace Account {
    std::string iconPath(AccountUid id);
 }

-#endif
@@ -24,9 +24,7 @@
 *         reasonable ways as different from the original version.
 */

-#ifndef COMMON_HPP
-#define COMMON_HPP
-
+#pragma once
 #include <algorithm>
 #include <arpa/inet.h>
 #include <codecvt>
@@ -62,4 +60,3 @@ namespace StringUtils {

 char* getConsoleIP(void);

-#endif
@@ -1,6 +1,6 @@
 #pragma once

-#include <Theme.hpp>
+#include <theme.hpp>

 #define COLOR(hex) pu::ui::Color::FromHex(hex)
 #define BACKGROUND_COLOR theme::color::BgBase
@@ -24,9 +24,7 @@
 *         reasonable ways as different from the original version.
 */

-#ifndef DIRECTORY_HPP
-#define DIRECTORY_HPP
-
+#pragma once
 #include <dirent.h>
 #include <errno.h>
 #include <string>
@@ -55,4 +53,3 @@ private:
    bool mGood;
 };

-#endif
@@ -24,9 +24,7 @@
 *         reasonable ways as different from the original version.
 */

-#ifndef FILESYSTEM_HPP
-#define FILESYSTEM_HPP
-
+#pragma once
 #include "account.hpp"
 #include <switch.h>

@@ -36,4 +34,3 @@ namespace FileSystem {
    void unmount(void);
 }

-#endif
@@ -24,9 +24,7 @@
 *         reasonable ways as different from the original version.
 */

-#ifndef IO_HPP
-#define IO_HPP
-
+#pragma once
 #include "account.hpp"
 #include "directory.hpp"
 #include "title.hpp"
@@ -52,4 +50,3 @@ namespace io {
    bool fileExists(const std::string& path);
 }

-#endif
@@ -1,85 +1,55 @@
-/*
- *   This file is part of Checkpoint
- *   Copyright (C) 2017-2021 Bernardo Giordano, FlagBrew
- *
- *   This program is free software: you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License as published by
- *   the Free Software Foundation, either version 3 of the License, or
- *   (at your option) any later version.
- *
- *   This program is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY; without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *   GNU General Public License for more details.
- *
- *   You should have received a copy of the GNU General Public License
- *   along with this program.  If not, see <http://www.gnu.org/licenses/>.
- *
- *   Additional Terms 7.b and 7.c of GPLv3 apply to this file:
- *       * Requiring preservation of specified reasonable legal notices or
- *         author attributions in that material or in the Appropriate Legal
- *         Notices displayed by works containing it.
- *       * Prohibiting misrepresentation of the origin of that material,
- *         or requiring that modified versions of such material be marked in
- *         reasonable ways as different from the original version.
- */
+#pragma once

-#ifndef LOGGER_HPP
-#define LOGGER_HPP
-
-#include "common.hpp"
-
-#include <stdio.h>
+#include <cstring>
 #include <string>

-class Logger {
-public:
-    static Logger& getInstance(void)
+// New API — use these going forward.
+namespace nxst::log {
+
+enum class Level { Debug, Info, Warn, Error };
+
+void write(Level level, const char* fmt, ...) __attribute__((format(printf, 2, 3)));
+void debug(const char* fmt, ...)             __attribute__((format(printf, 1, 2)));
+void info (const char* fmt, ...)             __attribute__((format(printf, 1, 2)));
+void warn (const char* fmt, ...)             __attribute__((format(printf, 1, 2)));
+void error(const char* fmt, ...)             __attribute__((format(printf, 1, 2)));
+
+// No-op: writes are immediate. Kept for source compatibility during migration.
+inline void flush() {}
+
+} // namespace nxst::log
+
+// Backward-compat shim — existing Logger::getInstance().log(...) call sites compile
+// unchanged. Format args are dropped (same behavior as broken original). Migrate
+// call sites to nxst::log::* in Phase 3.
+struct Logger {
+    static Logger& getInstance()
    {
-        static Logger mLogger;
-        return mLogger;
+        static Logger instance;
+        return instance;
    }

-    inline static const std::string INFO  = "[INFO]";
-    inline static const std::string DEBUG = "[DEBUG]";
-    inline static const std::string ERROR = "[ERROR]";
-    inline static const std::string WARN  = "[WARN]";
+    // clang-tidy naming suppressed: these must match existing call sites during migration.
+    static constexpr const char* INFO  = "[INFO]";   // NOLINT(readability-identifier-naming)
+    static constexpr const char* DEBUG = "[DEBUG]";  // NOLINT(readability-identifier-naming)
+    static constexpr const char* ERROR = "[ERROR]";  // NOLINT(readability-identifier-naming)
+    static constexpr const char* WARN  = "[WARN]";   // NOLINT(readability-identifier-naming)

+    static void flush() { nxst::log::flush(); }
+
+    // Args intentionally dropped — format string still logged for visibility.
    template <typename... Args>
-    void log(const std::string& level, const std::string& format = {}, Args... args)
+    void log(const std::string& level, const std::string& fmt, Args&&... /*args*/)
    {
-        // buffer += StringUtils::format(("[" + DateTime::logDateTime() + "] " + level + " " + format + "\n").c_str(), args...);
-        // buffer += StringUtils::format("%s\n", format.c_str());
-        // buffer += StringUtils::format("%s\n",StringUtils::format("[" + DateTime::logDateTime() + "] " + level + " " + format + "\n").c_str(), args...);
-        printf(StringUtils::format("[" + DateTime::logDateTime() + "] " + level + " " + format + "\n").c_str(), args...);
+        if (level == ERROR)      nxst::log::error("%s", fmt.c_str());
+        else if (level == WARN)  nxst::log::warn("%s", fmt.c_str());
+        else if (level == DEBUG) nxst::log::debug("%s", fmt.c_str());
+        else                     nxst::log::info("%s", fmt.c_str());
    }

-    void flush(void)
-    {
-        mFile = fopen(mPath.c_str(), "a");
-        if (mFile != NULL) {
-            fprintf(mFile, buffer.c_str());
-            fprintf(stderr, buffer.c_str());
-            fclose(mFile);
-        }
-    }
+    Logger()  = default;
+    ~Logger() = default;

-private:
-    Logger(void) { buffer = ""; }
-    ~Logger(void) {}
-
-    Logger(Logger const&) = delete;
-    void operator=(Logger const&) = delete;
-
-#if defined(__SWITCH__)
-    const std::string mPath = "/switch/NXST/log.log";
-#else
-    const std::string mPath = "log.log";
-#endif
-
-    FILE* mFile;
-
-    std::string buffer;
+    Logger(const Logger&)            = delete;  // NOLINT(modernize-use-equals-delete)
+    Logger& operator=(const Logger&) = delete;  // NOLINT(modernize-use-equals-delete)
 };
-
-#endif
@@ -1,5 +1,4 @@
-#ifndef MAIN_HPP
-#define MAIN_HPP
+#pragma once
 #include <const.h>
 #include "account.hpp"
 #include "title.hpp"
@@ -23,4 +22,3 @@ inline std::string g_currentFile = "";
 inline bool g_isTransferringFile = false;
 inline const std::string g_emptySave = "New...";

-#endif
@@ -1,8 +1,8 @@
 #pragma once

 #include <pu/Plutonium>
-#include <UsersLayout.hpp>
-#include <TitlesLayout.hpp>
+#include <users_layout.hpp>
+#include <titles_layout.hpp>

 namespace ui {

@@ -24,9 +24,7 @@
 *         reasonable ways as different from the original version.
 */

-#ifndef TITLE_HPP
-#define TITLE_HPP
-
+#pragma once
 #include "account.hpp"
 #include "filesystem.hpp"
 #include "io.hpp"
@@ -88,4 +86,3 @@ void rotateSortMode(void);
 void refreshDirectories(u64 id);
 std::unordered_map<std::string, std::string> getCompleteTitleList(void);

-#endif
@@ -5,8 +5,8 @@
 #include <unordered_map>
 #include <vector>
 #include <memory>
-#include <ui/HeaderBar.hpp>
-#include <ui/HintBar.hpp>
+#include <ui/header_bar.hpp>
+#include <ui/hint_bar.hpp>

 namespace ui {

@@ -1,6 +1,6 @@
 #pragma once
 #include <pu/Plutonium>
-#include <Theme.hpp>
+#include <theme.hpp>
 #include <util.hpp>

 namespace ui {
@@ -1,6 +1,6 @@
 #pragma once
 #include <pu/Plutonium>
-#include <Theme.hpp>
+#include <theme.hpp>

 namespace ui {

@@ -1,7 +1,7 @@
 #pragma once
 #include <pu/Plutonium>
-#include <Theme.hpp>
-#include <ui/UiContext.hpp>
+#include <theme.hpp>
+#include <ui/ui_context.hpp>
 #include <account.hpp>

 namespace ui {
@@ -1,6 +1,6 @@
 #pragma once
 #include <pu/Plutonium>
-#include <Theme.hpp>
+#include <theme.hpp>
 #include <vector>
 #include <string>

@@ -1,7 +1,7 @@
 #include <pu/Plutonium>
 #include <const.h>
-#include <ui/HeaderBar.hpp>
-#include <ui/HintBar.hpp>
+#include <ui/header_bar.hpp>
+#include <ui/hint_bar.hpp>
 #include <memory>

 namespace ui {
@@ -24,9 +24,7 @@
 *         reasonable ways as different from the original version.
 */

-#ifndef UTIL_HPP
-#define UTIL_HPP
-
+#pragma once
 #include "account.hpp"
 #include "common.hpp"
 #include "io.hpp"
@@ -50,4 +48,4 @@ namespace StringUtils {
    std::string elide(const std::string& s, size_t maxChars);
 }

-#endif
+
@@ -19,7 +19,7 @@
 #endif

 #include <protocol.hpp>
-#include <TransferState.hpp>
+#include <transfer_state.hpp>

 namespace fs = std::filesystem;
 using path = fs::path;
@@ -116,7 +116,7 @@ Result io::copyDirectory(const std::string& srcPath, const std::string& dstPath)

 Result io::createDirectory(const std::string& path)
 {
-    mkdir(path.c_str(), 777);
+    mkdir(path.c_str(), 0777);
    return 0;
 }

@@ -228,8 +228,8 @@ std::tuple<bool, Result, std::string> io::restore(size_t index, AccountUid uid,
    Logger::getInstance().log(Logger::INFO, "Started restore of %s. Title id: 0x%016lX; User id: 0x%lX%lX.", title.name().c_str(), title.id(),
        title.userId().uid[1], title.userId().uid[0]);

-    // Если сейв ещё не существует (игра не запускалась) — создаём его через NACP.
-    // fsCreateSaveDataFileSystem возвращает ошибку если сейв уже есть — это нормально.
+    // If save data does not yet exist (game was never launched), create it via NACP.
+    // fsCreateSaveDataFileSystem returns an error if the save already exists — this is expected.
    {
        NsApplicationControlData* nsacd = (NsApplicationControlData*)malloc(sizeof(NsApplicationControlData));
        if (nsacd != NULL) {
@@ -0,0 +1,64 @@
+#include <logger.hpp>
+
+#include <cstdarg>
+#include <cstdio>
+#include <ctime>
+#include <mutex>
+
+namespace {
+
+std::mutex g_log_mutex;
+
+#if defined(__SWITCH__)
+constexpr const char* kLogPath = "/switch/NXST/log.log";
+#else
+constexpr const char* kLogPath = "nxst.log";
+#endif
+
+void writeEntry(const char* tag, const char* fmt, va_list args)
+{
+    char msg[2048];
+    vsnprintf(msg, sizeof(msg), fmt, args);
+
+    time_t now = time(nullptr);
+    struct tm tm_buf;
+    localtime_r(&now, &tm_buf);
+    char time_str[16];
+    strftime(time_str, sizeof(time_str), "%H:%M:%S", &tm_buf);
+
+    std::lock_guard<std::mutex> lock(g_log_mutex);
+
+    fprintf(stderr, "[%s]%s %s\n", time_str, tag, msg);
+
+    FILE* log_file = fopen(kLogPath, "a");
+    if (log_file != nullptr) {
+        fprintf(log_file, "[%s]%s %s\n", time_str, tag, msg);
+        fclose(log_file);
+    }
+}
+
+} // namespace
+
+namespace nxst::log {
+
+void write(Level level, const char* fmt, ...)
+{
+    const char* tag = "[INFO] ";
+    switch (level) {
+        case Level::Debug: tag = "[DEBUG]"; break;
+        case Level::Info:  tag = "[INFO] "; break;
+        case Level::Warn:  tag = "[WARN] "; break;
+        case Level::Error: tag = "[ERROR]"; break;
+    }
+    va_list args;
+    va_start(args, fmt);
+    writeEntry(tag, fmt, args);
+    va_end(args);
+}
+
+void debug(const char* fmt, ...) { va_list args; va_start(args, fmt); writeEntry("[DEBUG]", fmt, args); va_end(args); }
+void info (const char* fmt, ...) { va_list args; va_start(args, fmt); writeEntry("[INFO] ", fmt, args); va_end(args); }
+void warn (const char* fmt, ...) { va_list args; va_start(args, fmt); writeEntry("[WARN] ", fmt, args); va_end(args); }
+void error(const char* fmt, ...) { va_list args; va_start(args, fmt); writeEntry("[ERROR]", fmt, args); va_end(args); }
+
+} // namespace nxst::log
@@ -1,4 +1,4 @@
-#include <MainApplication.hpp>
+#include <main_application.hpp>
 #include "util.hpp"
 #include "main.hpp"
 #include <server.hpp>
@@ -2,7 +2,7 @@
 #include <switch.h>
 #include <switch/services/hid.h>
 #include <vector>
-#include <MainApplication.hpp>
+#include <main_application.hpp>

 namespace ui {
    MainApplication *mainApp;
@@ -19,8 +19,8 @@
 #endif

 #include <protocol.hpp>
-#include <TransferState.hpp>
-#include <net/Socket.hpp>
+#include <transfer_state.hpp>
+#include <net/socket.hpp>

 static TransferState    g_server_state;
 static std::atomic<int> g_server_client_sock{-1};
@@ -1,10 +1,10 @@
-#include <MainApplication.hpp>
+#include <main_application.hpp>
 #include <stdio.h>
 #include <main.hpp>
 #include <const.h>
 #include <client.hpp>
 #include <server.hpp>
-#include <TransferOverlay.hpp>
+#include <transfer_overlay.hpp>

 namespace ui {
    extern MainApplication *mainApp;
@@ -1,5 +1,5 @@
 #include <cstdio>
-#include <MainApplication.hpp>
+#include <main_application.hpp>
 #include "main.hpp"

 namespace ui {
@@ -26,7 +26,7 @@

 #include "util.hpp"
 #include <logger.hpp>
-#include <MainApplication.hpp>
+#include <main_application.hpp>
 #include "main.hpp"

 void servicesExit(void)
@@ -47,16 +47,6 @@ Result servicesInit(void)
        Logger::getInstance().log(Logger::WARN, "Please do not run NXST in applet mode.");
    }

-    // Result socinit = 0;
-    // if ((socinit = socketInitializeDefault()) == 0) {
-    //     nxlinkStdio();
-    // }
-    // else {
-    //     Logger::getInstance().log(Logger::INFO, "Unable to initialize socket. Result code 0x%08lX.", socinit);
-    // }  
-
-    // g_shouldExitNetworkLoop = R_FAILED(socinit); 
-
    Result res = 0;

    romfsInit();
@@ -0,0 +1,6 @@
+#!/usr/bin/env bash
+set -euo pipefail
+cd "$(dirname "$0")/.."
+
+find src include -name '*.cpp' -o -name '*.hpp' | xargs clang-format -i
+echo "Formatted."