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

[#893] Add system for selecting summoning placement #3235

Merged
merged 2 commits into from
Mar 21, 2024
Merged

Conversation

arbron
Copy link
Collaborator

@arbron arbron commented Mar 11, 2024

The TokenPlacement class right now is small and only delegates to the TokenPlacementTemplate which is responsible for rendering and returning placement information. With the V12 improvements to placeables hopefully we'll be able to shift more logic out of the MeasuredTemplate subclass and into the TokenPlacement class.

The TokenPlacementConfiguration data structure currently just includes the prototype token information, but will eventually have quantity, origin, and range values to handle multiple summons and restricting range from the summoner.

Works with tokens of any size or scale on the square grid. On hex grids, it handles 1x1 tokens pretty good with only a bit of offset on the final token placement. Stranger token sizes lead to some issues with positioning, but I'm not sure how important those are to fix before the other grid improvements in V12.

Closes #893

@arbron arbron added epic feature request in progress ux User experience related features or bugs labels Mar 11, 2024
@arbron arbron added this to the D&D5E 3.1.0 milestone Mar 11, 2024
@arbron arbron requested a review from Fyorl March 11, 2024 17:24
@arbron arbron self-assigned this Mar 11, 2024
@arbron arbron removed the request for review from Fyorl March 11, 2024 17:24
@arbron arbron force-pushed the summoning-placement branch from fe71c7f to 7242a0a Compare March 13, 2024 17:06
@arbron arbron requested a review from trioderegion March 13, 2024 17:20
@arbron arbron force-pushed the summoning-placement branch from 7242a0a to 89a54f2 Compare March 13, 2024 20:56
Base automatically changed from summoning-workflow to 3.1.x March 15, 2024 22:06
@arbron arbron requested a review from Fyorl March 15, 2024 22:07
The `TokenPlacement` class right now is small and only delegates
to the `TokenPlacementTemplate` which is responsible for rendering
and returning placement information. With the V12 improvements to
placeables hopefully we'll be able to shift more logic out of the
`MeasuredTemplate` subclass and into the `TokenPlacement` class.

The `TokenPlacementConfiguration` data structure currently just
includes the prototype token information, but will eventually have
quantity, origin, and range values to handle multiple summons and
restricting range from the summoner.

Works with tokens of any size or scale on the square grid. On hex
grids, it handles 1x1 tokens pretty good with only a bit of offset
on the final token placement. Stranger token sizes lead to some
issues with positioning, but I'm not sure how important those are
to fix before the other grid improvements in V12.
@arbron arbron force-pushed the summoning-placement branch 2 times, most recently from 0e1fa23 to 5b0c07f Compare March 20, 2024 22:44
Copy link
Contributor

@Fyorl Fyorl left a comment

Choose a reason for hiding this comment

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

Please see my comments, but this looks great.

module/canvas/token-placement.mjs Show resolved Hide resolved
module/canvas/token-placement.mjs Outdated Show resolved Hide resolved
module/canvas/token-placement.mjs Outdated Show resolved Hide resolved
Comment on lines +144 to +149
#getSnapAdjustment(token) {
const size = canvas.dimensions.size;
switch ( canvas.grid.type ) {
case CONST.GRID_TYPES.SQUARE:
return {
x: token.width % 2 === 0 ? Math.round(size * 0.5) : 0,
y: token.height % 2 === 0 ? Math.round(size * 0.5) : 0
};
default:
return { x: 0, y: 0 };
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be able to use canvas.grid.getSnappedPosition (getSnappedPoint in v12) for this. I don't think we need to handle this ourselves.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I find that using getSnappedPosition works fine for 1x1 tokens, but results in larger or stranger shaped tokens being offset in rather than centered under the token. I tried this:

const center = canvas.grid.getSnappedPosition(
  point.x - Math.round(canvas.dimensions.size * 0.5),
  point.y - Math.round(canvas.dimensions.size * 0.5),
  1,
  { token: this.config.tokens[0].object }
);

For 2x2 tokens for example, this would result in the token being places with the top-left quarter centered over the square that I am hovering which makes it non-intuitive where to move the mouse to place the token.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, that's fair enough. I think this has all been improved in v12 so I will experiment with checking game.release here and calling getSnappedPoint instead, but this seems good enough for now.

module/canvas/token-placement.mjs Outdated Show resolved Hide resolved
@arbron arbron force-pushed the summoning-placement branch from 5b0c07f to ad057a7 Compare March 21, 2024 14:42
@arbron arbron requested a review from Fyorl March 21, 2024 14:42
@arbron arbron merged commit 438a986 into 3.1.x Mar 21, 2024
@arbron arbron deleted the summoning-placement branch March 21, 2024 17:09
@trioderegion trioderegion removed their request for review June 5, 2024 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants