ArnoldSmith86 / virtualtabletop

a virtual surface in the browser on which you can play board, dice and card games
https://virtualtabletop.io
GNU General Public License v3.0
164 stars 29 forks source link

Grid should be able to be limited by simple mathematical functions #133

Open RaphaelAlvez opened 3 years ago

RaphaelAlvez commented 3 years ago

As a security feature we could only accept numbers, point (.), comparators, and basic mathematical functions (+, -, *, /, ^, PI, sin, cos, tan,...). This would allow making inclined limits or even circles using basic parametric functions.

robartsd commented 3 years ago

So you want to evaluate a simple math expression to determine if the x or y coordinates fall within the grid boundaries?

RaphaelAlvez commented 3 years ago

Yes.

I'm having to deal with this kind of grid:

image

comparing it against 4 simple line equations would solve this problem but I think the solution can be more flexible allowing any simple math equation

I can also see this being a problem to other hexagonal games as catan

robartsd commented 3 years ago

I'd propose a new grid property boundary that takes a string to evaluate. If the string evaluates to true, the grid is valid for the coordinates otherwise the grid is not valid for the coordinates. I don't think we need PI or trig functions (a circle can be tested without them as "(x - center x)^2 + (y - center y)^2 < radius^2"). String would need to be checked every time before it is evaluated as a button could change the grid property of a widget including the boundary string.

larkob commented 3 years ago

@RaphaelAlvez Can you please provide some more detail about why this enhancement is needed or desirable? The grid is basically just a list of rectangles with individual coordinates, so in the case of this game, you would just add 47 individual rectangles to the list. What kind of limit do you have in mind?

robartsd commented 3 years ago

This particular setup can accomplished with 6 overlapping grids with x and y min and max set on each. The 7 positions in the center would each be a part of 3 of those grids, 16 additional positions would each be a part of 2 grids. This layout could also be accomplished with 10 grids where each row or column only belonged to one grid definition, but I don't see a clear reason to avoid defining overlapping grids that define the same positions.

With Raphael's proposal, the setup would be 2 overlapping grids with each position defined in only one grid. I'm not sure about performance implications of checking 6 grids using existing code vs. 2 grids where each grid has a string to check against a regular expression and evaluate in addition to current code; however, it is clear to me that Raphael's proposal would scale better for similar layouts with more positions. It would also be easier for game design and maintenance.

RaphaelAlvez commented 3 years ago

Well @larkob as @robartsd pointed out, even if any grid is possible I'm the current system, adding parametric functions as limits would make "complex" grids like this way more simple to make. That's the main reason for me

RaphaelAlvez commented 3 years ago

Reading the comments again I am going to agree with @robartsd and we can simplify it and exclude trigonometric functions. So we can build this only whitelisting x, y, numbers, comparators and math symbols (+,-,*,/,(,),.,^)

larkob commented 3 years ago

Instead of working with mathematical functions, we could improve the grid feature by allowing to specify a polygon for the grid boundary instead of a rectangle. For games like the above, we could "draw imaginary lines" around the desired grid area, which would result in an area defined by 6 connected points, which we'd need to specify as coordinates for the grid boundary. Then the grid check function would determine whether the current position is within that area or not, using a simple or complex algorithm.

RaphaelAlvez commented 3 years ago

Well even though I would still prefer parametric functions, I agree that this might be the best option as it might look less scary to some people. I know there is going to be someone out there not comfortable to make a y<=67-36*x.

I spent about 10 minutes trying to find a situation where a polygon wouldn't work and I couldn't find one. Circles are the only ones that could be a problem but we can create a macro on ghetto to create a polígono with 12 sides and should be enough

robartsd commented 3 years ago

I don't think making the boundary an array of (x, y) pairs would be easier to implement or less computationally expensive to code, but it would be easier on most JSON authors and easier to create a UI for (if someone decides creating a UI for this feature is important enough). It also would mean much less risk of a bug being a security issue (I am confident that we could adequately check the user input for the original idea here before calling eval() on it if we kept the allowed expressions simple enough).

What limitations do we place on the boundary shape set of points? Can it be concave? Can it cross itself? Does it matter if the points are listed clockwise or anti-clockwise?

Easiest to implement would be a convex polygon with the points listed in a defined direction (calculate if the point is on the correct side of each of the boundary lines). We could also accept a list of similarly defined shapes that should be excluded from the grid so that a grid with holes in it could be created (a hole on the edge would form a concave shape).

"boundary": [ {"x":x0,"y":y0}, {"x":x1,"y":y1}, ... {"x":xn,"y":yn} ], "exclusions": [ [ {"x":x0,"y":y0}, {"x":x1,"y":y1}, ... {"x":xn,"y":yn} ], [ {"x":x0,"y":y0}, {"x":x1,"y":y1}, ... {"x":xn,"y":yn} ] ].

We'd write a subroutine that takes the shape array and x, y coordinate and returns true if the point is within the shape (on the correct side of each line assuming that the shape is a convex polygon with vertices listed in clockwise order) or false if the point is outside the region defined by the lines. The grid would be skipped if the boundary shape returns false or any exclusion shape returns true. Authors would be responsible for ensuring that the shape is convex and defined in the correct order.

robartsd commented 3 years ago

One of the tricky things about grids is that the x, y pair being checked is the top, left corner of the object. Even more surprising is that the origin of a grid without offsets positions the object so that its center is at 0,0.

I guess it makes sense to use centers of objects for grid locations (and other drop targets), but if grids are based on object centers, it would be nice if their boundaries are based on object centers as well. Can't think of how we could implement that for minX, maxX, minY, maxY without breaking existing grids, but we could use center based coordinates for this feature.

RaphaelAlvez commented 3 years ago

One of the tricky things about grids is that the x, y pair being checked is the top, left corner of the object. Even more surprising is that the origin of a grid without offsets positions the object so that its center is at 0,0.

I guess it makes sense to use centers of objects for grid locations (and other drop targets), but if grids are based on object centers, it would be nice if their boundaries are based on object centers as well. Can't think of how we could implement that for minX, maxX, minY, maxY without breaking existing grids, but we could use center based coordinates for this feature.

Is this the current situation? I had the impression it was using the top left corner as a reference

robartsd commented 3 years ago

Sorry, you're right it's not the center of the object, It's just offset by half the grid spacing for seemingly arbitrary reasons (I've just mostly used grids that place the objects right next to each other so half the grid spacing is the same as half the height and width).

bjalder26 commented 3 years ago

I'd propose a new grid property boundary that takes a string to evaluate. If the string evaluates to true, the grid is valid for the coordinates otherwise the grid is not valid for the coordinates. I don't think we need PI or trig functions (a circle can be tested without them as "(x - center x)^2 + (y - center y)^2 < radius^2"). String would need to be checked every time before it is evaluated as a button could change the grid property of a widget including the boundary string.

I've been thinking about this and had a similar solution. I was going to implement it as a changeRoutine, but was waiting for atan. I'm not sure how the engine would work exactly, but if it would need to operate by immediately 'correcting' the x and y values (which is what I would do in a changeRoutine), then I think you would need to use atan to get the angle (from the center of the circle to the center of the widget) then using that angle, calculate the point on the circle at that same angle, and move the widget to that point. Like the sliders we made, it would happen so quickly that it would never be seen outside of the circle.