Allow declaring variables of type dtMeshTile

[elsid]

In general this change allows to write code like:

dtMeshTile tile;

With the only private copy constructor it's not possible.

Also it's completely ok to copy and copy-assign object of this type because there is no custom destructor. For example if I need to write custom function to map unsigned char* into dtMeshTile like it's done here I want to write a function:

dtMeshTile asMeshTile(unsigned char* data);

And this is fine because dtMeshTile is only a view to data.

All this is required to check equivalence of two navmesh tiles when one tile is dtNavMesh::getTileAt result and the other is dtCreateNavMeshData output.

[Bromeon]

And this is fine because dtMeshTile is only a view to data.

This is not true, dtMeshTile owns the data, when you create it with dtTileFlags::DT_TILE_FREE_DATA. Your change would cause UB.

This is managed externally, see here for example. It's a weird design choice that there is no destructor in dtMeshTile itself, but it's probably due to the lack of move semantics.

[elsid]

dtMeshTile owns the data, when you create it with dtTileFlags::DT_TILE_FREE_DATA.

I can't agree with this statement.

dtCreateNavMeshData doesn't not have a flag to say who owns the data. It just provides unsigned char* to somehow allocated and initialized memory. After dtCreateNavMeshData is called, a caller owns the data. And that's where I need to view this data as a structured type. My alternative here is to create a new type that will be mostly a copy of dtMeshTile definition.

Then this data may or may not be passed to dtNavMesh::addTile. There it's possible to say whether the data ownership will be transferred to dtNavMesh or no via flags argument. After that the owner of the data is either the caller or dtNavMesh object. If it's dtNavMesh object it will be deallocated later by one of the following functions:

1. dtNavMesh::~dtNavMesh
2. dtNavMesh::removeTile

Ownership means to have some logic to release owned resource. So I don't agree that dtMeshTile is the owner at any point of this chain.

Your change would cause UB.

Could you provide some reference how an absence of a private copy constructor declaration and a copy-assignment operator is UB? Or explain what other code constructions may cause it.

This is managed externally, see here for example.

This contradicts with your first statement.

I assume your arguments are about copying dtMeshTile objects. Note that I never mentioned that I actually want to copy it. But let's say users start to do this after it's allowed. What will happen? Need to take a look at dtMeshTile members. All of them either integral types or pointers. Is it ok to copy integral type or a pointer? Yes, it is. The next question. What if the underlying value of dtNavMesh::getTile result pointer is copied? The copy is valid until dtNavMesh destructor or removeTile function is called for this tile if dtNavMesh owns the data. It will be UB to dereference pointer members of a copy after the data is deallocated. But it's a common practice in C++ to have such situation as a result of a trade-off between safety and performance. Iterators to STL containers is one example. And dtNavMesh::getTile is also an example because it's result may become a dangling pointer after tile data is deallocated. Similar logic applies to the copy-assignment.

[Bromeon]

Ownership means to have some logic to release owned resource. So I don't agree that dtMeshTile is the owner at any point of this chain.

Thanks for the detailed elaboration! You're right, the dtMeshTile stores the pointer, but the logic (memory management) is external.

So if I understand you correctly, you consider dtMeshTile simply as a POD struct (which happens to be used as the internal storage of nav meshes), and following that argument, it should have value semantics, including copy/assign.

I can only speculate, but I would guess dtMeshTile was never intended to be managed by the user. It's an type used internally for tile storage, however the dtNavMesh API hands out pointers to it (but not values). This abstraction is leaky in many ways and I would argue that this type should have been private in the first place, as many of its fields are implementation details.

You mention you need this to check for equivalence between mesh tiles; what does this involve? You need actual inspection beyond just checking for tile identity (via index/ID)?

[elsid]

You mention you need this to check for equivalence between mesh tiles; what does this involve? You need actual inspection beyond just checking for tile identity (via index/ID)?

I need a partial deep equality. Something similar to:

bool operator==(const dtMeshHeader& a, const dtMeshHeader& b) {
    return /*...*/ && a.polyCount == b.polyCount && /*...*/
}
bool operator==(const dtMeshTile& a, const dtMeshTile& b) {
    return *a.header == *b.header
         && std::equal(a.polys, a.polys + a.header->polyCount, b.polys)
         && /*...*/;
}
bool isEquivalent(const dtMeshTile& a, unsigned char* b) {
    return a == asMeshTile(b);
}

Without this pr I will need to create another type instead of dtMeshTile and convertors from dtMeshTile and unsigned char*:

struct MeshTileView { /* same members as in dtMeshTile */ };
bool isEquivalent(dtMeshTile& a, unsigned char* b) {
    return MeshTileView(a) == MeshTileView(b);
}

And it's not a problem. But it would be nice to reuse dtMeshTile.