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

Change BEAM lights and sights to accept width instead of arc #4516

Merged
merged 4 commits into from
Dec 23, 2023

Conversation

kwvanderlinde
Copy link
Collaborator

@kwvanderlinde kwvanderlinde commented Dec 1, 2023

Identify the Bug or Feature request

Improves #4448

Description of the Change

Instead of an arc= parameter, BEAM lights and sights now take a width= parameter measured in map units (same as ranges).

Also includes clean up for SightType and fixed sight output in getInfo("campaign").

Possible Drawbacks

It's possible to make beams so wide they don't look like beams anymore.

Documentation Notes

BEAM lights can take width= parameter to set the thickness of the beam.

Release Notes

N/A


This change is Reviewable

@bubblobill
Copy link
Collaborator

The code looks good to me, but I couldn't find a default value for width in the parsing (I just got out of bed so may have missed it). A default width should be thin enough to be useful but thick enough to be visible.
Something like this, fiddlethe value to produce a satisfactory result.

this.width = (width == 0) ? 0.2 : width;
this.arcAngle = (arcAngle == 0) ? 90 : arcAngle;

Aim for something like this
image

@kwvanderlinde
Copy link
Collaborator Author

@bubblobill Any default value or lower bound for width won't necessarily be meaningful for all maps as they can have wildly settings. So instead I changed the meaning of width=0 (or any tiny width) to mean a rectangle that's just about as thin as possible, something we can determine dynamically.

image

@cwisniew cwisniew added the feature Adding functionality that adds value label Dec 7, 2023
@kwvanderlinde kwvanderlinde force-pushed the feature/4448-beam-width branch from 8f69c53 to 7052fc0 Compare December 7, 2023 17:21
Width is measure in map units, so works the same as ranges.
- Remove unused methods and constructors.
- Remove all mutators (the few in use were only at construction time anyways).
- Move the constructor to be above the methods, not in the middle.
- Move the personal light source to be the last constructor parameter, for better literals.
- `SightType.shape` is no longer allowed to be `null`.
- Remove default constructor, use the real constructor in `fromDto()`
- Initial sight type improvements:
  - They are written as `SightType`, not `Object[]`.
  - Darkvision no longer coupled to the Generic light sources
When getting its shaped area, the width will be forced to be at least one pixel wide.
@kwvanderlinde kwvanderlinde force-pushed the feature/4448-beam-width branch from 248210c to 374af69 Compare December 16, 2023 11:03
@cwisniew cwisniew added this pull request to the merge queue Dec 23, 2023
Merged via the queue into RPTools:develop with commit 894a298 Dec 23, 2023
4 checks passed
@kwvanderlinde kwvanderlinde deleted the feature/4448-beam-width branch January 9, 2024 09:12
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
Development

Successfully merging this pull request may close these issues.

4 participants