-
-
Notifications
You must be signed in to change notification settings - Fork 28
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
First pass at editor GUI #421
base: main
Are you sure you want to change the base?
Conversation
- Create stub app for Editor - Begin work on new OpenXR client - Create "editor" feature flag to hide new work
- Define a way for the editor and clients to talk to eachother - Simple request/response protocol that's transport agnostic - Works perfectly and has no flaws
f7d0c62
to
153463f
Compare
.vscode/settings.json
Outdated
"rust-analyzer.check.extraArgs": [ | ||
"--target-dir", | ||
"C:/windows/temp/rust-analyzer-check" | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't look portable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'm thinking I should just remove this file from the repo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not convinced the best way forward is to remove the whole file. These settings are good for keeping the files tidy and consistent. Can we at least resolve this in a separate PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the problem is that there's always going to be a case where a developer might have overrides that are unique to their specific environment, right? For me the rust analyzer one is super common that I need per project to avoid locking issues.
I think anything related to style etc should probably best be left to clippy, otherwise there's no way to, for example. get developers who aren't using VSCode to have their code correctly formatted.
static FRAGMENT_SHADER: &'_ [u8] = include_bytes!("shaders/triangle.frag.spv"); | ||
static VERTEX_SHADER: &'_ [u8] = include_bytes!("shaders/triangle.vert.spv"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These files are not checked in
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah! Good pickup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may want to add a .gitignore
with exceptions for these files. Checking in files that are ignored by git can lead to confusing stuff, eg. local changes are not picked up by git even though the file is checked in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hrm, I've found that it's usually fine after you've explicitly added that file to the index, but I've added an exception to the gitignore just to be on the safe side
fea90af
to
901a0c9
Compare
Need to fix the dependencies so we're not pulling in UDS on Linux. |
} else { | ||
let _name = String::from_utf8_unchecked(name.to_vec()); | ||
unsafe extern "system" fn bang() -> Result { | ||
panic!("UNIMPLEMENTED FUNCTION!"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could potentially add the name to the panic to make it easier to track down.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a fun one! So I've tried to pass this in but for whatever reason - potentially FFI weirdness, I am clearly not clever enough to understand those machinations - it isn't possible to return a function that will panic with the correct function name.
What is this related to?
Closes #419
What's changed?