Skip to content

Add Translate C to build.zig, remove @cImport from graphics.zig - #3039

Merged
IntegratedQuantum merged 15 commits into
PixelGuys:masterfrom
AfmanS:replace_cImport
May 8, 2026
Merged

Add Translate C to build.zig, remove @cImport from graphics.zig#3039
IntegratedQuantum merged 15 commits into
PixelGuys:masterfrom
AfmanS:replace_cImport

Conversation

@AfmanS

@AfmanS AfmanS commented May 5, 2026

Copy link
Copy Markdown
Contributor

Progress towards #2930.
graphics.zig seemed the most complex so it's a good first file to discuss the move.
Mostly followed example from https://ziglang.org/download/0.16.0/release-notes.html#cImport-Moving-to-Build-System.
All C import consts in graphics.zig were renamed to c.
I don't have the knowledge to write better comments on the c.h file.
Builds, passes tests and plays fine on Windows x86. I have no way to test that that MACH preproc actually works on macOS.

@BoySanic BoySanic moved this to Low Priority in PRs to review May 5, 2026
Comment thread src/graphics.zig
Comment thread src/c.h Outdated
Comment thread src/graphics.zig
@IntegratedQuantum IntegratedQuantum moved this from Low Priority to In review in PRs to review May 6, 2026
@AfmanS
AfmanS requested a review from IntegratedQuantum May 7, 2026 13:58
Comment thread src/graphics.zig Outdated
Comment thread src/c.h Outdated
Comment thread src/c.h Outdated
Comment thread src/c.h Outdated
AfmanS and others added 4 commits May 7, 2026 15:47
Co-authored-by: IntegratedQuantum <43880493+IntegratedQuantum@users.noreply.github.com>
Co-authored-by: IntegratedQuantum <43880493+IntegratedQuantum@users.noreply.github.com>
Co-authored-by: IntegratedQuantum <43880493+IntegratedQuantum@users.noreply.github.com>
Comment thread src/client/Entity.zig Outdated
const game = main.game;
const graphics = main.graphics;
const c = graphics.c;
const c = @import("c");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you please reorder them to be in a new block? Each module import and subimports should be in its own block.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tried changing just Entity.zig. Is this how it should be?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yes

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Comment thread src/gui/windows/performance_graph.zig Outdated
}

pub fn render() void {
const c = @import("c");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

All module importants should be done at the top of the file, please apply this to the other files as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

@AfmanS
AfmanS requested a review from IntegratedQuantum May 7, 2026 17:31
Comment thread src/renderer.zig Outdated
const models = @import("models.zig");
const network = @import("network.zig");
const settings = @import("settings.zig");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is actually incorrect, these are not real imports, they are legacy code (see #2950) and will be converted to main. at some point, so please leave them like they were and just put c at the bottom.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is this acceptable?

Comment thread src/graphics.zig Outdated
@cInclude("hb.h");
@cInclude("hb-ft.h");
});
const c = @import("c");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you move the c at the bottom, always below main?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@AfmanS
AfmanS requested a review from IntegratedQuantum May 8, 2026 09:50
@IntegratedQuantum
IntegratedQuantum merged commit 5286061 into PixelGuys:master May 8, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants