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 Camera and Shadertoy examples #25

Merged
merged 15 commits into from
Aug 20, 2020
Merged

Update Camera and Shadertoy examples #25

merged 15 commits into from
Aug 20, 2020

Conversation

zotho
Copy link
Contributor

@zotho zotho commented Jul 18, 2020

}
}

#[macroquad::main(window_conf)]
Copy link
Owner

Choose a reason for hiding this comment

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

Nice, I really like how it looks! :)

Cargo.toml Outdated
@@ -23,3 +23,6 @@ glam = {version = "0.8", features = ["scalar-math"] }
image = { version = "0.22", default-features = false, features = ["png_codec", "tga"] }
megaui = { version = "=0.2.11" }
macroquad_macro = { version = "0.1", path = "macroquad_macro" }

[patch.crates-io]
Copy link
Owner

Choose a reason for hiding this comment

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

This probably should not be a part of this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I will remove that. Just trying to make cross-compile, checks work correctly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

async fn main() {
let ferris = load_texture("rust.png").await;
let ferris = load_texture("examples/rust.png").await;
Copy link
Owner

Choose a reason for hiding this comment

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

IIRC examples/rust.png will not work on wasm - there is no examples folder for textures

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case have to run executables from examples directory.
Otherwise just run cargo run --example shadertoy works correctly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also there an issue of this problem #26

@@ -34,12 +34,19 @@ pub fn main(attr: TokenStream, item: TokenStream) -> TokenStream {
}
modified.extend(source);

let (method, ident) = match attr.into_iter().next() {
Copy link
Owner

Choose a reason for hiding this comment

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

Could you make separate PR with just macro changes?
It would be way easier to review!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok

Copy link
Contributor Author

@zotho zotho Jul 22, 2020

Choose a reason for hiding this comment

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

Moved macro changes to #28 PR

let mut offset = (0., 0.);

'main: loop {
for &key in get_down_keys(&mut down_keys).iter() {
Copy link
Owner

Choose a reason for hiding this comment

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

I am not quite sold on the idea of get_down_keys

As for me, this example would be a bit more readable with just a number of

if is_key_down(Key::Whatever) {
  target.1 -= 0.1;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was just to write more compact code. Which is easy to add new cases. See diff: 1bd4e3e#diff-6a6a8253e0291eb499839f6f1f4c15c3L35
However, I removed it.

(_x, y) if y != 0.0 => {
// Normalise mouse wheel values is browser (chromium: 53, firefox: 3)
#[cfg(target_arch = "wasm32")]
let y = if y < 0.0 {-1.0} else if y > 0.0 {1.0} else {0.0};
Copy link
Owner

Choose a reason for hiding this comment

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

It looks like rustfmt would not be happy with this line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

KeyCode::Up => offset.1 += 0.1,
KeyCode::Down => offset.1 -= 0.1,
#[cfg(not(target_arch = "wasm32"))]
KeyCode::Q | KeyCode::Escape => break 'main,
Copy link
Owner

Choose a reason for hiding this comment

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

I think we will need some exit mechanism in macroquad.
Until it's done - I would try to avoid 'main on loops to not confuse people too much :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Label removed


match mouse_wheel() {
(_x, y) if y != 0.0 => {
// Normalise mouse wheel values is browser (chromium: 53, firefox: 3)
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe do normalization on macroquad side, not on the user code side?
Not necessarily as a part of this PR though

Copy link
Contributor Author

@zotho zotho Aug 20, 2020

Choose a reason for hiding this comment

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

Maybe move normalization into mouse_wheel function?

@not-fl3
Copy link
Owner

not-fl3 commented Jul 24, 2020

Hi!
I like how the new camera example looks! But maybe keep both, the old one for super-simple camera concept showcase and a new one, with some additional logic behind?

@saucesaft
Copy link

This pull request looks promising, what can i do to help?

@zotho
Copy link
Contributor Author

zotho commented Aug 20, 2020

Hi!
I like how the new camera example looks! But maybe keep both, the old one for super-simple camera concept showcase and a new one, with some additional logic behind?

@not-fl3 I moved new code to camera_transformations example.

@not-fl3 not-fl3 merged commit 4ebff32 into not-fl3:master Aug 20, 2020
@zotho zotho mentioned this pull request Aug 20, 2020
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