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

[BUG] MeasuredTemplatePF2e get snappingMode returns incorrect snapmodes on Hex Grid #15615

Closed
FolkvangrForgent opened this issue Jul 28, 2024 · 1 comment · Fixed by #15616
Closed
Labels
bug Functionality that is not working as intended

Comments

@FolkvangrForgent
Copy link
Contributor

FolkvangrForgent commented Jul 28, 2024

As a residence hex grid user I was tracking down odd snapping behavior of template on hex grids. It looks like the get snappingMode function in MeasuredTemplatePF2e is written with the expectation that it is called from a square grid but it can also be called from a hex grid via the chat message button. As far as I can tell it still can't be called from gridless.

I would suggest replacing the current code:

switch (this.areaShape) {
case "burst":
return M.CORNER;
case "emanation":
return M.CENTER;
case "cone":
return M.CENTER | M.CORNER | M.SIDE_MIDPOINT;
case "line":
return M.SIDE_MIDPOINT | M.CORNER;
default:
return M.CENTER | M.CORNER;
}

with something like the following code:

        if (canvas.grid.isSquare) {
            switch (this.areaShape) {
                case "burst":
                    return M.CORNER;
                case "emanation":
                    return M.CENTER;
                case "cone":
                    return M.CENTER | M.CORNER | M.SIDE_MIDPOINT;
                case "line":
                    return M.SIDE_MIDPOINT | M.CORNER;
                default:
                    return M.CENTER | M.CORNER;
            }
        }
        if (canvas.grid.isHexagonal) {
            switch (this.areaShape) {
                case "burst":
                    return M.VERTEX;
                case "emanation":
                    return M.CENTER;
                case "cone":
                    return M.CENTER | M.VERTEX;
                case "line":
                    return M.CENTER;
                default:
                    return M.CENTER | M.VERTEX;
            }
        }

(I'll admit that I have left the "cone" to the default of M.CENTER | M.VERTEX as I do not know if a more restrictive set of modes would be cleaner for hex grids)

Additionally with the snappingMode working on hex grids the following code:

if (!template || !canvas.grid.isSquare) {

can be changed to:

        if (!template || canvas.grid.isGridless) { 

I will lastly note that there is a current MR #15552 that add more square grid dependent logic.

@ammalagonc ammalagonc added the bug Functionality that is not working as intended label Jul 28, 2024
@FolkvangrForgent FolkvangrForgent changed the title [BUG] MeasuredTemplatePF2e get snappingMode return incorrect snapmodes on Hex Grid [BUG] MeasuredTemplatePF2e get snappingMode returns incorrect snapmodes on Hex Grid Jul 28, 2024
@FolkvangrForgent
Copy link
Contributor Author

FolkvangrForgent commented Jul 28, 2024

As another suggestion of code cleanup the function hasHexGrid that is part of ScenePF2e is redundant with the new isHexagonal grid flag. The following code can be removed:

get hasHexGrid(): boolean {
const squareOrGridless: number[] = [CONST.GRID_TYPES.GRIDLESS, CONST.GRID_TYPES.SQUARE];
return !squareOrGridless.includes(this.grid.type);
}

if the three references to canvas.scene.hasHexGrid are changed to canvas.scene.grid.isHexagonal. I have linked the references below for quick access:
CONFIG.MeasuredTemplate.defaults.angle = canvas.scene.hasHexGrid ? 60 : 90;

const snapAngle = Math.PI / (canvas.scene.hasHexGrid ? 6 : 4);

const coneMultiplier = shapeType === "cone" ? (canvas.scene.hasHexGrid ? 2 : 3) : 1;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Functionality that is not working as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants