Skip to content

revise lua script to use zig tool chain for stride #1

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jazzay
Copy link

@jazzay jazzay commented Oct 1, 2023

These changes are required to support building NativePath and Celt for newer platforms like osx-arm64.

This will have no impact on Stride itself until I push the changes to deps, which would be into the Stride repo.

Copy link
Member

@manio143 manio143 left a comment

Choose a reason for hiding this comment

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

Overall I think it looks good. Left some small comments.
Not sure how this Lua is part of the build to be honest, so an example on how to invoke it would be nice.

@jazzay
Copy link
Author

jazzay commented Oct 1, 2023

Overall I think it looks good. Left some small comments. Not sure how this Lua is part of the build to be honest, so an example on how to invoke it would be nice.

This is old build tooling that predates the formation of the OSS Xenko. You can see where it is used to build NativePath inside deps/NativePath/build.bat. I have also added a new build.bat for the Celt dependency that uses this as well, since we need an update to that dep to support the new platform targets. I will be putting up a separate PR in the main Stride repo that illustrates changes related to this soon. Sadly this is in a separate repo.

@jazzay
Copy link
Author

jazzay commented Oct 1, 2023

Overall I think it looks good. Left some small comments. Not sure how this Lua is part of the build to be honest, so an example on how to invoke it would be nice.

This is old build tooling that predates the formation of the OSS Xenko. You can see where it is used to build NativePath inside deps/NativePath/build.bat. I have also added a new build.bat for the Celt dependency that uses this as well, since we need an update to that dep to support the new platform targets. I will be putting up a separate PR in the main Stride repo that illustrates changes related to this soon. Sadly this is in a separate repo.

Actually it was Celt dep that was using this already, I've added a new build.bat for NativePath that's incoming.

Copy link
Member

@manio143 manio143 left a comment

Choose a reason for hiding this comment

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

I'm happy with it but would have @xen2 look it over. Thanks for addressing my comments.

@xen2
Copy link
Member

xen2 commented Oct 2, 2023

Sorry, don't hesitate to add me as reviewer in case I miss the message next time!

@xen2 xen2 self-requested a review October 2, 2023 10:57
Copy link
Member

@xen2 xen2 left a comment

Choose a reason for hiding this comment

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

LGTM!
Maybe I better create a zig-cross branch on this repo so you that you can merge the successive PR?

@jazzay
Copy link
Author

jazzay commented Oct 2, 2023

LGTM! Maybe I better create a zig-cross branch on this repo so you that you can merge the successive PR?

I think we can just merge straight into master even with these new commits?

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