Claude/refactor codebase iz ey h#490
Merged
Merged
Conversation
…untime
Replaces the 9 deprecated free-floating `g_graphics`/`g_input`/`g_timer`/
`g_eventBus`/`g_moduleManager`/`g_audioEngine`/`g_audioBackend`/
`g_moduleHotReload`/`g_physicsOwned` globals — previously defined in
SparkEngine.cpp and `extern`-declared in each platform entry file — with
a single `EngineRuntime` ownership struct accessed via `GetEngineRuntime()`.
Subsystem lookup for the rest of the engine continues to go through
EngineContext. External code never touched these globals (confirmed by
grep) — the migration is contained to Core/SparkEngine{,Windows,Linux}.cpp.
Net diff is ~zero lines (token swap); no behavioural change. Tests pass,
all 5869. Updates CLAUDE.md and .claude/knowledge/codebase-observations.md
to document the new access pattern.
https://claude.ai/code/session_015coEh2fxVdjJWj2NyWxR7t
D3D11, D3D12, Vulkan, and OpenGL backends all carried identical copies
of the same three members:
- RHIDeviceCapabilities m_capabilities
- RHIStatistics m_statistics
- TransientBufferAllocator m_transientBuffers{4M, 2M}
…plus the identical inline GetCapabilities/GetStatistics getters and
a one-line `ResetStatistics() { m_statistics = {}; }` body. Those now
live on a new RHIDeviceBase that each concrete backend inherits from.
BeginFrame/EndFrame still own their backend-specific work (fence
pacing on D3D12, deletion queue on D3D11, etc.) and call through to
the inherited transient pump and ResetStatistics().
NullRHIDevice intentionally stays on IRHIDevice directly — it uses
different field names (m_caps / m_rhiStats) and a smaller transient
budget (64K/32K) for the headless fallback.
Net: -76 lines across the four backends, +51 lines for the new base.
Future RHI backends (Metal, WebGPU) now inherit this boilerplate for
free. Tests: all 5869 pass; build clean.
https://claude.ai/code/session_015coEh2fxVdjJWj2NyWxR7t
…torUI EditorUI.cpp went from 1777 lines to 1515 by extracting two cohesive, low-coupling slices: 1. **EditorCommandRegistry.cpp** (sibling-file pattern, mirrors EditorMenuBar.cpp). Holds the InitializeCommandPalette / RegisterPanelToggleCommands / RegisterEditCommands / RegisterSceneCommands / RegisterToolCommands methods — pure data-driven registration that only touches m_commandPalette and already-public state. No public-API change. 2. **EditorNotificationManager** (independent class). Owns the Notification struct, the m_notifications vector, and the Update()/Render()/Show() methods. EditorUI now holds a unique_ptr<EditorNotificationManager> and its ShowNotification() becomes a thin delegate. RenderNotifications() and UpdateNotifications() helpers are gone — Update/Render call the manager directly. Net: -262 lines removed from EditorUI.cpp (-14.7%). Two new files introduced; the new class is independently testable and the command registry file keeps the same EditorUI class surface. All 5869 tests pass; build clean. https://claude.ai/code/session_015coEh2fxVdjJWj2NyWxR7t
Contributor
Code Coverage (GCC + lcov)Per-Subsystem Coverage
Total: 50.3% (27443/54578 lines) |
Contributor
❌ CI Error ReportFailed jobs: check-format, clang-tidy, linux-clang-Release, macos-Debug, macos-Release, windows-vs2022-Release Build Errors
Other errors (2)Full error output
|
| Test | Jobs |
|---|---|
| [20:30:12.025] [TID:7364] [WARN ] [Network ] RCON unknown command: nonexistent_cmd (DedicatedServer.cpp:573) | windows-vs2022-Release |
| [20:30:12.040] [TID:7364] [WARN ] [Network ] NetBuffer::ReadUint8 — buffer overrun at pos 1 (size=1) (NetworkBuffer.c... | windows-vs2022-Release |
| [20:30:12.060] [TID:7364] [WARN ] [AI ] BuildNavMeshWithRecast: empty geometry (RecastDetourBackend.cpp:35) | windows-vs2022-Release |
| [20:30:12.060] [TID:7364] [WARN ] [AI ] NavMeshBuilder: Recast build failed, falling back to triangle-soup builder (N... | windows-vs2022-Release |
| [20:30:12.060] [TID:7364] [WARN ] [Audio ] XAudio2 backend requested but no AudioEngine provided, falling back to Nul... | windows-vs2022-Release |
| [20:30:12.061] [TID:7364] [WARN ] [Audio ] XAudio2 backend requested but no AudioEngine provided, falling back to Nul... | windows-vs2022-Release |
| [ [20:30:12.062] [TID:7364] [WARN ] [Graphics ] No graphics backend available — falling back to NullRHIDevice (headle... | windows-vs2022-Release |
| [20:30:12.062] [TID:7364] [WARN ] [Graphics ] No graphics backend available — falling back to NullRHIDevice (headless... | windows-vs2022-Release |
| [20:30:12.067] [TID:7364] [WARN ] [Editor ] Cannot host: userName is empty. (CollaborativeEditSession.cpp:454) | windows-vs2022-Release |
| [20:30:12.067] [TID:7364] [WARN ] [Editor ] Already connected. (CollaborativeEditSession.cpp:459) | windows-vs2022-Release |
| [20:30:12.069] [TID:7364] [WARN ] [Editor ] BroadcastEdit rejected: nodeId is empty. (CollaborativeEditSession.cpp:1108) | windows-vs2022-Release |
| [20:30:12.069] [TID:7364] [WARN ] [Editor ] BroadcastEdit rejected: sourceEditor is not set. (CollaborativeEditSessio... | windows-vs2022-Release |
| [20:30:12.070] [TID:7364] [WARN ] [Editor ] SetLocalSelection rejected: nodeId exceeds 255 chars (length=300). (Colla... | windows-vs2022-Release |
| [20:30:17.649] [TID:7364] [WARN ] [Network ] Connection rejected for pending client 5: server full (4/4) (NetworkConn... | windows-vs2022-Release |
| [20:30:17.650] [TID:7364] [WARN ] [Network ] Connection rejected for pending client 5: server full (4/4) (NetworkConn... | windows-vs2022-Release |
Compiler Warnings (6)
../../../ThirdParty/SDL2/src/joystick/hidapi/SDL_hidapi_steam.c:165:9: warning: 'printf' macro redefined [-Wmacro-redefined] [linux-clang-Release]
../SparkEngine/Source/Engine/Animation/BlendSpace.cpp:313:16: warning: variable 'validTriCount' set but not used [-Wunused-but-set-variable] [linux-clang-Release]
../SparkEngine/Source/Physics/PhysicsBodyImpl.cpp:270:23: warning: comparing floating point with == or != is unsafe [-Wfloat-equal] [linux-clang-Release]
../SparkEngine/Source/Physics/PhysicsSystem.cpp:401:12: warning: use of old-style cast [-Wold-style-cast] [linux-clang-Release]
../../Tests/TestReliableChannel.cpp:390:9: warning: variable 'retransmitCount' set but not used [-Wunused-but-set-variable] [linux-clang-Release]
../../Tests/TestSparkError.cpp:34:5: warning: ignoring return value of function declared with 'warn_unused_result' attribute [-Wunused-result] [linux-clang-Release]
Updated: 2026-04-17T20:30:51Z — this comment is updated in-place, not duplicated.
Krilliac
pushed a commit
that referenced
this pull request
Apr 18, 2026
build-linux-clang (Release) has been failing with: /usr/bin/ld: ../lib/libJolt.a: error adding symbols: file format not recognized clang++: error: linker command failed with exit code 1 Root cause: actions/cache restores a build/ directory from a previous run. When apt-get upgrades clang / binutils between runs, the cached libJolt.a can end up in a format the current linker no longer recognizes — similar to the existing PCH-vs-unistd_64.h mtime mismatch that this job already handles. Extend the existing stale-artifact cleanup to also scan lib*.a archives. Each one is probed with \`ar t\`; any archive that can't be listed is purged, forcing CMake to rebuild it on the next step. ccache still keeps .o compilation fast. Pre-existing failure (also hit PR #490 on the same job); fixed here.
Krilliac
pushed a commit
that referenced
this pull request
Apr 18, 2026
Two pre-existing failures that were already present on PR #490 / Working: 1. NetworkStack_UDPInitializeSucceeds on Windows `UDPTransport::Initialize()` called ::socket() without calling WSAStartup() — on Windows that fails with WSANOTINITIALISED. The only call to WSAStartup() lived inside `NetworkManager::Initialize()`, so any test or subsystem creating a bare NetworkStack / UDPTransport couldn't open a socket. Fix: add a reference-counted `WinsockScope` RAII helper inside UDPTransport.h (Windows-only). Each UDPTransport takes one reference in Initialize() and releases it in Shutdown(); the first scope calls WSAStartup, the last calls WSACleanup. 2. build-macos (Debug/Release) — same `libJolt.a: error adding symbols: file format not recognized` as linux-clang. apt-get / brew upgrades leave stale archives in actions/cache, which the current ld can't parse. Extended the build step with the same `ar t`-based probe + rm that we just added to linux-clang. Local: 5616/5616 tests pass, including the now-portable NetworkStack_UDPInitializeSucceeds.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.