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

Introduce ShapeType.LINE #4449

Merged
merged 7 commits into from
Nov 24, 2023
Merged

Introduce ShapeType.LINE #4449

merged 7 commits into from
Nov 24, 2023

Conversation

bubblobill
Copy link
Collaborator

@bubblobill bubblobill commented Nov 21, 2023

Identify the Bug or Feature request

resolves #4448

Description of the Change

Added ShapeType.LINE for use with vision and lights.
Allows defining a line (really a rectangle) that covers up to 180 degrees of arc of the token's size.

Possible Drawbacks

none foreseen

Documentation Notes

New Shape Type for vision and lights, Line.
Refer to i18n light definitions.
image

Release Notes

Added new shape "Line" for vision and lighting.


This change is Reviewable

Copy link
Collaborator

@kwvanderlinde kwvanderlinde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll have a closer look at this later, but a few things that stand out to me right away:

First, there's some new commented-out code that should be removed.

Second, IsometricGrid.getShapedArea() also needs an update otherwise lines will be circles there.

Finally, I don't like arc as the way to control line width. It's not intuitive to interpret as a user, and it makes the grid size an arbitrary maximum width for the line. That also means a line light is dependent on grid settings which is not good. Instead I would suggest a width parameter that allows the width to be written direcltly in map distances. E.g., a very D&D-ish 5'x30' line could be written as line width=5 30.

@bubblobill
Copy link
Collaborator Author

Good catch, in my quest for impacts I missed the isometric override.

I was envisioning it as an actual line, as it is born of a need for something to assist with determining LoS.
I started out drawing a line but this obviously doesn't work as an area. With no control over stroke/width to improve visibility I needed to use a rectangle.

Navigating dTOs is a nightmare for me and introducing another parameter is more than I am capable of. I used arc= because it was already there, limits width to token size, and makes sense (admittedly, with a picture).

Your example D&D line isn't really a line; it's actually a proper rectangle and warrants a separate shape (conceptually speaking). I might change my LINE to BEAM to leave room for adding a not-really-a LINE later, as line as an area does occupy a place in the minds of most of our users as an area-of-effect template.

Shall modify my code and go again.

Undid unnecessary changes to LightSourceCreator
Added BEAM to getShapedArea in IsometricGrid
Removed commenting on some code in Grid
Fixed issue with BEAM not being parsed for sight in Campaign Properties.
@bubblobill
Copy link
Collaborator Author

Revisions done

@kwvanderlinde
Copy link
Collaborator

I think this is shaping up nicely. I like the term "beam" 🙂

If I provide the changes for it, would you be alright with moving away from arc?

@bubblobill
Copy link
Collaborator Author

Happy to use an alternative to arc if I don't have to do the work : )

Copy link
Collaborator

@kwvanderlinde kwvanderlinde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, looks good, then! I'll take care of changing arc -> width in a follow-up PR.

@cwisniew cwisniew added this pull request to the merge queue Nov 24, 2023
Merged via the queue into RPTools:develop with commit 1768ead Nov 24, 2023
4 checks passed
@bubblobill bubblobill deleted the LineLight branch November 25, 2023 06:03
@bubblobill
Copy link
Collaborator Author

Awesome, looks good, then! I'll take care of changing arc -> width in a follow-up PR.

Give me a heads-up when you do and I will update the campaign properties dialogue descriptive text

@kwvanderlinde
Copy link
Collaborator

Awesome, looks good, then! I'll take care of changing arc -> width in a follow-up PR.

Give me a heads-up when you do and I will update the campaign properties dialogue descriptive text

Sounds good. I'm hoping to have some time for it this weekend.

@cwisniew cwisniew added the feature Adding functionality that adds value label Mar 16, 2024
@cwisniew cwisniew changed the title All this in order to introduce ShapeType.LINE Introduce ShapeType.LINE Mar 16, 2024
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.

[Feature]: Line shape type for vision/lights
3 participants