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.

Sign up for GitHub

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

Jump to bottom

Lamp Bloom, Exposure and Glare #24534

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

Lamp Bloom, Exposure and Glare #24534

wants to merge 35 commits into from
+830 −91

Conversation

MrRomainzZ
Copy link
Contributor

@MrRomainzZ MrRomainzZ commented Mar 10, 2024 •
edited

What Does This PR Do

Adapts TauCetiStation/TauCetiClassic#11943

Requires DB update (at least, if no one tells me how to avoid it)

Adds new effects for lamps by adding bloom, exposure and glare. Everything may be toggled on or off. Admins even have Bloom Edit button in debug section to test different parameters (or to play with players)

Why It's Good For The Game

Beauty

Images of changes

https://youtu.be/KuCSgDfvQ3M

v2
https://youtu.be/1cOyj2OkLEA

v3
Comment below

Testing

A lot on my localhost

Changelog

🆑
add: Added togglable through preferences bloom, exposure and glare effects on lamps
/:cl:

@meow20
Copy link
Contributor

meow20 commented Mar 10, 2024

That looks amazing! Does it have any performance impact?

@MrRomainzZ
Copy link
Contributor Author

@meow20 I guess it does, but I haven't noticed any on my GTX 650 Ti :)
And you can turn it off whenever you'd like

@Arthri
Copy link
Contributor

Arthri commented Mar 10, 2024

I believe that the glare could be better adjusted to the light's brightness. There's a few awkward scenarios such as the Paramedic's room where the light is somewhat dim but the glare cuts through it with a large brightness

image

@m-dzianishchyts
Copy link
Contributor

vM0aVX

@SynthTwo
Copy link
Contributor

This PR is an absolute beauty, I agree with Athri's concerns however, looks particularly awkward in darker rooms.

@MrRomainzZ
Copy link
Contributor Author

@Arthri I'll look into that after fixed tests

@dj-34 dj-34 mentioned this pull request Mar 10, 2024
Open
@meow20
Copy link
Contributor

meow20 commented Mar 10, 2024

The "circadian rhythms" bull might be active and screwing up things a bit?

@ParadiseSS13-Bot ParadiseSS13-Bot added -Status: Awaiting type assignment This PR is waiting for its type to be assigned internally SQL Change This PR modifies the game database. This PR must go through the host. Sprites This PR modifies the game sprites TGUI This PR modifies TGUI, will conflict labels Mar 10, 2024
1080pCat
1080pCat reviewed Mar 10, 2024
View reviewed changes
Copy link
Contributor

@1080pCat 1080pCat left a comment

Choose a reason for hiding this comment

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

woe, white space upon ye.
i've tried to avoid ones that appear to be for code reading improvement, probably worth a second opinion

code/__HELPERS/matrices.dm Outdated Show resolved Hide resolved
code/__HELPERS/matrices.dm Outdated Show resolved Hide resolved
code/__HELPERS/matrices.dm Show resolved Hide resolved
code/__HELPERS/matrices.dm Show resolved Hide resolved
code/__HELPERS/matrices.dm Outdated Show resolved Hide resolved
code/modules/client/preference/preferences_toggles.dm Outdated Show resolved Hide resolved
code/modules/lighting/lighting_atom.dm Outdated Show resolved Hide resolved
code/modules/lighting/lighting_atom.dm Outdated Show resolved Hide resolved
code/modules/lighting/lighting_atom.dm Outdated Show resolved Hide resolved
code/modules/lighting/lighting_atom.dm Outdated Show resolved Hide resolved
@Henri215 Henri215 added the Feature This PR is a new addition to the game label Mar 10, 2024
@ParadiseSS13-Bot ParadiseSS13-Bot added the Configuration Change This PR changes the game configuration files. Please run via the host. label Mar 10, 2024
@MrRomainzZ
Copy link
Contributor Author

Now it's time to fix that brightness...

@ParadiseSS13-Bot ParadiseSS13-Bot added -Status: Awaiting approval This PR is waiting for approval internally and removed -Status: Awaiting type assignment This PR is waiting for its type to be assigned internally labels Mar 10, 2024
@github-actions github-actions bot added the Merge Conflict This PR is merge conflicted label Mar 10, 2024
@MrRomainzZ
Copy link
Contributor Author

@Arthri, @SynthTwo, what will you say about this one? Changed glare logic a bit and tweaked lights
https://youtu.be/1cOyj2OkLEA

@MrRomainzZ
Tweaking here and there
33dc10b
@MrRomainzZ
Merge branch 'official_paradise' into light-update to fix merge conflict
b6e7b7d 
# Conflicts:
#	code/modules/lighting/lighting_atom.dm
#	tgui/public/tgui.bundle.js
@Arthri
Copy link
Contributor

Arthri commented Mar 11, 2024

Awesome work!

@github-actions github-actions bot added Merge Conflict This PR is merge conflicted and removed Merge Conflict This PR is merge conflicted labels Mar 11, 2024
@github-actions github-actions bot added Merge Conflict This PR is merge conflicted and removed Merge Conflict This PR is merge conflicted labels Apr 24, 2024
@github-actions github-actions bot removed the Merge Conflict This PR is merge conflicted label Apr 28, 2024
@volas
Copy link
Contributor

volas commented Apr 28, 2024

Just to note, we still not resolved problems with lighting effects above objects. It can be noticeable if, for example, you have a tree or something else blocking the lamp. TG solved similar problems with emissive lighting/planes. Before we adapted some similar solution, our compromise was that we can live with it and just do some map fixes.

And we also dimmed our base light globally for better effect and replaced floors with more clean ones

If you interested

изображение

@Henri215 Henri215 requested a review from 1080pCat April 30, 2024 01:42
@github-actions github-actions bot added the Merge Conflict This PR is merge conflicted label Apr 30, 2024
@github-actions GitHub Actions
Copy link
Contributor

This pull request seems to be stale as there have been no changes in 14 days, please make changes within 7 days or the PR will be closed. If you believe this is a mistake, please inform a development team member on Discord.

@github-actions github-actions bot added the Stale This PR has been left inactive and requires an update. label May 15, 2024
@AyIong
Copy link
Contributor

AyIong commented May 16, 2024

2+ month... Sad

@github-actions github-actions bot removed the Stale This PR has been left inactive and requires an update. label May 17, 2024
@github-actions GitHub Actions
Copy link
Contributor

This pull request seems to be stale as there have been no changes in 14 days, please make changes within 7 days or the PR will be closed. If you believe this is a mistake, please inform a development team member on Discord.

@github-actions github-actions bot added the Stale This PR has been left inactive and requires an update. label May 31, 2024
@github-actions github-actions bot removed the Merge Conflict This PR is merge conflicted label May 31, 2024
Burzah
Burzah approved these changes May 31, 2024
View reviewed changes
Copy link
Member

@Burzah Burzah left a comment

Choose a reason for hiding this comment

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

TGUI and SQL changes look good!

tgui/packages/tgui/interfaces/BloomEdit.js Outdated Show resolved Hide resolved
tgui/packages/tgui/interfaces/BloomEdit.js Outdated Show resolved Hide resolved
@Burzah Burzah removed the Stale This PR has been left inactive and requires an update. label May 31, 2024
@AffectedArc07
Copy link
Member

I think I might rip the SQL changes from this PR into their own PR so this can be TM'd easier.

@MrRomainzZ
Copy link
Contributor Author

I'm done with current conversations (except that layers issue). Need reviews

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FunnyMan3595 FunnyMan3595 FunnyMan3595 approved these changes

@Burzah Burzah Burzah approved these changes

@AffectedArc07 AffectedArc07 Awaiting requested review from AffectedArc07 AffectedArc07 is a code owner

@lewcc lewcc Awaiting requested review from lewcc

@1080pCat 1080pCat Awaiting requested review from 1080pCat

Requested changes must be addressed to merge this pull request.

Assignees

@AffectedArc07 AffectedArc07

Labels
-Status: Awaiting review This PR is awaiting review from the review team Configuration Change This PR changes the game configuration files. Please run via the host. Feature This PR is a new addition to the game Sprites This PR modifies the game sprites SQL Change This PR modifies the game database. This PR must go through the host. TGUI This PR modifies TGUI, will conflict
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet