eliemichel / bmeshunity Goto Github PK

I'm rather hoping your intent is for this to be under a permissive licence but I can't see any mention of licencing at all - which I believe means this is currently all rights reserved!

Summary

Change the return type of methods that return List<> to IEnumerable<>.

Motivation

Manipulating the lists that returns from the methods have no effect on the actual BMesh, as such there is really no reason you ever should or would need to add/remove items to the List<>s. It also does not clearly communicate the purpose and use of the returned collection when it is a List<>.
Where as IEnumerable<> more clearly communicates the intent behind the method.
Returning a List<> does not follow the C# design guidelines. An added benefit of returning IEnumerable<> is that the methods can make uses of yield return for potentially slightly better performance.

Drawbacks

This is a breaking changing if people use any of the List<> methods such as Add() Remove() or Contains(), or for people that declare the variable type when assigning the returned value List<Edge> edges = vertex.NeighborEdges().

Alternatives

One option is to return IReadOnlyList<> instead so people still have access to the indexer and Count property. However this makes it less generic and can no longer use yield return.

I would be happy to make a PR for this change if desired after any further discussion.

Summary

Rename the project and any references of BMesh to UMesh instead.

Motivation

The "B" in "BMesh" stands for Blender (At least I assume as much), so it makes sense to use "U" for a Unity implementation. I believe that renaming it makes it cleaning (no longer needs to be "BMesh for Unity"), and more distinct, as the project grows and becomes less of a one to one with the Blender BMesh I think it would be good for it to have it's own name.
There are also a number of proposals open that, if accepted would already introduce breaking changes, so if it is to ever be renamed, now would be a good time to do so.

I do understand that this is rather unimportant overall, but I think that it would make it look nicer and more distinct.

Drawbacks

This is a breaking change that would require changing any references to the BMesh class to UMesh.

I would be happy to make a PR with this change if desired after any further discusion.

Is it possible to continue to add to bmesh after BMeshUnity.SetInMeshFilter(mesh, MakeMeshIndependent());

If not - is it possible to re-initialize it, so that it can generate a continuous result for the meshfilter?

apply a material with texture , it didn't show correctly

We should profile the amount of new memory allocation when manipulating arbitrary attributes, and organize the API in a way that hints the user into using the most efficient approach. For instance:

v1.attributes["uv"] = new FloatAttributeValue(0.0f, 0.0f);

is less efficient than

var uv = v1.attributes["uv"].asFloat();
uv.data[0] = 0.0f;
uv.data[1] = 0.0f;

but the latter is a bit annoying to write.

Summary

Change all public fields to either be properties or to private fields if applicable.

Motivation

There are numerus advantages to properties over fields, which include being able restrict setting the value of properties, more clearly communicate the intended usage of it, and being able to change how they function without introducing breaking changes. For BMesh specifically this would be especially useful to prevent manually setting the values of properties like Edge.loop.
Public fields also do not follow the C# design guidelines

Drawbacks

This is a breaking change as it would also involve renaming all fields from using camelCasing to PascalCasing to follow C# design guidelines for property naming.
It would be a breaking change for anyone that is setting a value of a field that is changed to a get only property. However they should not be doing that anyways so that would be desired I believe.

I would be happy to create a PR with required changes if it is desired after any further discussion.

Hi Elié,

First thanks a lot for BMeshUnity!!
I'm trying the samples and they all work at runtime but not in the Editor. I'm not sure how mesh can be not null in the samples as it's created in the Start function? I might have missed something.

Also, can BMesh be used for runtime procedural mesh creation? If so should I set the InMesh filter each time I update the mesh components?

Thanks

Hi @eliemichel, thanks for you excellent work! May I ask does BMesh support loading a mesh file(like .obj) in runtime? Or how can I achieve this with BMesh?

Summary

Move the Vertex, Edge, Loop, and Face classes that are nested in the BMesh class in to their own classes and files.

Motivation

It would make the BMesh class smaller and easier to read and edit, and allows for easier editing and reading of multiple classes at once. Using the BMesh API currently requires additional boilerplate because it requires you to either declare a static using of the BMesh class, or preface every reference with BMesh. (eg BMesh.Vertex).
The current implementation also does not follow the C# design guidelines making it looks less consistent, clean, and intuitive.

Drawbacks

This is a breaking change, if anyone were referencing the nested types directly (eg BMesh.Vertex) they would need to change the usage from BMesh.Vertex to just Vertex.

I would be more than willing to make a PR for this, but thought it better to make an issue for it for easier discussion first.