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

Update to newest orca bindings (UI Update) #4887

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Skytrias
Copy link
Contributor

@Skytrias Skytrias commented Feb 26, 2025

Update to a ui revision update in orca land: https://github.com/orca-app/orca/releases/tag/test-release-4f124dd346

  • Large update to the foreign bindings since the UI system was reworked
  • Examples will have to be updated on the other repo at some point

@laytan
Copy link
Collaborator

laytan commented Feb 26, 2025

Simplified the macros.odin file to exclude core:fmt usage, as it complains on the latest odin master branch

Huh, I don't see any usage of fmt in the package?

@Skytrias
Copy link
Contributor Author

Skytrias commented Feb 28, 2025

Simplified the macros.odin file to exclude core:fmt usage, as it complains on the latest odin master branch

Huh, I don't see any usage of fmt in the package?

it used to exist in the macros.odin file

Edit: Ah maybe it was some old macros file i used to have in the other repo my bad! but yea that had resulted in problems

@Skytrias Skytrias changed the title Update to newest orca bindings (UI Update), remove logging due to cyclic fmt import Update to newest orca bindings (UI Update) Feb 28, 2025
@laytan
Copy link
Collaborator

laytan commented Mar 1, 2025

Yeah, at the very beginning I remember we had the fmt import here so you could do orca.errorf etc. but that quickly gave problems like you mention.

I do think log_error, log_warning etc. (the ones not using fmt) that you removed in this pr were valuable and harmless though.

@@ -578,8 +569,28 @@ foreign {
// A unicode codepoint.
utf32 :: rune

// This enum declares the possible return status of UTF8 decoding/encoding operations.
utf8_status :: enum u32 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could the OC_UTF8_ prefix be removed in the members here?

@@ -1607,7 +1772,7 @@ mouse_state :: struct {
wheel: vec2,
using _: struct #raw_union {
buttons: [5]key_state,
using _: struct {
_: struct {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should using really be removed here?

@@ -1678,10 +1861,94 @@ ui_size :: struct {
minSize: f32,
}

ui_overflow :: enum u32 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the OVERFLOW_ prefix be removed from these members?

ATTRIBUTE_COUNT = 27,
}

ui_attribute_mask :: enum u32 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the MASK_ prefix be removed from these members?

ui_box_size :: [2]ui_size

ui_box_floating :: [2]bool

ui_draw_mask :: enum u32 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the DRAW_MASK_ prefix be removed from these members?

@@ -17,7 +17,7 @@ _now :: proc "contextless" () -> Time {
_sleep :: proc "contextless" (d: Duration) {
// NOTE: no way to sleep afaict.
if d > 0 {
orca.log_warning("core:time 'sleep' is unimplemented for orca")
// orca.log_warning("core:time 'sleep' is unimplemented for orca")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably not be commented out?

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.

2 participants