Skip to content
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

Odin Bindings - First Pass #5

Merged
merged 18 commits into from
Aug 30, 2024
Merged

Odin Bindings - First Pass #5

merged 18 commits into from
Aug 30, 2024

Conversation

nicbarker
Copy link
Owner

This is a draft in-progress PR containing clay bindings for Odin, will tidy up and write docs once it's in a complete state.

@nicbarker nicbarker changed the title Draft odin bindings Draft Odin bindings Aug 28, 2024
@Dudejoe870
Copy link
Contributor

Dudejoe870 commented Aug 28, 2024

Hey, I know this is a draft but I was looking through it and noticed you forgot to add @(default_calling_convention = "c") to the foreign block! Odin has a custom calling convention, so this could lead to potential issues!
(the docs that reference this are here https://odin-lang.org/docs/overview/#foreign-system)

EDIT: I am technically wrong, foreign blocks actually do default to the C calling convention by default. My bad!

@laytan
Copy link
Contributor

laytan commented Aug 28, 2024

Hey, I looked it over and here are some notes, mostly on Odin things:

  • I would do Vector2 :: [2]c.float, this allows taking advantage swizzling and array programming
  • Same thing with Color :: [4]c.float
  • You can take advantage of attributes to do away with all the wrappers you got:
@(link_prefix="Clay_")
foreign Clay {
    MinMemorySize :: proc() -> c.uint32_t ---
}

This will link to Clay_MinMemorySize but the symbol will be MinMemorySize in Odin.

  • There is @(private) which you could use to not allow calling the private symbols at all, not sure if applicable here but good to know
  • Where you do @(deferred_none=blah) you may want to @(require_results, deferred_none=blah) this will force the "handling" of the return value so clay.Rectangle(...) would not compile, that should hint the user to doing the scope thing with if clay.Rectangle(...) {, they could still do _ = clay.Rectangle(...) of course
  • I would make ClayString -> String so you don't have clay.ClayString(...)

Thanks for doing this BTW! I am excited to use this.

Also, getting this working with Odin's WASM story should be pretty do-able since you don't use the stdlib / emscripten.

@nicbarker
Copy link
Owner Author

@laytan Thanks so much for taking a look! I really appreciate it. Some great TIL's in there, I'll get them in ASAP.

@nicbarker
Copy link
Owner Author

Bindings are working and I've tested it out with a raylib example and it works great!
Will get onto writing the docs and getting the example worked out a little better tomorrow.

@nicbarker nicbarker closed this Aug 30, 2024
@nicbarker nicbarker deleted the 011-odin-bindings branch August 30, 2024 06:45
@nicbarker nicbarker restored the 011-odin-bindings branch August 30, 2024 06:45
@nicbarker nicbarker reopened this Aug 30, 2024
@nicbarker nicbarker changed the title Draft Odin bindings Odin Bindings - First Pass Aug 30, 2024
@nicbarker
Copy link
Owner Author

These are in a decently working state, I've replicated the clay website using Odin + Raylib:

Screen.Recording.2024-08-30.at.1.40.53.PM.mov

Going to merge them now and work a bit more on the docs / finessing later.

@Dudejoe870
Copy link
Contributor

This looks great, awesome work! On both the bindings, and the library itself!

@nicbarker nicbarker merged commit 3f7005f into main Aug 30, 2024
@nicbarker nicbarker deleted the 011-odin-bindings branch August 30, 2024 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants