Make zig-ecs compatible with current zig version (0.11.0-dev)

[stefanpartheym]

First of all, thanks for the effort. I tried this library with zig v0.9.1 and I got started very fast.

However, one thing that would be nice is to use zig-ecs with the current v0.11.0 version of the zig compiler. Currently, if I try to build a simple project (or the examples) using zig v0.11.0-dev.1571+c1f71963a I receive a quite generic error:

$ zig build
error: zigecs-test...
error: The following command terminated unexpectedly:
/home/stefanpartheym/.zvm/master/zig build-exe /home/stefanpartheym/develop/stefanpartheym/zigecs-test/src/main.zig --cache-dir /home/stefanpartheym/develop/stefanpartheym/zigecs-test/zig-cache --global-cache-dir /home/stefanpartheym/.cache/zig --name zigecs-test --pkg-begin ecs /home/stefanpartheym/develop/stefanpartheym/zigecs-test/deps/zig-ecs/src/ecs.zig --pkg-end --enable-cache 
error: the following build command failed with exit code 11:
/home/stefanpartheym/develop/stefanpartheym/zigecs-test/zig-cache/o/ee157244c8abf2f8e61ae7f5813ecc82/build /home/stefanpartheym/.zvm/master/zig /home/stefanpartheym/develop/stefanpartheym/zigecs-test /home/stefanpartheym/develop/stefanpartheym/zigecs-test/zig-cache /home/stefanpartheym/.cache/zig run

If I then try to execute the actual command line run by zig build:

$ /home/stefanpartheym/.zvm/master/zig build-exe /home/stefanpartheym/develop/stefanpartheym/zigecs-test/src/main.zig --cache-dir /home/stefanpartheym/develop/stefanpartheym/zigecs-test/zig-cache --global-cache-dir /home/stefanpartheym/.cache/zig --name zigecs-test --pkg-begin ecs /home/stefanpartheym/develop/stefanpartheym/zigecs-test/deps/zig-ecs/src/ecs.zig --pkg-end --enable-cache
fish: Job 1, '/home/stefanpartheym/.zvm/maste…' terminated by signal SIGSEGV (Adressbereichsfehler)

... the process segfaults.

I suspect, this is may be due to some very complex comptime syntax in src/ecs/component_storage.zig, which might be deprecated in current compiler versions: In line 186 of component_storage.zig there is pub usingnamespace if (is_empty_struct). Which, if I remove like follows, reveals a different error (but at least not a segfault):

diff --git a/src/ecs/component_storage.zig b/src/ecs/component_storage.zig
index e3aa6b7..72a4408 100644
--- a/src/ecs/component_storage.zig
+++ b/src/ecs/component_storage.zig
@@ -183,17 +183,6 @@ pub fn ComponentStorage(comptime Component: type, comptime Entity: type) type {
             return self.set.len();
         }

-        pub usingnamespace if (is_empty_struct)
-            struct {
-                /// Sort Entities according to the given comparison function. Only T == Entity is allowed. The constraint param only exists for
-                /// parity with non-empty Components
-                pub fn sort(self: Self, comptime T: type, context: anytype, comptime lessThan: fn (@TypeOf(context), T, T) bool) void {
-                    std.debug.assert(T == Entity);
-                    self.set.sort(context, lessThan);
-                }
-            }
-        else
-            struct {
                 /// Direct access to the array of objects
                 pub fn raw(self: Self) []Component {
                     return self.instances.items;
@@ -262,7 +251,6 @@ pub fn ComponentStorage(comptime Component: type, comptime Entity: type) type {
                         self.set.arrange(length, swap_context, SortContext.sort, swap_context);
                     }
                 }
-            };

         /// Direct access to the array of entities
         pub fn data(self: Self) []const Entity {

Using this patch will result in the following compilation error:

deps/zig-ecs/src/ecs/registry.zig:287:13: error: unable to resolve comptime value
        self.assure(@TypeOf(value)).add(entity, value);
        ~~~~^~~~~~~
deps/zig-ecs/src/ecs/registry.zig:287:13: note: argument to function being called at comptime must be comptime-known
deps/zig-ecs/src/ecs/registry.zig:217:54: note: expression is evaluated at comptime because the generic function was instantiated with a comptime-only return type
    pub fn assure(self: *Registry, comptime T: type) *Storage(T) {
                                                     ^~~~~~~~~~~
referenced by:
    add__anon_4436: deps/zig-ecs/src/ecs/registry.zig:287:13
    main: src/main.zig:16:8
    remaining reference traces hidden; use '-freference-trace' to see all reference traces

Finally, as reference, the build.zig and src/main.zig files I used:

build.zig:

const std = @import("std");

pub fn build(b: *std.Build) void {
    const target = b.standardTargetOptions(.{});
    const optimize = b.standardOptimizeOption(.{});

    const exe = b.addExecutable(.{
        .name = "zigecs-test",
        .root_source_file = .{ .path = "src/main.zig" },
        .target = target,
        .optimize = optimize,
    });
    exe.addModule("ecs", b.createModule(.{
        .source_file = std.build.FileSource{ .path = "deps/zig-ecs/src/ecs.zig" },
    }));
    exe.install();

    const run_cmd = exe.run();
    run_cmd.step.dependOn(b.getInstallStep());
    if (b.args) |args| {
        run_cmd.addArgs(args);
    }

    const run_step = b.step("run", "Run the app");
    run_step.dependOn(&run_cmd.step);

    const exe_tests = b.addTest(.{
        .root_source_file = .{ .path = "src/main.zig" },
        .target = target,
        .optimize = optimize,
    });

    const test_step = b.step("test", "Run unit tests");
    test_step.dependOn(&exe_tests.step);
}

src/main.zig

const std = @import("std");
const builtin = @import("builtin");
const ecs = @import("ecs");

const Position = struct {
    x: f32,
    y: f32,
};

pub fn main() !void {
    std.debug.print("## Testing zig-ecs (zig v{}) ##\n", .{ builtin.zig_version });

    var reg = ecs.Registry.init(std.heap.page_allocator);

    var player = reg.create();
    reg.add(player, Position{ .x = 10, .y = 10 });
}

Thanks and best regards Stefan

[stefanpartheym]

Ok, after reading the Language changes in release v0.10.0 I was able to fix a couple of compilation errors, see commit 73d190f of my fork.

However, after running example "simple" I realized, there's still a lot of work to do. For me, it currently prints the following (obviously changed print format string to satisfy zig v0.10.0 compiler):

entity: 1, pos: (x = 10, y = 10), vel: (x = 15, y = 17)
---- resetting iter

I think the output should be something like this:

entity: 1, pos: Position{ .x = 1.0e+01, .y = 1.0e+01 }, vel: Velocity{ .x = 1.5e+01, .y = 1.7e+01 }
entity: 0, pos: Position{ .x = 0.0e+00, .y = 0.0e+00 }, vel: Velocity{ .x = 5.0e+00, .y = 7.0e+00 }
---- resetting iter
entity: 1, pos: Position{ .x = 2.5e+01, .y = 2.7e+01 }, vel: Velocity{ .x = 1.5e+01, .y = 1.7e+01 }
entity: 0, pos: Position{ .x = 5.0e+00, .y = 7.0e+00 }, vel: Velocity{ .x = 5.0e+00, .y = 7.0e+00 }

[prime31]

Hi! I haven't actually updated to v0.10 for many of my libs. There was quite a storm of changes with functions all needing to be * const and a slew of other changes. I was holding out until perhaps zig fmt could do some of the fixing and I also feared that there were still a lot more breaking changes coming (local functions are on their way amongst other things).

That being said, it sounds like you are pretty close. If you do want to send in a PR I'll for sure pull it in. Looking at your fork nothing specifically stands out. It seems you got the * const for all the fns and the unused vars. I'll give it a read after work fully and see if something pops out.

[stefanpartheym]

Hey @prime31,

Hi! I haven't actually updated to v0.10 for many of my libs. There was quite a storm of changes with functions all needing to be * const and a slew of other changes. I was holding out until perhaps zig fmt could do some of the fixing and I also feared that there were still a lot more breaking changes coming (local functions are on their way amongst other things).

Yes you are right, there have been a ton of changes. The release notes for v0.10.0 are lengthy :sweat_smile:

That being said, it sounds like you are pretty close. If you do want to send in a PR I'll for sure pull it in. Looking at your fork nothing specifically stands out. It seems you got the * const for all the fns and the unused vars. I'll give it a read after work fully and see if something pops out.

To be honest, I don't think I'm that close yet. There is still a lot of stuff broken (like Iterators). Also there are one or two compilation errors, that cause building the tests to fail. Unfortunately, mit zig knowledge isn't really worth mentioning, so it could take some time to figure out what needs to be done :sweat_smile:

Best regards Stefan

[stefanpartheym]

Hey @prime31,

I managed to fix a couple of errors. Also the Iterator implementation in MultiView should work now. All tests seem to run successfully. And at least the simple example now shows expected output:

entity: 1, pos: (x = 10, y = 10), vel: (x = 15, y = 17)
entity: 0, pos: (x = 0, y = 0), vel: (x = 5, y = 7)
---- resetting iter
entity: 1, pos: (x = 25, y = 27), vel: (x = 15, y = 17)
entity: 0, pos: (x = 5, y = 7), vel: (x = 5, y = 7)

If you could review my latest commits, I'd be more than happy :)

EDIT: Latest fixes also work for zig compiler v0.10.1. I think upgrading to zig compiler v0.11.0-dev is only a matter of updating the build.zig.

Best regards Stefan

[stefanpartheym]

Ok, I upgraded to zig v0.11.0-dev.1580+a5b34a61a and fixed some compilation errors in this commit. So it is now compatible with zig v0.11.0.

Best regards Stefan

[prime31]

That all looks good to me!

[stefanpartheym]

Great! I'll create a PR for this ;)

[stefanpartheym]

Hey @prime31, thanks for the merge. This issue resolved now :)