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

GridCoords and FieldValue points using different origins #124

Open
vivax3794 opened this issue Oct 13, 2022 · 1 comment
Open

GridCoords and FieldValue points using different origins #124

vivax3794 opened this issue Oct 13, 2022 · 1 comment

Comments

@vivax3794
Copy link

Problem

I have a entity that needs it grid cords and also has a component (ldtk attribute) that contains a vector of points.

image

now looking at that I can see that LDtk uses the upper left of the grid as 0, 0 (and +y going down), now if we just use the value of a FieldValue::Points directly that is what we get.

but if we use #[grid_coords], it uses the bottom left (with y+ going up), which is actually nice as it matches bevy, BUT the fact that these two are different is REALLY annoying to work around.

because if I want them to be the same I need to modify the value from the field based on the grid height, which I don't have access to in a From<EntityInstace> implementation, and I need it as my levels are of different heights.

I looked at this plugins source code, and I can see that it does a conversion for GridCoords:

bevy_ecs_ldtk/src/utils.rs

Lines 102 to 104 in 66df968

fn ldtk_coord_conversion_origin_adjusted(coords: IVec2, height: i32) -> IVec2 {
IVec2::new(coords.x, height - coords.y - 1)
}

bevy_ecs_ldtk/src/utils.rs

Lines 116 to 134 in 66df968

/// Performs LDtk grid coordinate to [GridCoords] conversion.
///
/// This conversion is performed so that both the LDtk grid coords and the resulting [GridCoords]
/// refer to the same tile.
/// This is different from them referring to the same position in space, because the tile is
/// referenced by its top-left corner in LDtk, and by its bottom-left corner with [GridCoords].
pub fn ldtk_grid_coords_to_grid_coords(ldtk_coords: IVec2, ldtk_grid_height: i32) -> GridCoords {
ldtk_coord_conversion_origin_adjusted(ldtk_coords, ldtk_grid_height).into()
}
/// Performs [GridCoords] to LDtk grid coordinate conversion.
///
/// This conversion is performed so that both the [GridCoords] and the resulting LDtk grid coords
/// refer to the same tile.
/// This is different from them referring to the same position in space, because the tile is
/// referenced by its top-left corner in LDtk, and by its bottom-left corner with [GridCoords].
pub fn grid_coords_to_ldtk_grid_coords(grid_coords: GridCoords, ldtk_grid_height: i32) -> IVec2 {
ldtk_coord_conversion_origin_adjusted(grid_coords.into(), ldtk_grid_height)
}

Possible solutions

Now I could implement the LdtkEntity trait manually, which would have access to all then needed information, but that seems like it is a bit overkill for something this simple that I feel like should be possible with the From<...> Implementation, there is #47 which might make this a bit simpler, but honestly I feel like the FieldValue::Points/FieldValue::Point should already use the same coordinate system as GridCoords

I understand this makes the implementation harder, as from a glance at the code the parsing of those seems to be deeply nested in Serde de-serialization, which means we cant access the world height at that point, meaning we would in theory have to iterate over the field values and "fix" this before constructing the entity

another solution is to allow components to have more access to the world, which is of course hard to do in a From implementation, but you could use a custom trait, or maybe even extend #47 to components .... somehow?

@Trouv
Copy link
Owner

Trouv commented Oct 14, 2022

Yeah, I think manually implementing LdtkEntity in general is a bit of a pain, and it's the only solution here. However, I feel like changing the way these values are serialized isn't good. I have some interest in honestly representing the LDtk data in those serde types, and swapping the coordinate direction in those types would not only be difficult (as you mentioned), but it would also "misrepresent" the LDtk data.

#47 would probably help a lot but honestly I'm no closer to figuring out how to implement it than the day I filed it lol.

Since the introduction of "entity references" in LDtk 1.0, I've wanted to have some field instance support in the LdtkEntity derive macro. Something like #[field_instance("my_field_identifier")]. We could probably cover some common use cases, including logic for swapping the coordinate direction, in a macro like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants