Not-Nik / raylib-zig

Manually tweaked, auto-generated raylib bindings for zig. https://github.com/raysan5/raylib
MIT License
473 stars 101 forks source link

Update to raylib 5.0 #62

Closed iacore closed 5 months ago

jeanlauliac commented 6 months ago

Thank you, I was looking for this. There are 2 changes missing I think, one of the example has another var that needs changing to const, and the project_setup.sh script needs getArtifact replaced by getRaylib. I was able to run the template project locally but didn't try the emscripten target.

iacore commented 6 months ago

There are 2 changes missing I think

Send patch please.


A lot of new API got updated. Need to update the API list too.

there's also this: https://github.com/raysan5/raylib/pull/3673

iacore commented 5 months ago

@Not-Nik check this patch

With this patch, raylib-zig can be used as dependency.

example code:

    // added in build.zig.zon
    const rl = b.dependency("raylib", .{ .target = target, .optimize = optimize });

    const exe = b.addExecutable(.{
        .name = "program",
        .root_source_file = .{ .path = "src/main.zig" },
        .target = target,
        .optimize = optimize,
    });
    exe.addModule("raylib", rl.module("raylib"));
    exe.addModule("raylib-math", rl.module("raylib-math"));
    exe.linkLibrary(rl.artifact("raylib"));
Not-Nik commented 5 months ago

Terribly sorry for not answering on this sooner. GitHub somehow didn't send me an E-Mail about this PR and I wanted to take a closer look before I merge.

First of all, this looks great, I do have some minor points that need addressing though.

iacore commented 5 months ago

The commit seems randomly chosen. Is there a reason for not using the 5.0 release commit?

I mostly forgot! I remember sawing the latest commits having some bug fixes, so I chose the latest commit.

I'd prefer to not have raylib.h and raymath.h in this repository.

I object to this. This software should be able to be developed with a fixed version of the headers. I was then tweaking the code generator, and having a fixed version of the headers was necessary. This ensures that anyone who want to tweak the code generator in the future will have the same copy of headers.

The added rlm imports seem unnecessary.

Yes, it is unused now. It was useful during development. Should I delete this? afaik the library still behaves the same.

Not-Nik commented 5 months ago

I mostly forgot! I remember sawing the latest commits having some bug fixes, so I chose the latest commit.

There are also some API changes, like added functions, so I'd prefer if we stuck to raysan5/raylib@ae50bfa2cc569c0f8d5bc4315d39db64005b1b08.

I object to this. This software should be able to be developed with a fixed version of the headers. I was then tweaking the code generator, and having a fixed version of the headers was necessary. This ensures that anyone who want to tweak the code generator in the future will have the same copy of headers.

That seems reasonable. The headers should also be from the 5.0 release.

Yes, it is unused now. It was useful during development. Should I delete this? afaik the library still behaves the same.

Yes, please.

Not-Nik commented 5 months ago

Hey, can you just give me a quick feedback on why you added the .zig to raylib-zig in raylib-zig-math*?

const rl = @import("raylib-zig.zig");

I think this is causing #69 (nice), because math is including the file instead of the module.

Not-Nik commented 5 months ago

Just realised the module is actually called raylib, not raylib-zig, maybe we should import that

Not-Nik commented 5 months ago

Nope, the dependency has another name: raylib-zig, so importing that is fine, I'll update it, if you have objections post them in #69.

iacore commented 4 months ago

Hey, can you just give me a quick feedback on why you added the .zig to raylib-zig in raylib-zig-math*?

const rl = @import("raylib-zig.zig");

I think this is causing #69 (nice), because math is including the file instead of the module.

Sorry about causing the bug :laughing:

I added .zig because I saw the file is local, and importing a file that way is more "robust" (what I felt).

Zig doesn't support importing the same file with two names, even if it's the same file...