to be an union, or not to be an union

[random garnet]

hi,

in odin i could do:

Ressource :: struct {
    variant: union{
        ^FontAsset,
    }
}

FontAsset :: struct {
    using base: Ressource,
}

RessourceCache :: struct {
    map_ressources: map[string]^Ressource
}

load_asset::proc($T: typeid,  self: ^RessourceCache, path: string) -> ^T {
    // here, great
    asset: ^T = self.ressources[path].variant.(^T)

    return asset
}

in zig, i try to achieve something similar:

pub const Ressource = struct {

    variant: union(enum) {
        font: *FontAsset,
    },
};

pub const RessourceCache = struct {
    map_ressources: std.StringHashMap(*Ressource),



    pub fn load(self: *RessourceCache, comptime T: type, path: []const u8) ?*T {

        var existing = switch (self.map_ressources.get(path).?.variant) {
                .tex => |val| val,
                .font => |val| val,
                .model => |val| val,
        };

        return asset;
    }
};

This is the best i could come up with, even thought i don't like the switch.. and it doesn't compile:

error: incompatible types: '*assets.TextureAsset' and '*assets.FontAsset'
            var existing = switch (self.map_ressources.get(path).?.variant) {
                           ^~~~~~

Is there a better way?

Thanks

[random garnet]

I'm trying to move away from this painful approach:

pub const RessourceCache = struct {
    pool_texture: std.heap.MemoryPool(TextureAsset),
    pool_font: std.heap.MemoryPool(FontAsset),
    pool_model: std.heap.MemoryPool(ModelAsset),

    map_textures: std.StringHashMap(*TextureAsset),
    map_fonts: std.StringHashMap(*FontAsset),
    map_models: std.StringHashMap(*ModelAsset),

It worked great, but odin let me approach this issue in a cleaner (imo) way, and it'd like to replicate it

[stray umbra]

I assume there's a typo on your post, should it be return existing? Or are we missing code

What's your callsite for load look like too?

[random garnet]

copy/pasta error, i tried to shrink the code to the absolute minimum for this post, real code is obviously not what i have shared

[silver breach]

pub fn load(self: *ResourceCache, comptime T: type, path: []const u8) ?*T {
    const res = self.map_resources.get(path) orelse return null;
    switch (res) {
        inline else => |val| {
            if (@TypeOf(val) == T) {
                return val;
            } else {
                return null;
            }
        },
    }
}

[random garnet]

i'll try that, thanks, even thought looks hard to understand

wait, you changed the whole logic

i don't want to return here, i want to set it to a variable

[silver breach]

sure, that's fine - it's in a variable inside the inline else :)

just put your logic in there and do whatever you like

[random garnet]

i don't understand

can you show it?

i explicitly wrote:

        var existing = switch (self.map_ressources.get(path).?.variant) {
                .tex => |val| val,
                .font => |val| val,
                .model => |val| val,
        };

asset: ^T = self.ressources[path].variant.(^T)

[silver breach]

Here's a more direct translation of your original code: ```ts
pub fn load(self: *ResourceCache, comptime T: type, path: []const u8) ?*T {
const res = self.map_resources.get(path) orelse {
// Return if the resource doesn't exist
return null;
};
const asset = switch (res) {
inline else => |val| if (@TypeOf(val) != *T)
// Return if the resource is the wrong type
return null
else val,
};

return asset;

}

[random garnet]

thanks, that works

but i'm not going to go with it, i don't like reading that code

i'll rethink the problem

@blissful marsh the "zen of zig", look at the result

asset: ^T = self.ressources[path].variant.(^T) is the only readable solution

[silver breach]

it's doing the same thing as your odin code, just safer

because it actually checks for missing entries or incorrect types

[random garnet]

i don't see it as safer, i see it as unreadable code that i will never remember how to write correctly

[silver breach]

then maybe zig's explicitness isn't for you :)

[random garnet]

i do the proper checks earlier in the body

nah, zig is for me, it's that solution that is not for me

[silver breach]

that is how you do it in zig

[random garnet]

and that's okay, i'll just rethink the problem

i'm just pointing it out instead of ignoring how annoying that solution is to write

[blissful marsh]

looks pretty alright, not too meta. inline else is something one needs to figure out as it's pretty zig unique concept

[random garnet]

no, that code makes nosense when one reads it

[silver breach]

if you share your full function, i might be able to help reorganize things to make this approach more readable :)

it sounds like you may be coming at this from a C-style approach of "check, then dereference", whereas Zig tends to use a Python-style approach of "ask forgiveness, not permission"

[random garnet]

@stray umbra please remove the answered mark, my question is still open "Is there a better way? "

[stray umbra]

Whoops must have been a misclick sorry!

[random garnet]

Thanks @stray umbra

[blissful marsh]

I mean zig isn't a nobrainer language, but what is important it's consistent and has symmetry in it's concepts, so fairly easy to predict

[silver breach]

if you can share more info about what you're trying to achieve, i'd be happy to suggest other approaches - atm i don't have enough info to suggest a "better way"

[random garnet]

i'll try to extract the whole code to show the logic i'm trying to go with

[stray umbra]

I've only been using zig as hobby work for a few months and the code makes good sense to me. That's not to say zig isn't a more involved language than Odin (that's kinda a given) but calling it nonsense is perhaps not constructive

Tbf what I would say is I have a friend who's using Odin and we discuss the differences a lot, and liberal use of Unions like this seems super common in Odin. However I have had to help him debug multiple hard to track down instances of assumptions about which union variant is currently active, or when it was safe or not safe to cast a reference to a variant type up to a union type that contains it (which Odin lets you do). I use unions in my Zig code too but haven't had to think about or debug issues like that at all, because Zig's unions are remarkably safe.

[random garnet]

this:

    const asset = switch (res) {
        inline else => |val| if (@TypeOf(val) != *T)
            // Return if the resource is the wrong type
            return null
        else val,
    };

is not safe, if i remember poorly and make a typo, i'll introduce a sneaky bug, because you expect me to be the compiler

asset: ^T = self.ressources[path].variant.(^T) this is safe, the compiler will do the check, if T is not part of the variant, i'll get an error message

yall argue about things that make sense to you, not me

[stray umbra]

Maybe you're conflating the compiler with the runtime but the compiler can't do the check in either case because the variant is not known at compile time

The zig code cannot error at runtime, it is empirically correct (edit: maybe a bit extreme wording, but it's definitely more safe). The Odin code can.

[sly oracle]

theyre saying in odin the compiler inserts the check

[stray umbra]

Ah gotcha 🙂

I guess I'd say trust the zig compiler? It should be quite hard to introduce a sneaky bug there - it's enforcing you get a concrete value out of the union and the type system is quite strict on that.

[eager peak]

@setRuntimeSafety(true);
inline for (std.meta.fields(Resource)) |field| {
    if (field.type == T) return @field(res, field.name);
}
@compileError("Invalid resource type");

Cooked and untested, but does it work?

[random garnet]

I give up, no more union in zig for me

error: expected type 'assets.Ressource__union_23726', found '*assets.FontAsset'
        asset.base.variant = asset;
                             ^~~~~

what yall excuse gonna be to justify this error message, let's see

[blissful marsh]

it's probably due to the pointer. Left hand side is not a pointer type, righthand side is a pointer

could be clearer error message yea

[sly oracle]

i dont see how the error is unclear, you cant coerce something to a union that holds it

[random garnet]

?

[sly oracle]

?

[random garnet]

what me do

[sly oracle]

what are the types

[sly oracle]

asset.base.variant = .{ .font = asset };

[random garnet]

bro, it is T

comptime type

[sly oracle]

ok yes true

youll have to use @unionInit

i feel like if you just rewrite something in another language instead of rethinking it its 90% of the time gonna be aids

[blissful marsh]

yea though you still need to figure out the active field name, so need to use a switch to figure out what variant T resolves to.

But this is getting convoluted, surely this could be made nicer

[random garnet]

i'm trying to use zig's union right now, this doesn't tell me how to use it

[blissful marsh]

@unionInit(Resource, idk_figure_this_out, asset);
where idk_figure_this_out could come from a switch that gives the union field name from T

but this does seem like working against grain of zig, this doesn't seem like big of an ask for zig, but this definitely isn't optimal

okay i know how i'd do it, instead of passing in T, i'd pass in the union's backing enum value. From that you can figure out the union field's type

i'll see if I can throw together minimal example

[silver breach]

your confusion here may be coming from the fact that Zig's unions have named fields, like C unions and Rust enums, whereas Odin's unions are more like restricted "variants", in that fields are identified solely by their type, and don't have names

[blissful marsh]

Nicer imo. Still need the inline else switch, but otherwise this is easier to deal with and also nicer API, since you can use it like cache.load(.font, "path/to/font.ttf")

const std = @import("std");

// Asset stubs
const TextureAsset = opaque{};
const FontAsset = opaque{};
const ModelAsset = opaque{};

const AssetVariant = union(enum) {
    tex: *TextureAsset,
    font: *FontAsset,
    model: *ModelAsset,
};
const AssetVariantEnum = std.meta.Tag(AssetVariant);

const ResourceCache = struct {
    fn load(
        self: *ResourceCache,
        comptime variant: AssetVariantEnum,
        path: []const u8
    ) ?*std.meta.FieldType(AssetVariant, variant) {
        if (self.get(path)) |res| {
            switch(res) {
                inline else => |asset, tag| {
                    std.debug.assert(tag == variant);
                    std.log.info("asset {} {s} already existing", .{ variant, path });
                    return asset;
                },
            }
        }

        const FieldType = std.meta.FieldType(AssetVariant, variant);
        const asset = std.testing.allocator.create(FieldType);
        return @unionInit(AssetVariant, @tagName(variant), asset);
    }
};

i paracoded(as in paraphrased, idk, i'm cooking) ofc since I don't have all your types, but this structurally matches your problem and should serve as example

[random garnet]

@blissful marsh thanks for trying to help me, i appreciate you taking the time for my issue but this is not valid solution for me either, this look obfuscated now

It returns ?*std.meta.FieldType(AssetVariant, variant) what's even that

[silver breach]

what do you think it could be? :)

[blissful marsh]

gets union field's type. You can make it nicer by making taht as method of the union

[silver breach]

Though we have @FieldType now, so really it should be @FieldType(AssetVariant, @tagName(variant))

[random garnet]

I don't like it sorry

[silver breach]

it really sounds like you're set on using odin tbh

[unkempt apex]

alternative suggestion: return a union

[random garnet]

i give up union for now, i'll stick to what i had earlier, even tho it's more annoying to manage https://gist.github.com/ryuukk/e11742f4cc8649bc994c9e0b29e610f1

[blissful marsh]

const AssetVariant = union(enum) {
  tex: *TextureAsset,
  font: *FontAsset,
  model: *ModelAsset,

  fn T(variant: AssetVariant) type {
    return @FieldType(AssetVariant, @tagName(variant));
  }
};

fn load(comptime variant: AssetVariant, ...) ?*AssetVariant.T(variant) {
  ...
}

probably yeah, the main differences emerge from the fact that zig unions have named fields

[unkempt apex]

what I would suggest as a small improvement to this code would be to replace comptime T: type in load with comptime kind: enum { texture, font, model }

[blissful marsh]

wait I actually messed up there, the return type is just AssetVariant since I @unionInit. The return asset in the if case should also return union then

[blissful marsh]

that's basically what I offered in the example, just took the enum from union

[unkempt apex]

yeah

but if the metaprogramming isn't appreciated, can always peel it back

[blissful marsh]

In that case i'd duplicate the tag names and have explicitely defined enum type, then use it as backing enum for union. But then got to duplicate names in both types

[unkempt apex]

const AssetKind = enum {
    tex,
    font,
    model,

    fn T(comptime kind: AssetKind) type {
        return switch (kind) {
            .tex => TextureAsset,
            .font => FontAsset,
            .model => ModelAsset,
        };
    }
};

fn load(cache: *ResourceCache, comptime kind: AssetKind, path: []const u8) ?*kind.T() {
    // etc
}

can also just do that if the pursuit is in avoiding metaprogramming using @FieldType

[blissful marsh]

this would be the corrected version, which returns union value instead of the underlying value:

const std = @import("std");

// Asset stubs
const TextureAsset = opaque{};
const FontAsset = opaque{};
const ModelAsset = opaque{};

const AssetVariant = union(enum) {
    tex: *TextureAsset,
    font: *FontAsset,
    model: *ModelAsset,
};
const AssetVariantEnum = std.meta.Tag(AssetVariant);

const ResourceCache = struct {
    fn load(
        self: *ResourceCache,
        comptime variant: AssetVariantEnum,
        path: []const u8
    ) ?AssetVariant {
        if (self.get(path)) |res| {
            switch(res) {
                inline else => |asset, tag| {
                    std.debug.assert(tag == variant);
                    std.log.info("asset {} {s} already existing", .{ variant, path });
                    return @unionInit(AssetVariant, @tagName, asset);
                },
            }
        }

        const FieldType = std.meta.FieldType(AssetVariant, variant);
        const asset = std.testing.allocator.create(FieldType);
        return @unionInit(AssetVariant, @tagName(variant), asset);
    }
};

In C union can be implicitely initialised from just one of the accepted type values. In zig union variants are named, so here have to explicitely initialise which tag and what value to use.

What that means is that zig unions might have multiple differently named variants that store same datatype:

const BinaryBlob = union(enum) {
  text: []u8,
  executable: []u8,
  image: []u8,
};

in C you couldn't distinguish these variants without adding your own logic for active tag handling

[unkempt apex]

often missed that in Odin

in order to accomplish that you have to define a newtype for all same-typed variants

[blissful marsh]

also this means that in your code example this would be more powerful assert in the existing resource case - perhaps types of assets match, but tags do not. Here you can check if the loaded asset at some path has the same exact tag as used in current load invocation

but okay, you already define unique asset type per asset

[random garnet]

I'll say it again, this is not good code for me, and i don't want to return an union, what is even the point of comptime, anyways, when the switch inline else was introduced it was an indication that we should stop there, for my current use case, odin's approach is nicer; zig is not, so i won't use union for that use case

[random garnet]

will they ban me if i open an issue and request improvements to union

[unkempt apex]

it is unlikely you would garner any real support without at least first engaging on zulip, after learning about the culture of contributions and proposals - importantly, opening a proposal is only allowed if you can get a member of the core team to champion yoyur proposal

[random garnet]

oof

i'll pretend it's a bug

[blissful marsh]

lately andrew hasn't been patient so probably not the best moment to provoke him with such proposals on github issues. Though i guess at least could try to debate union ux improvements on zig zulip, then at least could reach out to core contributors

[random garnet]

what's zulip? webcam visio conference? i am not interested in that

slack alternative? hmm, if it's text based i may give it a try

i'm not sure they'll like my tone tho

[blissful marsh]

https://zsf.zulipchat.com/

[random garnet]

you have been pretty patient with me

[blissful marsh]

That could very well be the case judging from your current attitude

[random garnet]

this union thing has been frustrating, typing code that doesn't work the way i want is pretty annoying

[blissful marsh]

i mean I get why they don't tolerate such brashness and I've been through plenty of languages that I can get frustrations of how good features of one language don't map well to other

zig is at least 70% there for me so whatever drawbacks are copeable compared to what other alternatives there are. for now i trust the zsf to continue developing zig in the direction it's been going

[random garnet]

@blissful marsh found a way

if i were zig author, i'd make this a builtin

a clean one liner

@unionSet(asset), it should infer type like it does for casts

the reason i don't like all this crap is 1: it is not readable 2: i duplicate compiler's work, therefore it both sucks for my sanity and my build time

[unkempt apex]

That doesn't work with how zig's unions are

Zig's unions aren't distinguished by variant type, they're distinguished by a tag

This is a fundamental distinction between zig and odin

[random garnet]

if i can do it, the compiler can do it

[unkempt apex]

It actually can't, not unambiguously

[random garnet]

why can i

[unkempt apex]

Because you have explicit knowledge of your specific type. But not all types are equal or can be treated the same as you treat this one. Take union(enum) { foo: u8, bar: u8 }, two tags with the same type, here the compiler can't make a decision for you on which tag is active

Again, if you replace comptime T: type with comptime kind: AssetTag or the like, you can get the same thing but in natural zig. In fact it simplifies the code greatly:

const asset = @field(res.variant, @tagName(kind));`

assuming variant is a tagged union

[random garnet]

the compiler can always say: ambigious, and then user fall back to the junk code i had to write

[vivid breach]

pub const Ressource = struct {
    variant: union(enum) {
        font: *FontAsset,
    },
};

pub const RessourceCache = struct {
    map_ressources: std.StringHashMap(*Ressource),

    pub fn load(self: *RessourceCache, comptime T: type, path: []const u8) ?*T {
        return switch ((self.map_ressources.get(path) orelse return null).variant) {
            inline else => |value| value,
        };
    }
};

[unkempt apex]

That's just not how it's designed, they're distinguished by tag, not type

Just because that's what you prefer doesn't mean it makes sense in the language

[random garnet]

i don't want to return null, i want then to proceed with asset creation

[random garnet]

doesn't change the fact that it doesn't play nicely with other prominent zig's feature: comptime

i value nice UX, there is value in having some extra compiler help

i may be a bad programmer, but language should guide me to write good code

@vivid breach i ended up doing this: #1389861073817178142 message

[unkempt apex]

It does though, you just seem to dislike the comptime features provided for this

[unkempt apex]

Like this, it's functionally equivalent to what you ended up with, except better, because it doesn't force odin-like union assumptions into zig

[random garnet]

i fail to visualize this, if you have time and don't mind please showcase how cool your solution is

if i have to duplicate something, i'm not interested

[vivid breach]

how did you achieve this in odin?

[random garnet]

self.ressources[path] = asset

wait

wrong pasta

glad it wasn't a password LUL

asset.variant = asset

you now understand why i was mad

[vivid breach]

but is it a tagged union?

[random garnet]

no idea

i'm a odin newbie

[vivid breach]

I think unions are tagged by default in odin, so I guess you cant have 2 union fields with the same type in odin or there is another construct for this

[random garnet]

history of this code is:

D: raw Ressource with threads -> Zig: async IO with raw Ressources -> Odin: explored this union thing -> trying to backport these new ideas back to Zig

[vivid breach]

in zig unions can have a same type multiple times so there is no direct relationship between the tag and the type
this is why you have to precise the tag when initializing a tagged union in zig

[random garnet]

i'm still not sold on using union, so if any of you have better design idea, i'm listening

[vivid breach]

if you want the same behavior as in odin you would need to wrap the union in some way, basically making your own type.
(or have more code around the union init like you already did)

[random garnet]

i think that's what InKryption suggested with an enum tag

[vivid breach]

I sugest a utility function

fn initUnionFromValue(UnionT: type, value: anytype) UnionT {
    // could add checking that the union doesnt duplicate types
    return for (@typeInfo(UnionT).@"union".fields) |field| {
        if (field.type == @TypeOf(value)) break @unionInit(UnionT, field.name, value);
    } else @compileError("no union field with type of value");
}

[random garnet]

utility function for such thing is not a solution for me sorry

[vivid breach]

?? why
you could also put the function in the union's namespace

pub const Variant = union(enum) {
    font: *FontAsset,

    pub fn init(value: anytype) Variant {
        //...
    }
};

and then you can just do .init(value);

[blissful marsh]

This really feels like hammering a square in a circle hole

[unkempt apex]

const Asset = union(enum) {
    const Kind = std.meta.Tag(Asset);
    tex: TextureAsset,
    font: FontAsset,
    model: ModelAsset,
};

fn load(self: *ResourceCache, comptime kind: Asset, path: []const u8) !?*@FieldType(Asset, @tagName(kind)) {
    const T = @FieldType(Asset, @tagName(kind));
    const gop = try self.map_resources.getOrPut(path);
    errdefer if (!gop.found_existing) std.debug.assert(self.map_resources.remove(path));

    if (gop.found_existing) {
        const res = gop.value_ptr;
        const asset = &@field(res, @tagName(kind));
        rt.dbg.info("asset {} {s} already existing, rc: {}", .{ T, path, asset.base.ref_count });
        asset.base.ref_count += 1;
        return asset;
    }

    gop.value_ptr.* = try self.resource_allocator.create(Asset);
    errdefer self.resource_allocator.destroy(gop.value_ptr.*);

    gop.value_ptr.* = @unionInit(Asset, @tagName(kind), undefined);
    const asset = &@field(gop.value_ptr, @tagName(kind));

    // initialize asset.* however you need to, ie `asset.base = .{...}`
    // you can `switch (kind)` here with each prong doing the specific initialization procedure required for each
    // for the rest of their fields

    // once you're done, return
    return asset;
}

made some assumptions about some details, but that's the gist of it

if you would still refuse @FieldType on some strange ideological grounds, then not much else you can do

[random garnet]

i already don't like it, it return !?*@FieldType(Asset, @tagName(kind)), it's not readable, ?*T is readable, and i don't want to change that

[unkempt apex]

what's the difference

[random garnet]

i don't like maintaining code i don't like to read, makes me pause and waste time trying to udnerstand what it does

i know i am nitpicky

[unkempt apex]

is it really that un-obvious to you? It says what it's doing in the name

@FieldType - does this not literally tell you the exact thing it does?

[random garnet]

it's too much noise

[neat crest]

Thats why you use a utility with a name that makes sense to you.

[random garnet]

utility function is the same as putting problem under carpet and moving on

[unkempt apex]

it's called abstracting details away that are unimportant to you

[blissful marsh]

in the end I also don't really understand why use unions here when you don't want to return the union. If you really want to stick to comptime T: type, you can just store assets as pointers to opaque{}

[neat crest]

Zig is noisy to be explicit - it's just the way it is.

[blissful marsh]

then ptrcast them

so it's better that builtin puts things under carpet even though they could be functionally the same

[random garnet]

i'll have to think about this one, it could simplify this whole module

builtin means i don't duplicate compiler's work, and it's code that i don't have to maintain myself

if api change, i don't want to touch it

therefore builtin wins

[unkempt apex]

you're not "duplicating the compiler's work", you're doing things that aren't the compiler's job in the first place

[blissful marsh]

because you're not really benefitting from the unions here. I guess only the ability to assert that the fetched resources have consistently the same asset type

[random garnet]

besides, utility function for assigning an union is like admiting a language defect

[blissful marsh]

btw it has been considered a feature in zig that comptime can avoid unnescessary extra code in the compiler, such as the formatted print from std, whereas C needs to do compiler magic for printf to work

[unkempt apex]

you mean for getting the type of a union field?

[random garnet]

my brain have a really hard time accepting writting more code than asset.variant = asset

the more code i have to write to fix an issue, the more annoyed i am

[vivid breach]

the inability to have the same type multiple times in a tagged union could be considered defect a language as well if you need it.
This is just a very normal tradeoff between use casses that ultimately doesnt matter because a little bit more code can solve the issue.

[unkempt apex]

It's a different language, things are different. If you want Odin, write Odin

[random garnet]

the logic is no different tho

conceptually, on a high level, when describing your intent, it shouldn't be any more code than that, the compiler is smart enough

[unkempt apex]

It depends on what your intent is

[random garnet]

compiler knows, there is no duplicate type

[unkempt apex]

Your intent makes no sense in zig

[unkempt apex]

It doesn't matter, it's not now zig is structured

[random garnet]

i am annoyed as much as zig devs are annoyed at writting down code to make the compiler do the check

[unkempt apex]

You don't need to, you could just do it the way that doesn't require you to write a check if you weren't ideologically opposed to using a built-in with a self-explanatory name

Anyway, clearly this is a fruitless conversation, you can't be pleased

[vivid breach]

A special case for rarely hit scenario would be kind of strange.

I suggest this:

pub const Foo = union(enum) {
    foo: i32,
    bar: f32,

    pub fn init(value: anytype) Foo {
        return inline for (@typeInfo(Foo).@"union".fields) |field| {
            if (field.type == @TypeOf(value)) break @unionInit(Foo, field.name, value);
        } else @compileError("no union field with type of value");
    }
};

pub fn main() !void {
    const value: i32 = 10;
    const baz: Foo = .init(value);

    std.debug.print("{}\n", .{baz});
}

[blissful marsh]

right, if commiting to use unions, then use them and return union instead of union field's type. You can add methods to the union to have common way to interact with assets or unwrap them