glfw: Force init before using init dependent functions about mach HOT 10 CLOSED

Righto, if that's the new direction, I'll look into trying to come up with a more elaborated draft proposal in the coming days, 'less someone else makes one first.

from mach.

I also have similar issue in my own 2d rendering API: issue 41

from mach.

If others have feedback here on whether or not they like/dislike this, I'd love to hear it. One thing I struggle with here is:

While this is a clear win, it's also a clear substantial deviation from the current GLFW API. Where we otherwise added methods, enums, etc. you'd have to pass those around regardless. But with this, you need to pass around the API struct too / can't rely on globals. This could make porting existing code harder.

For new APIs such as your 2D rendering API, though, this is a clear win obviously. I'm just not sure if the benefits (relatively small chance you forget to call glfw.init) outweight the costs (harder to port code). Not to say I am against this, just... unsure?

from mach.

(just clarifying, PR for this is welcome)

from mach.

I like your proposal a lot. At first, I wasn't sure what benefit it would give us over what GLFW already does internally (after all, it's functions are documented as returning an error if GLFW is not initialized). I see two major benefits:

1. You get an assertion at runtime, rather than an error (which e.g. maybe someone logs by accident.)
2. We could eliminate that error altogether and that's one less thing for people to need to care about / handle.

Sounds like the right path forward to me!

from mach.

I personally would really like this change, but playing the devil's advocate here: I think it could be argued that something like this would be best made as a separate abstraction on top of the bindings, to remain as consistent as possible with canonical GLFW.

from mach.

I agree this change would be a deviation from the original API. I am also not sure if the gains are worth it, so feel free to close the issue for now @slimsag 😅

from mach.

I've tried my hand at making a draft of the proposed solution, but it generally always ended up being more cumbersome than what we currently have, being that you can't actually use an instance of a struct to access its public decls, leading to the need to have more C-like createWindow declarations within the returned struct, with a self parameter, to be able to make the guarantees this proposal aims to promise.

Though, an alternative occurred to me, working on #81: we could instead try and guarantee function call order via runtime checks when in debug mode, which are disabled when in any other mode. E.g.:

var debug_glfw_initialized = if (@import("builtin").mode == .Debug) false else @as(void, {});
pub inline fn init(hints: InitHints) Error!void {
    if (@import("builtin").mode == .Debug) debug_glfw_initialized = true;
    errdefer @import("builtin").mode == .Debug debug_glfw_initialized = false;
    ...
}

pub inline fn terminate() void {
    if (@import("builtin").mode == .Debug) std.debug.assert(debug_glfw_initialized);
    c.glfwTerminate();
}

Rough example, obviously could be greatly improved by modularizing/namespacing these things, such that they would be accessible from within all relevant parts of the library, but not outside, but this is the outline. Any thoughts, in favor, in opposition?

Edit: and to clarify, I really did try a slew of ideas before coming to this, such as having comptime type fields to limit the access of things like Window to the instance, which has its own issues and inconveniences.

from mach.

you can't actually use an instance of a struct to access its public decls, leading to the need to have more C-like createWindow declarations within the returned struct, with a self parameter, to be able to make the guarantees this proposal aims to promise.

Oh wow, I hadn't thought of that, I guess we would need stack capturing ala ziglang/zig#6965 in order to make that a nice experience. Very interesting.

from mach.

I also like that this reduces my original complaint here: "This could make porting existing code harder."

from mach.