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

Consistently render movement with different fog and vision settings #4558

Conversation

kwvanderlinde
Copy link
Collaborator

@kwvanderlinde kwvanderlinde commented Dec 13, 2023

Identify the Bug or Feature request

Addresses #4081
Addresses #4544

Description of the Change

For GMs, nothing about movement rendering should have changed, so the path and labels should always be rendered in GM view if available. For players, the new approach boils down to this:

  1. If fog is enabled, do not render the path or labels outside the exposed area for the current view.
  2. If vision is enabled, do not render the path or labels outside the visible area for the current view.

Those cases are not mutually exclusive, and they apply regardless of server settings, i.e., whether Individual FOW is toggled or not.

I've kept the owner logic in place from before. I.e., if the GM is moving your token, you can still see the labels even through hard FOW. I'm not actually sure this is desirable, but it looked like there was effort put into making it work this way in the firs place, so I kept it for now.

A bit of cleanup as well:

  • We no longer try to set the clip in the middle of the render loop as this was inconsistent (depended on which tokens were seen first) and the new approach allows it to be done once for all tokens.
  • We don't try to short-circuit the loop in the case that nothing will be rendered. That check is brittle, makes it more difficult to see what the core logic is, and masks potential problems. It's also not really gaining us anything, at least not in sane scenarios.
  • Some logic was pulled out into new functions (shouldShowMovementLabels(), calculateTraveledDistance()). This makes the label casework in showBlockedMoves a lot easier to see, though the token rendering is still pretty involved.
  • showBlockedMoves was the only remaining use of exposedScreenArea, which it no longer needs. So that field has been removed. The related visibleScreenArea is also used in token rendering, so that has not been removed.

Possible Drawbacks

Should be none.

Documentation Notes

When someone else (e.g., the GM) is dragging a player's token, the player will no longer be able to see the movement details in areas not visible to them. Here is a screenshot of this on a map with vision set to Night:
image
The current player owns the Hero token in the center, while the GM is dragging one of the Elf tokens behind the wall. Only the path in the visible area is actually visible to the player. And since the token is currently being dragged behind the wall, the player also cannot the movement details.

This is the same situation, but with vision set to Off:
image
Since the token would be visible to the player behind the wall, the path is also visible along with the movement details.

If fog is enabled for the map, then movement details will never be shown when a token is dragged under hard fog of war.

Release Notes

  • Fixed a bug where movement labels were rendered over player's FOW.

This change is Reviewable

For players, movement rendering now works as follows:
- If fog is enabled, do not render the path or labels outside the exposed area.
- If vision is enabled, do not render the path or labels outside the visible area.

Both cases can exist at the same time, in which case the intersection of the areas is what matters.
@cwisniew cwisniew added the bug label Dec 15, 2023
@cwisniew cwisniew added this pull request to the merge queue Dec 15, 2023
Merged via the queue into RPTools:develop with commit f0a7a7a Dec 15, 2023
4 checks passed
@kwvanderlinde kwvanderlinde deleted the bugfix/4081-4544-labels-outside-visible-area branch December 15, 2023 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

2 participants