fix descriptions of map_to_world/world_to_map

[pseidemann]

The description of map_to_world was explaining the method world_to_map and vice versa. Also I think these descriptions were pretty confusing.

I swapped the descriptions to match the actual methods and tried to make their functionality more clear.

[kleonc]

Current descriptions are correct besides incorrectly referring to the argument with [code]pos[/code] instead of [code]world_position[/code] in:

godot/modules/gridmap/doc_classes/GridMap.xml

Lines 136 to 143 in 7819378

	<method name="world_to_map" qualifiers="const">
	<return type="Vector3i" />
	<argument index="0" name="world_position" type="Vector3" />
	<description>
	Returns the coordinates of the grid cell containing the given point.
	[code]pos[/code] should be in the GridMap's local coordinate space.
	</description>
	</method>

Also I think your new descriptions are vague as it's not clear what "world position" is. And "world" in these method names actually does refer to the local space of the given GridMap so current descriptions are correct in that aspect.

However, wording might indeed be improved. Probably it should be made analogous to the same method descriptions in the TileMap:

godot/doc/classes/TileMap.xml

Lines 209 to 216 in 7819378

	<method name="map_to_world" qualifiers="const">
	<return type="Vector2" />
	<argument index="0" name="map_position" type="Vector2i" />
	<description>
	Returns a local position of the center of the cell at the given tilemap (grid-based) coordinates.
	[b]Note:[/b] This doesn't correspond to the visual position of the tile, i.e. it ignores the [member TileData.texture_offset] property of individual tiles.
	</description>
	</method>

godot/doc/classes/TileMap.xml

Lines 316 to 322 in 7819378

	<method name="world_to_map" qualifiers="const">
	<return type="Vector2i" />
	<argument index="0" name="world_position" type="Vector2" />
	<description>
	Returns the tilemap (grid-based) coordinates corresponding to the given local position.
	</description>
	</method>

[pseidemann]

@kleonc

And "world" in these method names actually does refer to the local space of the given GridMap so current descriptions are correct in that aspect.

do you mean it's kind of like transform.origin as opposed to global_transform.origin?
I thought the world position here is global but I admit I always had my GridMap at 0,0,0 so there is no difference.

how would you call the local (integer) coordinates of cells? I would call it local meaning local to the grid map. so when the world position is also local (local space), it would be really confusing

[kleonc]

do you mean it's kind of like transform.origin as opposed to global_transform.origin? I thought the world position here is global but I admit I always had my GridMap at 0,0,0 so there is no difference.

No, not really. Both global_transform.origin and transform.origin are the same position, just represented in different coordinate spaces: global_transform.origin in the global coordinate space, transform.origin in the parent's coordinate space (it's relative to the parent). Local coordinate space is relative to the node itself (here GridMap) and global_transform.origin/transform.origin locally is simply Vector3(0, 0, 0).

transform transforms from local to parent's space
global_transform transforms from local to global space (it's just combined (multiplied) transforms of the parent nodes)

how would you call the local (integer) coordinates of cells? I would call it local meaning local to the grid map. so when the world position is also local (local space), it would be really confusing

It's not local space (the one I've mentioned above), according to these method names (map_to_world/world_to_map) it's map space. But for me calling it grid space makes more sense, map is not clear for me. I'd guess TileMap node was created earlier and map_to_world/world_to_map were named so they would match TileMap's methods.

In fact I never liked map_to_world/world_to_map names, I find them confusing. Also I saw these names (the ones in the TileMap) confuse many users. I think if these would be called grid_to_local/local_to_grid there would be no confusion (or at least way less confusion). In 3.x of course it can't be renamed as it would break people's code. But for 4.0 it should be possible. Additionaly grid_to_global/global_to_grid could be added which would be just shorthands for to_global(grid_to_local(grid_position))/local_to_grid(to_local(global_position)). These are simple one liners but this is such a common use case that I think they're worth adding.

[mhilbrunner]

Closed as per the above discussion. Personally, I think we may indeed want to consider better names for these - discuss in godotengine/godot-proposals#4535