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

Set background color on lightning #2628

Merged
merged 31 commits into from
Mar 16, 2024
Merged

Conversation

tobbi
Copy link
Member

@tobbi tobbi commented Sep 6, 2023

Fixes #1243

@tobbi
Copy link
Member Author

tobbi commented Sep 6, 2023

2023-09-06.15-43-19.mp4

@tobbi
Copy link
Member Author

tobbi commented Sep 6, 2023

2023-09-06.22-35-46.mp4

@Rusty-Box
Copy link
Member

Not sure how far we can go in terms of brighting up the screen, but the flash should be very bright! Perhaps working with something on an "additive layer" would help out. I also think that it looks much nicer if the flash appears immideatly upon lighting strike but then fades out rather than dissappearing instantly again.

@Rusty-Box
Copy link
Member

Rusty-Box commented Sep 6, 2023

Something along the lines of this, if brighter:

lightning_before_after

EDIT: There really should be a strong difference between the two states

@tobbi tobbi force-pushed the set_background_color_on_lightning branch from dbed397 to cac5e9c Compare January 12, 2024 18:53
@tobbi tobbi requested review from Rusty-Box and Vankata453 January 12, 2024 23:00
@Vankata453
Copy link
Member

Personally I find the thunder too bright, making it uneasy on the eyes. Would there be an option to turn this off?

@tobbi tobbi requested a review from MatusGuy January 12, 2024 23:27
src/object/thunderstorm.cpp Outdated Show resolved Hide resolved
src/video/color.cpp Outdated Show resolved Hide resolved
@tobbi
Copy link
Member Author

tobbi commented Jan 21, 2024

Personally I find the thunder too bright, making it uneasy on the eyes. Would there be an option to turn this off?

This was Rusty's idea. Battle with him.

@Rusty-Box Rusty-Box requested a review from MatusGuy January 22, 2024 14:24
@Rusty-Box
Copy link
Member

It appears the white surface drawn when lightning strikes isn't even colored white. It looks rather gray. Even when the ambient light is set to r=1, g=1, b=1. Is its alpha value set to 1 when it flashes?

@Rusty-Box
Copy link
Member

Ideally, if we get the flash to be white we may not even have to tamper with the ambient light at all during the flash as the blend mode "add" brightens it all up anyways (unless I am mistaking, though that's how it works with a pure white gradient set to "add" and a z-layer of 500).

@tobbi
Copy link
Member Author

tobbi commented Jan 24, 2024

It starts at 1 and then quickly levels off to 0.

@Rusty-Box
Copy link
Member

Huh, it doesn't look like it though at all. I did notice something else though too. You should remove the stuff related to const Color Color::LIGHTNING_HIT_COLOR(0.6, 0.6, 0.6, 0.5); because it actually makes a fully bright sector darker when lightning strikes

@Rusty-Box
Copy link
Member

Not sure if that fixes all of it though since the white surfaces appears as not white even in a fully bright sector too.

@Rusty-Box
Copy link
Member

This is how it is supposed to look like. To give you a visual reference!
Note: I used a scripted Gradient object for this!

shocking_lightning.mp4

@tobbi tobbi force-pushed the set_background_color_on_lightning branch from f62ddbc to f8507bd Compare January 28, 2024 12:50
@tobbi
Copy link
Member Author

tobbi commented Jan 28, 2024

2024-01-28.17-20-08.mp4

@tobbi tobbi requested a review from MatusGuy January 28, 2024 17:10
@Rusty-Box
Copy link
Member

I'm still convied that the blending mode didn't really apply. For comparison I set up a Gradient Object with the same values as the drawn lighning flash you added (and on the same z-layer -> 500). But as you can see it is not the same. I don't know why and rn my only assuption is that the code may not support blending modes for you methode of drawing it.

Gradient set to r:1, g:1, b:1, a:0.9 and blend mode "Additive":
screenshot000011

Lighning Flash in this PR, which is supposedly also "Additive":
screenshot000012

Copy link
Member

@MatusGuy MatusGuy left a comment

Choose a reason for hiding this comment

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

I think this code could be much better, but this works

src/object/thunderstorm.cpp Outdated Show resolved Hide resolved
src/object/thunderstorm.hpp Outdated Show resolved Hide resolved
@tobbi
Copy link
Member Author

tobbi commented Jan 28, 2024

I think this code could be much better, but this works

In what way could the code be better?

@tobbi tobbi force-pushed the set_background_color_on_lightning branch from 1ce0b1b to 9d72c02 Compare March 2, 2024 15:09
@tobbi
Copy link
Member Author

tobbi commented Mar 2, 2024

2024-03-02_17-35-20.mp4

Copy link
Member

@Rusty-Box Rusty-Box left a comment

Choose a reason for hiding this comment

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

There we go.

src/object/thunderstorm.cpp Show resolved Hide resolved
src/object/thunderstorm.cpp Outdated Show resolved Hide resolved
src/video/canvas.cpp Outdated Show resolved Hide resolved
@mrkubax10 mrkubax10 requested a review from Vankata453 March 11, 2024 08:54
Copy link
Member

@Vankata453 Vankata453 left a comment

Choose a reason for hiding this comment

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

Codewise looks good.

@tobbi tobbi merged commit 8528399 into master Mar 16, 2024
33 checks passed
@mrkubax10 mrkubax10 deleted the set_background_color_on_lightning branch March 16, 2024 21:28
@Rusty-Box Rusty-Box mentioned this pull request Apr 25, 2024
@mrkubax10 mrkubax10 removed the status:needs-review Work needs to be reviewed by other people label Jun 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Level "Shocking" is a bit unfair w/o sound
5 participants