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

Restrict each drawable light to its lumens level #4940

Conversation

kwvanderlinde
Copy link
Collaborator

@kwvanderlinde kwvanderlinde commented Sep 19, 2024

Identify the Bug or Feature request

Addresses #4936

Description of the Change

When using the "overtop" lighting style, weak lights will no longer overlap with strong lights. This lighting style is more closely related to mechanics, and so it gets confusing when different lumens levels have overlapping colours.

When using the "environmental" lighting style, weak lights and strong lights can still overlap. This lighting style is meant to be more natural, so it's not desirable to have a hard cutoff where weak lights meet strong lights.

In either case, the interaction between darkness and light is the same: the weaker one will be cutoff by the stronger so there is no overlap.

Possible Drawbacks

Someone may have found a use for overlapping lights in the "overtop" style, and that is being removed here.

Documentation Notes

When using the "overtop" lighting style, each lumens level is like a separate layer for lights. Different lumens levels will not overlap with each. Instead, the strongest lumens level will "win" over weaker lumens levels. If darkness and light are tied, the darkness wins.

As an example, we can augment the default torch with lumens levels to represent bright and dim:

Torch - 20: circle 20+100 40#000000+50

If two tokens have torches, we can see that the bright areas are merged, and the dim areas are merged, but the dim areas do not overlap with the bright areas since the bright areas have a higher lumens value.
image

Release Notes

  • Change light rendering so that lights of different lumens do not overlap and blend when using "overtop" lighting style.

This change is Reviewable

@kwvanderlinde kwvanderlinde self-assigned this Sep 19, 2024
@kwvanderlinde kwvanderlinde force-pushed the feature/4936-render-lights-per-lumens-level branch 2 times, most recently from 0c714d9 to 064937a Compare September 19, 2024 23:39
@kwvanderlinde kwvanderlinde added the feature Adding functionality that adds value label Sep 19, 2024
@kwvanderlinde kwvanderlinde marked this pull request as ready for review September 20, 2024 00:01
@cwisniew cwisniew added this pull request to the merge queue Sep 24, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Sep 24, 2024
Environmental lighting continues to allow overlap between lumens levels as these are meant to be more natural and less
mechanical regarding their rendering.
@kwvanderlinde kwvanderlinde force-pushed the feature/4936-render-lights-per-lumens-level branch from e75018c to 740aec3 Compare September 26, 2024 04:02
@kwvanderlinde
Copy link
Collaborator Author

@cwisniew Looks like the verification got confused with the recent mac changes, and also affected the merge queue. Looks like we'll need manual intervention to merge this one (or I can close this PR and open an identical one).

@cwisniew
Copy link
Member

@cwisniew Looks like the verification got confused with the recent mac changes, and also affected the merge queue. Looks like we'll need manual intervention to merge this one (or I can close this PR and open an identical one).

I can push it through

@cwisniew cwisniew enabled auto-merge September 26, 2024 05:06
@cwisniew cwisniew disabled auto-merge September 26, 2024 05:10
@cwisniew cwisniew merged commit 8b200f3 into RPTools:develop Sep 26, 2024
4 checks passed
@kwvanderlinde kwvanderlinde deleted the feature/4936-render-lights-per-lumens-level branch September 26, 2024 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Adding functionality that adds value
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

2 participants