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

Stop rendering clear lights #4225

Conversation

kwvanderlinde
Copy link
Collaborator

@kwvanderlinde kwvanderlinde commented Jul 31, 2023

Identify the Bug or Feature request

Fixes #4213

Description of the Change

The primary change is to avoid building DrawableLight objects for clear lights (lights with a null paint).

This required changes to how player darkness is rendered as we can no longer rely on each darkness being represented as DrawableLights (darkness can be clear just like lights). So instead we now find the entire darkened area from the ZoneView and render that as black without needing to know about the individual darkness sources.

For a performance win, we also now avoid rendering the colour aspect of darkness when the player is not a GM, since only GMs can see them anyways.

Some minor edits made along the way were:

  • Removing some System.out.println() calls that were spamming my console logs.
  • Removing some redundant null checks in ZoneRenderer regarding the visible area.

Possible Drawbacks

Anyone with a clear light may have gotten used to the 1.13 behaviour of treating it as black. Can be worked around by explicitly defining the light as black.

Documentation Notes

Lights will be rendered a clear if no colour is given, e.g., Transparent 20: scale circle 20+100

Release Notes

  • Fixed a bug where clear lights were being rendered like black lights.

This change is Reviewable

`ZoneView.getVisibleArea(PlayerView)` is guaranteed to return a non-`null` result. This method is now marked as
`@Nonnull` to make this clear, and we can avoid several `null` checks in `ZoneRenderer` as a result.
We already cached total lit areas, and this change also adds darkness. Just as for lit areas, the necessary computation
is done lazily.

The terminology has also been cleaned up: the lit area used to be termed "visible area", but `Illumination` doesn't
actually know anything about visibility. So `Illumination.getVisibleArea()` has been renamed to
`Illumination.getLitArea()`, and other uses of the term "visible" have also been removed.
Non-GM players need to have darkness rendered as blackness rather than as lights. We used to accomplish this by
rendering each darkness as a regular light (as the GM would see it), then would paint each darkness as black
afterwards.

Now we don't even represent the darkness as a `DrawableLight` unless the player is a GM. This way we aren't wasting time
compositing darkness lights that won't even be visible in the end. We also don't render the darknesses individually, but
instead render the entire darkened area as black in a single pass.
Clear lights no longer get mapped to `DrawableLight` instances, making it so that they no longer get rendered.

With this change, `DrawableLight`s can no longer have `null` paints. This means we no longer need to support "default
paints" in `ZoneRenderer.renderLightOverlay()`, so that parameter and related logic have been removed.
@kwvanderlinde kwvanderlinde force-pushed the bugfix/4213-colourless-lights-as-transparent branch from 00f6a8e to d488009 Compare July 31, 2023 00:34
@cwisniew cwisniew added this pull request to the merge queue Jul 31, 2023
Merged via the queue into RPTools:develop with commit 40f9132 Jul 31, 2023
4 checks passed
@kwvanderlinde kwvanderlinde deleted the bugfix/4213-colourless-lights-as-transparent branch July 31, 2023 06:00
@cwisniew cwisniew added the feature Adding functionality that adds value label Aug 2, 2023
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.

[Bug]: Lights with no color show up as black on the map
2 participants