[Audit][High] ResourceManager.createShader is stubbed - returns error instead of creating shaders

[MichaelFisher1997]

🔍 Module Scanned

src/engine/graphics/vulkan/ (automated audit scan)

📝 Summary

The ResourceManager.createShader() function is a stub that always returns error.ExtensionNotPresent instead of actually compiling and creating Vulkan shader modules. This means any code path that calls rhi.createShader() will fail, which appears to bypass the actual GLSL→SPIR-V compilation pipeline that should be used for terrain and other shaders.

📍 Location

• File: src/engine/graphics/vulkan/resource_manager.zig:433-441
• Function/Scope: createShader

🔴 Severity: High

• High: Memory leaks, race conditions, incorrect rendering, broken features

💥 Impact

Any code that calls rhi.createShader() or ResourceManager.createShader() will receive an ExtensionNotPresent error. This appears to be dead/stub code, as the actual shader compilation likely happens elsewhere in the pipeline. However, if any production code path relies on this function, shader loading will fail silently. Additionally, the function name ExtensionNotPresent is misleading since shader creation is a core Vulkan feature, not an extension.

🔎 Evidence

pub fn createShader(self: *ResourceManager, vertex_src: [*c]const u8, fragment_src: [*c]const u8) rhi.RhiError!rhi.ShaderHandle {
    _ = self;
    _ = vertex_src;
    _ = fragment_src;
    // TODO: Implement actual shader creation when ready.
    // For now, return error to avoid silent failure.
    // NOTE: If engine code calls this, it will now fail loudly, which is better than silent failure.
    return error.ExtensionNotPresent; // Or proper NotImpl error
}

The function:

1. Accepts vertex and fragment GLSL source strings but ignores them
2. Returns a misleading error ExtensionNotPresent instead of a proper error like NotImplemented or ShaderCreationFailed
3. Has a TODO comment indicating it's incomplete
4. Has a corresponding destroyShader implementation that does nothing meaningful (_ = self; _ = handle;)

🛠️ Proposed Fix

Either:

1. Implement full shader creation: Use vkCreateShaderModule with SPIR-V bytecode compiled from GLSL sources via glslangValidator or a Zig-native compiler
2. Add proper error handling: Change ExtensionNotPresent to NotImplemented or ShaderCreationNotSupported to accurately reflect the state
3. Wire up destroyShader: The current destroyShader does nothing - if shaders are created elsewhere, this should be wired up to actually destroy them
4. Document the shader pipeline: If shader creation happens elsewhere (e.g., in a pipeline manager), add a comment explaining where and why

✅ Acceptance Criteria

• createShader either implements actual shader creation or returns a meaningful error
• destroyShader properly cleans up shader modules when creation is implemented
• Error type accurately reflects the failure mode (not ExtensionNotPresent)
• All callers of createShader handle the error appropriately
• Shader compilation pipeline is documented if it happens elsewhere

📚 References

• Vulkan spec: vkCreateShaderModule - core Vulkan feature, not an extension
• Related: build_options debug flags in rhi_vulkan.zig for shader validation
• Context: The render pipeline likely depends on external SPIR-V compilation via glslangValidator during build time

[github-actions]

The audit finding is valid. Here's my analysis:

Bug Classification: Bug - stubbed implementation with misleading error type.

Root Cause Confirmed:

• src/engine/graphics/vulkan/resource_manager.zig:433-441 - confirmed stubbed as reported
• Error type ExtensionNotPresent is incorrect - vkCreateShaderModule is core Vulkan, not an extension

No Active Callers: My search for .createShader( found zero production call sites. The RHI createShader VTable function is defined but never invoked by any system. Shader creation is handled differently - each rendering system (TAA, water, SSAO, bloom, etc.) calls Utils.createShaderModule() directly with pre-compiled SPIR-V bytecode at build time.

Impact Assessment: Low in practice since no code path currently exercises this stub, but the misleading error type and TODO indicate incomplete implementation. If any future code attempts to use rhi.createShader(), it will fail with a confusing error.

Suggested Fix:

1. Change error.ExtensionNotPresent → error.NotImplemented (or add ShaderCreationNotSupported)
2. Add a comment noting that actual shader creation is handled by per-system Utils.createShaderModule() calls with build-time compiled SPIR-V
3. Wire up destroyShader if shader creation is ever implemented

opencode session  |  github run

[MichaelFisher1997]

/oc investigate this PR and comments.
if its still valid, solve it and submit it as a PR against dev and link this issue

[MichaelFisher1997]

Created PR #496

opencode session  |  github run