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

GridMap only supports 16bit IDs while MeshLibrary supports 32bit IDs #43836

Open
wd40bomber7 opened this issue Nov 24, 2020 · 1 comment
Open

Comments

@wd40bomber7
Copy link

wd40bomber7 commented Nov 24, 2020

Godot version: v3.2.3.stable.mono.official

OS/device including version: Microsoft Windows 19041.630

Issue description: I am working on a very simple game and decided I wanted to use gridmap to paint the terrain. I wrote a tiny bit of C# script to take a list of textures and build simple 3d mesh tiles using them to create my mesh library. For the MeshLibrary IDs I decided to just hash the texture names so the IDs would stay consistent every time I regenerated the MeshLibrary. This is the important bit because I generated those ids like this:
Math.Abs(itemName.GetHashCode())

This generated entirely valid MeshLibrary entries:
image

However when I went to actually draw my grid tiles nothing happened (looking like this):
GridMapBroken

The problem is this code from gridmap.cpp:

void GridMap::set_cell_item(const Vector3i &p_position, int p_item, int p_rot) {
   // ... Lots of unrelated stuff ...
	Cell c;
	c.item = p_item;
	c.rot = p_rot;

	cell_map[key] = c;
}

Cell is defined like this:

	union Cell {
		struct {
			unsigned int item : 16;
			unsigned int rot : 5;
			unsigned int layer : 8;
		};
		uint32_t cell;
                // Constructor omitted...
	};

And there's the problem. Item is bitmapped to only the first 16 bits. This means that the latter 16 bits are just chopped right off without any error or warning! The shorter and completely wrong ID is then stored in the gridmap.

In summary:

  • MeshLibrary supports 32 bit positive IDs
  • GridMap only supports 16 bit IDs
  • GridMap takes 32 bit IDs and then cuts off the top 16 bits without any warning or error

This issue was very frustrating since GridMap doesn't even bother to put out a warning on invalid IDs. I suggest adding a warning (at least to the editor). As for actually fixing the issue I'm not sure what would be best, maybe switch MeshLibrary to use a short so this specific mistake becomes impossible? Also set_cell_item should absolutely take a short.

Minimal reproduction project: GridMapKeyLimitation.zip

@ATHellboy
Copy link

Any plans on supporting 32bit IDs for GridMap ?

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

No branches or pull requests

3 participants