-
Notifications
You must be signed in to change notification settings - Fork 7
build.zig: pulled out the options to its own structs #10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Could I get an update on the PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the build.zig file by extracting anonymous struct types from function parameters into named struct definitions. This change improves code reusability and maintainability, particularly for libraries that depend on zemscripten, and addresses Zig's removal of anonymous structs.
- Extracted three anonymous struct types into named struct definitions
- Maintained all existing functionality while improving API design
- Updated function signatures to use the new named struct types
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
||
pub const SettingsOptions = struct { | ||
optimize: std.builtin.OptimizeMode, | ||
emsdk_allocator: EmsdkAllocator = .emmalloc, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SettingsOptions struct is missing the shell_file field that was present in the original anonymous struct. This removes functionality that was previously available in the emccDefaultSettings function.
emsdk_allocator: EmsdkAllocator = .emmalloc, | |
emsdk_allocator: EmsdkAllocator = .emmalloc, | |
shell_file: ?[]const u8 = null, |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I neglected this. Just a couple or minor naming things other than the issue flagged by the bot. Note that the intended interface for flags and settings are hash maps... meaning these are just convenience fns that override some reasonable defaults since we don't intend to define all emcc flags and options.
Btw these are not actually anonymous, they are named by the compiler. I don't understand why the current interface is not sufficient sorry. Could you elaborate on the problem you are trying to solve?
@@ -120,10 +120,12 @@ pub fn activateEmsdkStep(b: *std.Build) *std.Build.Step { | |||
|
|||
pub const EmccFlags = std.StringHashMap(void); | |||
|
|||
pub fn emccDefaultFlags(allocator: std.mem.Allocator, options: struct { | |||
pub const FlagsOptions = struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency with other zig-gamedev libs, and for more clarity, let's name this EmccDefaultFlagsOverrides
.
mimalloc, | ||
}; | ||
|
||
pub const SettingsOptions = struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again for clarity and consistency let's name this EmccDefaultSettingsOverrides
I wanted to wrap zemscripten in the raylib build system with the needed default flags for raylib while the usage still feel like zemscripten. pub const emsdk = struct {
const zemscripten = @import("zemscripten");
pub fn shell(b: *std.Build, raylib_dep: *std.Build.Dependency) []const u8 {
return raylib_dep.path("src/shell.html").getPath(b);
}
pub const FlagsOptions = struct {
optimize: std.builtin.OptimizeMode,
asyncify: bool = true,
};
pub fn emccDefaultFlags(allocator: std.mem.Allocator, options: FlagsOptions) zemscripten.EmccFlags {
var emcc_flags = zemscripten.emccDefaultFlags(allocator, .{
.optimize = options.optimize,
.fsanitize = true,
});
if (options.asyncify)
emcc_flags.put("-sASYNCIFY", {}) catch unreachable;
return emcc_flags;
}
pub const SettingsOptions = struct {
optimize: std.builtin.OptimizeMode,
es3: bool = true,
emsdk_allocator: zemscripten.EmsdkAllocator = .emmalloc,
};
pub fn emccDefaultSettings(allocator: std.mem.Allocator, options: SettingsOptions) zemscripten.EmccSettings {
var emcc_settings = zemscripten.emccDefaultSettings(allocator, .{
.optimize = options.optimize,
.emsdk_allocator = options.emsdk_allocator,
});
if (options.es3)
emcc_settings.put("FULL_ES3", "1") catch unreachable;
emcc_settings.put("USE_GLFW", "3") catch unreachable;
emcc_settings.put("EXPORTED_RUNTIME_METHODS", "['requestFullscreen']") catch unreachable;
return emcc_settings;
}
pub fn emccStep(b: *std.Build, raylib: *std.Build.Step.Compile, wasm: *std.Build.Step.Compile, options: zemscripten.StepOptions) *std.Build.Step {
const activate_emsdk_step = zemscripten.activateEmsdkStep(b);
const emsdk_dep = b.dependency("emsdk", .{});
raylib.addIncludePath(emsdk_dep.path("upstream/emscripten/cache/sysroot/include"));
wasm.addIncludePath(emsdk_dep.path("upstream/emscripten/cache/sysroot/include"));
const emcc_step = zemscripten.emccStep(b, wasm, options);
emcc_step.dependOn(activate_emsdk_step);
return emcc_step;
}
pub fn emrunStep(
b: *std.Build,
html_path: []const u8,
extra_args: []const []const u8,
) *std.Build.Step {
return zemscripten.emrunStep(b, html_path, extra_args);
}
}; Also the problem I had at the start why I also mentioned anonymous structs was when I copy pasted the options into a struct for example pub const FlagsOptions = struct {
optimize: std.builtin.OptimizeMode,
fsanitize: bool,
}; and wanted to use it and just give it into the method const options: FlagsOptions = .{.optimize = optimize, .fsanitize = true};
const emcc_flags = emccDefaultFlags(b.allocator, options); it wouldn't work I would start fixing the comments if u feel like that your questions are answer why I want that change. |
I pulled out the structs from the function arguments because that makes it easier to include zemscripten in other libraries.
Specially because zig removed Anonymous Struct.