Screen shake example by moosama76 · Pull Request #15567 · bevyengine/bevy

moosama76 · 2024-10-01T16:35:47Z

Objective

Closes #15564

Solution

Adds a screen shake example.

moosama76 · 2024-10-01T16:36:09Z

Closes #15564

alice-i-cecile · 2024-10-01T16:37:59Z

@m-edlund could I get your review here?

moosama76 · 2024-10-01T16:46:44Z

A ci test fails probably because of the way I use rng, how can I fix this?

alice-i-cecile · 2024-10-01T16:50:56Z

Swap to a seeded RNG, picking an arbitrary number for the seed rather than thread_rng. See https://rust-random.github.io/book/guide-seeding.html

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>

github-actions · 2024-10-01T19:06:41Z

The generated examples/README.md is out of sync with the example metadata in Cargo.toml or the example readme template. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

atornity

Reading the issue #15564, it says we should be using Camera::sub_camera_view to do this rather than transform directly. I suggested some changes that switches it to that. Feel free to disregard this if you think we should be using Transform instead.

atornity · 2024-10-01T19:17:40Z

+fn screen_shake(
+    time: Res<Time>,
+    mut screen_shake: ResMut<ScreenShake>,
+    mut query: Query<&mut Transform, With<Camera>>,


Suggested change

mut query: Query<&mut Transform, With<Camera>>,

mut query: Query<&mut Camera>,

atornity · 2024-10-01T19:19:17Z

+        for mut transform in query.iter_mut() {
+            transform.translation.x = rng.gen_range(-shake_amount..shake_amount);
+            transform.translation.y = rng.gen_range(-shake_amount..shake_amount);
+        }


Suggested change

for mut transform in query.iter_mut() {

transform.translation.x = rng.gen_range(-shake_amount..shake_amount);

transform.translation.y = rng.gen_range(-shake_amount..shake_amount);

}

for mut camera in query.iter_mut() {

let sub_view = camera.sub_camera_view.as_mut().unwrap();

sub_view.offset = Vec2::new(

rng.gen_range(-shake_amount..shake_amount),

rng.gen_range(-shake_amount..shake_amount),

);

}

m-edlund · 2024-10-01T19:30:22Z

+    fn new(intensity: f32, duration: f32) -> Self {
+        ScreenShake {
+            intensity,
+            duration,
+            timer: Timer::from_seconds(0.0, TimerMode::Once), // Start timer at 0
+        }
+    }


If intensity and duration is set on every call of start_shake, setting the values in new serves no purpose and can be replaced by deriving Default for ScreenShake. As a bonus the Resource can then also be created with init_resource instead of insert_resource.

m-edlund · 2024-10-01T19:31:14Z

+#[derive(Resource)]
+struct ScreenShake {
+    intensity: f32,
+    duration: f32,


Duration is already encoded in the timer, and can thus be omitted.

github-actions · 2024-10-01T19:46:31Z

The generated examples/README.md is out of sync with the example metadata in Cargo.toml or the example readme template. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

m-edlund · 2024-10-01T19:59:56Z

+    if screen_shake.timer.finished() {
+        return; // No shake if the timer has finished
+    }


There should be something to reset the transformation / the offset back to the default, once the screen shake timer has finished. Else the screen might end up stuck slightly off-center when the timer ends, depending on how intense the screen was shook.

janhohenheim

Thanks a lot for the contribution! I think having a screen shake example would be a great addition :)
This particular solution falls into a few traps discussed in this talk: Math for Game Programmers: Juicing Your Cameras With Math.
I would suggest you give it a watch and then overhaul the implementation. Feel free to ping me afterwards for a review.
Sorry I'm not enumerating my particular issues, but I don't think there is much value in doing that when the talk I linked does a way better job of explaining these things than I could hope to do :)

moosama76 · 2024-10-01T20:45:47Z

@janhohenheim thank you I'll watch it

m-edlund

Looks good to me

moosama76 · 2024-10-06T20:42:11Z

@m-edlund could I get your review here?

He approved the current version

pablo-lua

I think this would be good if the user could increase or decrease those variables with key bindings, but not a blocker for this PR, LGTM

alice-i-cecile · 2024-10-07T15:14:50Z

@moosama76 you'll need to adjust this example to match the latest changes to how cameras work :) Let us know if you need a hand.

moosama76 · 2024-10-07T16:40:24Z

@moosama76 you'll need to adjust this example to match the latest changes to how cameras work :) Let us know if you need a hand.

Sure

moosama76 · 2024-10-07T19:53:07Z

@alice-i-cecile I need help
sub_camera_view position not changing after new camera syntax

atornity

You changed the size/full_size of SubCameraView (it was defualt previously, witch is 1.0:1.0 I think) and since offset is relative to those values, you also have to change MAX_OFFSET.

alice-i-cecile · 2024-10-07T20:39:41Z

@Jondolf I think there's a bug introduced in #15641 that needs resolution. Adding this to the milestone so we don't accidentally ship it.

atornity · 2024-10-07T20:42:05Z

+
+// screen_shake parameters, maximum addition by frame not actual maximum overall values
+const MAX_ANGLE: f32 = 0.5;
+const MAX_OFFSET: f32 = 0.5;


Suggested change

const MAX_OFFSET: f32 = 0.5;

const MAX_OFFSET: f32 = 500.0;

Jondolf · 2024-10-07T21:09:30Z

@alice-i-cecile Is there an issue? It works for me at the latest commit, assuming this is what it's supposed to look like:

2024-10-08.00-06-33.mp4

alice-i-cecile · 2024-10-07T21:13:50Z

Looks like it's been resolved :)

Screen shake example

97aeed8

alice-i-cecile added A-Rendering Drawing game state to the screen C-Examples An addition or correction to our examples D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 1, 2024

alice-i-cecile requested a review from janhohenheim October 1, 2024 16:38

documentation

3deb310

moosama76 added 2 commits October 1, 2024 20:55

use seeded rnd, add intensity and duration as constants

f0c84f8

formatting

b6c4075

alice-i-cecile reviewed Oct 1, 2024

View reviewed changes

Comment thread examples/camera/screen_shake.rs Outdated

alice-i-cecile reviewed Oct 1, 2024

View reviewed changes

Comment thread Cargo.toml Outdated

moosama76 and others added 2 commits October 1, 2024 21:51

Update examples/camera/screen_shake.rs

be71d66

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>

Update Cargo.toml

4916f61

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>

alice-i-cecile reviewed Oct 1, 2024

View reviewed changes

Comment thread examples/camera/screen_shake.rs Outdated

atornity suggested changes Oct 1, 2024

View reviewed changes

Comment thread examples/camera/screen_shake.rs Outdated

atornity reviewed Oct 1, 2024

View reviewed changes

m-edlund suggested changes Oct 1, 2024

View reviewed changes

janhohenheim suggested changes Oct 1, 2024

View reviewed changes

janhohenheim added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 1, 2024

Rework using all modifications mentioned

6e630dd

moosama76 force-pushed the screen_shake branch from 9a35c08 to 6e630dd Compare October 3, 2024 19:44

fmt

b778aad

m-edlund approved these changes Oct 4, 2024

View reviewed changes

moosama76 requested a review from janhohenheim October 5, 2024 18:47

pablo-lua approved these changes Oct 6, 2024

View reviewed changes

pablo-lua added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Oct 6, 2024

alice-i-cecile approved these changes Oct 7, 2024

View reviewed changes

alice-i-cecile added this pull request to the merge queue Oct 7, 2024

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 7, 2024

moosama76 and others added 2 commits October 7, 2024 19:42

Merge branch 'main' into screen_shake

dc63896

Camera position not changing after new camera syntax

87b6ec3

atornity reviewed Oct 7, 2024

View reviewed changes

alice-i-cecile added S-Needs-Help The author needs help finishing this PR. and removed S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it labels Oct 7, 2024

alice-i-cecile added this to the 0.15 milestone Oct 7, 2024

atornity reviewed Oct 7, 2024

View reviewed changes

keep up with new camera updates

34e155a

alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Help The author needs help finishing this PR. labels Oct 7, 2024

alice-i-cecile removed this from the 0.15 milestone Oct 7, 2024

alice-i-cecile added this pull request to the merge queue Oct 7, 2024

Merged via the queue into bevyengine:main with commit 91bed8c Oct 7, 2024

	mut query: Query<&mut Transform, With<Camera>>,
	mut query: Query<&mut Camera>,

-        for mut transform in query.iter_mut() {
-            transform.translation.x = rng.gen_range(-shake_amount..shake_amount);
-            transform.translation.y = rng.gen_range(-shake_amount..shake_amount);
-        }
+        for mut camera in query.iter_mut() {
+            let sub_view = camera.sub_camera_view.as_mut().unwrap();
+            sub_view.offset = Vec2::new(
+                rng.gen_range(-shake_amount..shake_amount),
+                rng.gen_range(-shake_amount..shake_amount),
+            );
+        }

Uh oh!

Conversation

moosama76 commented Oct 1, 2024 • edited by alice-i-cecile Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Objective

Solution

Uh oh!

moosama76 commented Oct 1, 2024

Uh oh!

alice-i-cecile commented Oct 1, 2024

Uh oh!

moosama76 commented Oct 1, 2024

Uh oh!

alice-i-cecile commented Oct 1, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Oct 1, 2024

Uh oh!

atornity left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

atornity Oct 1, 2024

Choose a reason for hiding this comment

Uh oh!

atornity Oct 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

m-edlund Oct 1, 2024

Choose a reason for hiding this comment

Uh oh!

m-edlund Oct 1, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Oct 1, 2024

Uh oh!

m-edlund Oct 1, 2024

Choose a reason for hiding this comment

Uh oh!

janhohenheim left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

moosama76 commented Oct 1, 2024

Uh oh!

m-edlund left a comment

Choose a reason for hiding this comment

Uh oh!

moosama76 commented Oct 6, 2024

Uh oh!

pablo-lua left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alice-i-cecile commented Oct 7, 2024

Uh oh!

moosama76 commented Oct 7, 2024

Uh oh!

moosama76 commented Oct 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

atornity left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alice-i-cecile commented Oct 7, 2024

Uh oh!

atornity Oct 7, 2024

Choose a reason for hiding this comment

Uh oh!

Jondolf commented Oct 7, 2024

Uh oh!

alice-i-cecile commented Oct 7, 2024

Uh oh!

Reviewers

moosama76 commented Oct 1, 2024 •

edited by alice-i-cecile

Loading

atornity left a comment •

edited

Loading

atornity Oct 1, 2024 •

edited

Loading

janhohenheim left a comment •

edited

Loading

moosama76 commented Oct 7, 2024 •

edited

Loading

atornity left a comment •

edited

Loading