feat: extract NosCore.ClientTools so a launcher can reuse the client plumbing - #18
feat: extract NosCore.ClientTools so a launcher can reuse the client plumbing#18erwan-joly wants to merge 2 commits into
Conversation
…plumbing The auth flow and the PE patches are not packet-logger concerns — they are what any NosCore-facing launcher needs: trade credentials for an auth code, point a client binary at a server, and hand it a gf_wrapper replacement. They lived in a WinExe, so nothing outside this repo could reference them. Moves NosCoreAuthClient and ClientPatcher into a packable AnyCPU library and gives the embedded stub a real API (GfStub.OpenStream / DeployTo) instead of a bare GetManifestResourceStream call in MainForm. The x86 constraint stays where it belongs: on the injector and the stub, not on consumers that only spawn the client. Also drops Build.0 for the two NativeAOT payload projects. The solution was building them AnyCPU while the Exec targets published them for win-x86 at the same time, racing on the same obj/ directory. Adding a third project changed the scheduling enough to start losing that race. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 49 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (8)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change adds a ChangesClientTools integration
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant MainForm
participant GfStub
participant EmbeddedResource
MainForm->>GfStub: DeployTo(outDir)
GfStub->>EmbeddedResource: Open embedded noscore_gf.dll
EmbeddedResource-->>GfStub: Return payload stream
GfStub-->>MainForm: Return deployed DLL path
Merge Risk: ⚪ Minimal · up to The payload extraction preserves the shared DLL-name and deployment contract, with no actionable merge-blocking risk identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 5 files. (3 skipped: 3 unsupported.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 participant
Groundwork for the .NET 10 rewrite of NosCoreLegend/Launcher. That launcher needs exactly three things this repo already has —
NosCoreAuthClient,ClientPatcher, and thenoscore_gf.dllstub — but they live inside aWinExe, so nothing outside this repo can reference them.What moves
src/NosCore.ClientTools—net10.0, AnyCPU, packable asNosCore.ClientTools:NosCoreAuthClient(moved verbatim)ClientPatcher+EntryPatchMode(moved verbatim)GfStub(new) —FileName,OpenStream(),DeployTo(dir).PatchImportNamenow writesGfStub.FileNamerather than its own copy of the literal, so the "both names are 14 chars" invariant has one owner.The GfStub publish-and-embed target moves here too.
NosCore.DeveloperToolskeeps the Hook payload and picks the rest up byProjectReference;MainForm's inlineGetManifestResourceStreamblock collapses toGfStub.DeployTo(outDir).No behaviour change — the stub is byte-identical and the patch logic is untouched.
Why AnyCPU
The x86 constraint exists for same-arch injection. A launcher only spawns the client, so it has no reason to inherit it; the stub travels as prebuilt bytes, not as code.
Solution build fix
Dropped
Build.0forNosCore.DeveloperTools.HookandNosCore.DeveloperTools.GfStub. The solution was building them AnyCPU while theExectargets randotnet publish -r win-x86on them concurrently, racing on the sameobj/directory:Latent on master — adding a third project changed the scheduling enough to start losing it. These are RID-specific payloads, so an AnyCPU solution build of them was wasted work regardless; they still build in the IDE and are still published by the
Exectargets.Verified
dotnet buildclean (0 warnings,TreatWarningsAsErrorson). Packed and consumed the.nupkgfrom a throwaway console app:Not in this PR
workflowscope, so I can't push.github/workflows/*. Needed before the Launcher repo canPackageReferencethis.ClientDriver.csusesNosCoreAuthClientfrom the old namespace. One-line fix (using NosCore.ClientTools;) on whichever merges second.Summary by CodeRabbit
New Features
Improvements