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

Color correction #9587

Merged
merged 17 commits into from
Oct 2, 2023
Merged

Conversation

DrDuckedGoose
Copy link
Contributor

@DrDuckedGoose DrDuckedGoose commented Aug 3, 2023

About The Pull Request

Adds a level of color correction to station areas. It's not overly complex or probably true color correction, but it works for our purposes. This also includes mapping helpers, one of these can be placed in an area to change that entire area's correction.

Essentially it's a subtle color-shift to areas

Why It's Good For The Game

Emphasizes the visual atmosphere of a given area

Testing Photographs and Procedure

Screenshots&Videos

(null)scrnshot1

New Project (1)

New Project (2)

New Project (3)

New Project (4)

New Project

Changelog

🆑
add: Adds color correction to areas
/:cl:

@PowerfulBacon
Copy link
Member

The transition effect needs to be extremely slow and the colours need to be very, very subtle to prevent this being too noticeable

@DrDuckedGoose DrDuckedGoose marked this pull request as ready for review August 9, 2023 07:14
@DrDuckedGoose
Copy link
Contributor Author

This is probably fine for now

code/game/sound.dm Outdated Show resolved Hide resolved
code/modules/mob/living/living_movement.dm Outdated Show resolved Hide resolved
code/modules/mob/living/living.dm Show resolved Hide resolved
@Penwin0 Penwin0 added the Test Merged This PR is currently in rotation label Aug 11, 2023
Copy link
Member

@PowerfulBacon PowerfulBacon left a comment

Choose a reason for hiding this comment

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

The fade needs to be way longer, at least 30 seconds.
Only warm/dark colours should be used for colour fading. Purples and greens don't work with colour correction very well.

@DrDuckedGoose
Copy link
Contributor Author

increased fade time & changed hydro & science to cold_ish

@itsmeow itsmeow removed the Test Merged This PR is currently in rotation label Aug 13, 2023
@PowerfulBacon
Copy link
Member

Putting this on testmerge again

@PowerfulBacon PowerfulBacon added the Test Merged This PR is currently in rotation label Sep 3, 2023
Copy link
Member

@Penwin0 Penwin0 left a comment

Choose a reason for hiding this comment

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

My code reviews were adressed, and I think it looks good in game

@github-actions
Copy link

github-actions bot commented Sep 3, 2023

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@Cartlord
Copy link

Cartlord commented Sep 4, 2023

Will users have an option to disable this, as they do with ambient occlusion?

@DrDuckedGoose
Copy link
Contributor Author

Will users have an option to disable this, as they do with ambient occlusion?

No, but if someone hates it that much, they can surely ask someone else(not me) to do that

@CydiaLamiales

This comment was marked as off-topic.

@PowerfulBacon
Copy link
Member

I would rather not bloat preference anymore with unnecessary options that players don't know the effect of.

@PowerfulBacon PowerfulBacon removed the Test Merged This PR is currently in rotation label Sep 4, 2023
@Cartlord
Copy link

Cartlord commented Sep 5, 2023

Will users have an option to disable this, as they do with ambient occlusion?

No, but if someone hates it that much, they can surely ask someone else(not me) to do that

Why do you, the person adding the feature, expect someone else to add a remedy if you end up creating a problem?

I would rather not bloat preference anymore with unnecessary options that players don't know the effect of.

That sounds like it could be fixed by adding a description to the pref

@DrDuckedGoose

This comment was marked as off-topic.

@CydiaLamiales
Copy link
Contributor

Will users have an option to disable this, as they do with ambient occlusion?

No, but if someone hates it that much, they can surely ask someone else(not me) to do that

Why do you, the person adding the feature, expect someone else to add a remedy if you end up creating a problem?

There is no problem. Only you hate that the game doesn't look like it's 2014.

@PowerfulBacon
Copy link
Member

I still don't want to add too many settings. I don't want players to have the option to make their game look worse and if this looks worse than without it and is too obvious, I won't be merging it.

@Cartlord
Copy link

Cartlord commented Sep 6, 2023

Will users have an option to disable this, as they do with ambient occlusion?

Game not being fullbright burns the eyes of the /vg/ player

What of it? I personally dislike color correction as a concept, and I think adding it is a bad idea because nothing in the game was made with it in mind - so either it makes things look noticeably worse since it's fucking up the palettes, or it's so inconsequential that it might as well not be there - but I'm just one guy, which is why I'd rather it be a preference.

I still don't want to add too many settings. I don't want players to have the option to make their game look worse and if this looks worse than without it and is too obvious, I won't be merging it.

By this logic, why do we have preferences for things like Point Sampling vs Bilinear Filtering vs Nearest Neighbor, or for ambient occlusion, or to enable and disable tint from glasses? If there's an objective best way for the game to look, why not remove those settings so people have to play the game in the way that (you think) looks best?

@github-actions
Copy link

github-actions bot commented Sep 6, 2023

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@PowerfulBacon
Copy link
Member

Point Sampling vs Bilinear Filtering vs Nearest Neighbor, or for ambient occlusion, or to enable and disable tint from glasses?

Ambient occlusion could genuinely be removed, tint from glasses just gets in the way so kind of needs to be turned off and while I would love to remove the setting to change scaling mode, different monitor resolutions look blurry on certain settings so you need to choose the one for you

@Rukofamicom Rukofamicom added this pull request to the merge queue Oct 2, 2023
Merged via the queue into BeeStation:master with commit c34678d Oct 2, 2023
8 checks passed
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.

7 participants