feat(ghx_grid): Use Grid trait
- Replace GridDefinition with Grid Trait, adding generic params where needed
- Use DirectionTrait
Will resolve #1 together with Henauxg/ghx_grid#1
I didn't touched the bevy facing parts yet. I'm creating this pull request to show an approach. I have introduced a Grid Trait and used that Grid as a generic parameter throughout the library. Which, in my opinion, is the shortest path to generalize it without touching the internals.
Cheers
Thank you, this seems great. I will have a look in the coming days
I saw you issue #1 a few days ago and tried a few changes myself, which looked a lot similar. Although I tried to go a bit deeper in order to try and fight against the current Direction concept which supposes an opposite direction.
As you mentionned it in the original issue, for an irregular grid, the opposite concept is not ideal. I see that you now modified your issue. Did you manage to solve your issue with just the 2 existing PRs ?
During my rework, I found that the generator could use a trait even lighter than the new Grid trait with no concept of Position (just indexes), and a simple get_neighbors_in_all_directions method returning neighbors indexes alongside their respective direction indexes.
I did not finish this work yet though, and some initializations methods in the Rules and the Generator still needed the opposite concept. I think an irregular grid is more akin to a graph than a grid, and as such, some more work needs to be put onto the Grid/Topology definition to allow for a graph back-end to implement the trait.
Edit: I think this PR is a good first step towards a more generic topology definition, it's fine as is.
I think I also stumbled upon the fact that an irregular, non-oriented grid may not need a lot of the features designed for a regular 'oriented" grid, such as rotations and model variations (the current ModelRotation is also currently very tied to the cartesian coordinate system)
You are right, Grid may be lighter. I've first made the Grid with just the indexes as you, then realized its debug use in InternalGenerator, and readded them. I didn't want to make too much modification. Though they would better be removed.
Yes an irregular grid just requires get_neighbors_in_all_directions and indices. But rotations still have a place. Check out [1] "intro to marching squares" part. Tiles rotated and "trilinear interpolated" [2]. As long as the grid formed by same-sided polygons, tiles can be and need to be rotated. Though in case of Tris, or Hexes, there should be Rot120, Rot240 etc. on ModelRotation. It should also be tied to the coordinate system. There may be systems where rotation does not make sense, they can use a default empty rotation implementation.
As for opposite. I couldn't comprehend their use completely. I may be wrong, please correct me. But as far as I understand, they are just used to attach sockets on opposite directions. Generator does not expect them to be absolute directions, they are relative. Which is why I've scratched them on my issue. I would define my triangles with a, b, c and return same for opposite directions, and z, -z of course. They would not be a hassle then. Though I've yet to complete my work, so I'm not exactly sure on this. However, then it means it is better to remove them altogether, and iterate directions instead of opposites.
I'm going to test these on a simple 2d irregular triangle mesh.
[1] https://john-wigg.dev/SphereScaper/ [2] https://sketchpunklabs.github.io/irregular_grid/
Took a quick look, I'll have a more precise look and response tomorrow
I simplified the added Grid trait even more and its usage in the InternalGenerator.
I kept the Position type, but it really is facultative now for a custom Grid implementation if you set it to be GridIndex, while still allowing for debug use cases and NodeRef implementations.
I’ll update the bevy plugins to the new API and to use CartesianGrid instead of the Grid trait when I get back middle of next week.
There are still some Todos for later, such as:
- Only 2 call sites left needing
directions()(DirectionTrait::opposite) access:Rules::newandInternalGenerator::initialize_supports_count. I am thinking that those should ideally go away in favor of a more generic DirectionTrait. ModelRotationneeds to be more generic
Concerning directions and opposite. I believe what you are doing "should" work. I think it's my turn to scratch my head and ponder on why I designed it this way. I will come back to it.
Looks great.
While re-implementing ModelRotation, BorisTheBrave's Tileset Creator may give an idea. Instead of an enum, rotation may just be usize. Also a reflection field reduces the tile count even further.
Updated the branch to integrate ghx_grid version 0.3.2 as well as to fix all examples & bevy plugins
As said previously, there is still more that could be worked-on:
There are still some Todos for later, such as:
* Only 2 call sites left needing `directions()` (`DirectionTrait::opposite`) access: `Rules::new` and `InternalGenerator::initialize_supports_count`. I am thinking that those should ideally go away in favor of a more generic DirectionTrait. * `ModelRotation` needs to be more generic
These points will not be treated in this PR.