Files
NXST/PLAN.md
T
DragonSpirit 7bd6a90bde
CI / Build NRO (push) Has been cancelled
CI / Upload release asset (push) Has been cancelled
CI / Format check (push) Has been cancelled
CI / Layering check (push) Has been cancelled
finish refactor, add docs and CI
2026-04-27 01:49:41 +03:00

25 KiB
Raw Blame History

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 Done M (~1d) src/ + include/nxst/ layered tree
4 Make → CMake migration Done M (~1d) devkitpro Switch.cmake toolchain
5 TransferService extraction Done L (~2d) kill globals, sever UI ↔ net coupling
6 Result<T> + RAII Done M (~1d) tagged union, OS handle wrappers, fix raw memory
7 Documentation + license Done S (~half-day) README, ARCHITECTURE, PROTOCOL, CHANGELOG, GPLv3 LICENSE
8 CI Done S (~2h) GitHub Actions, .nro artifact, format check, layering check

Active phase: — All phases complete. Last updated: 2026-04-27.

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:
    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:119mkdir(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.cppmain.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/cppdomain/.
    • 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.hppinfra/net/socket.hpp.
    • io.hpp/cpp, directory.hpp/cpp, filesystem.hpp/cppinfra/fs/.
    • new log.hpp/cpp, switch-service init pieces from account.cppinfra/sys/.
    • main_application.{hpp,cpp}, main.{hpp,cpp}app/.
    • titles_layout.*, users_layout.*, transfer_overlay.hpp, theme.hpp, ui/*.hpp, header_bar.hppui/.
  3. Update Makefile (still in use — CMake migration is Phase 4):
    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 96101 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):
    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:
    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.

  • 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.