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

Don't let light sources above a roof illuminate the space below the roof #8996

Closed
dev7355608 opened this issue Mar 6, 2023 · 3 comments
Closed
Assignees
Labels
lighting/fog Issues related to dynamic lighting or fog of war tiles Issues related to Tiles and their appearance ux Issues focused on user experience improvements

Comments

@dev7355608
Copy link

User Experience

Just like light sources under a roof don't illuminate the space above the roof, light sources above a roof shouldn't illuminate the space below the roof (the light is blocked by the roof in both directions). We can use the new weather mask channel to make this happen.

In this example the light source is above the roof. Once the token steps under the roof, the roof is hidden, but the space below the roof is filled with the light that should be blocked by the roof, which is still there.
roof_before

After the suggested changes the light doesn't illuminated the space under the roof as expected.
roof_after

The changes are simple. Replace

step(depthColor.g, depthElevation)

by

step(depthColor.g, depthElevation) * (1.0 - step((255.5 / 255.0) - depthColor.b, depthElevation))

in the light shaders. One more change is required: the global light sources is special and should illuminate everything even below roofs (maybe it wouldn't have to behave that way, but changing this behavior would be breaking). So GlobalLightSource needs to override _updateCommonUniforms to set the depthTexture to the empty texture, which makes all depth checks pass (no additional uniform required).

  /** @override */
  _updateCommonUniforms(shader) {
    super._updateCommonUniforms(shader);
    shader.uniforms.depthTexture = PIXI.Texture.EMPTY;
  }
@aaclayton
Copy link
Contributor

Worth considering I think

@aaclayton aaclayton added lighting/fog Issues related to dynamic lighting or fog of war ux Issues focused on user experience improvements tiles Issues related to Tiles and their appearance labels Mar 6, 2023
@caewok
Copy link

caewok commented Mar 7, 2023

Wouldn’t light config need to have an elevation setting so the GM could set the light elevation for non-global lights? Otherwise, how would you light the inside of buildings with roofs?

To be clear, I think this is generally a good idea. And I don’t have laptop access right now, so for all I know elevation config is already in v11.

@dev7355608
Copy link
Author

There's no need for that. Almost nothing would change for non-Levels users, because in core all nonglobal lights are always at elevation 0 and therefore underneath roofs. The only exceptions are token lights, which have the same elevation as the token. Here's an example with ambient light (green, elevation 0) and a light-emitting token (red, elevation 20) on top of the roof:
roof_example

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lighting/fog Issues related to dynamic lighting or fog of war tiles Issues related to Tiles and their appearance ux Issues focused on user experience improvements
Projects
Status: Done
Development

No branches or pull requests

4 participants