Fix #2355 - VertexBuffer doesn't serialize the 'name' field

[capdevon]

Fix for #2355

The changes include:

• Adding serialization and deserialization support for the name field in the VertexBuffer class by writing and reading the name property in the appropriate methods.

These updates improve code consistency and ensure the name field is correctly handled during object serialization and deserialization.

[yaRnMcDonuts]

I wonder if the name field may have been intentionally left un serialized, since it is near useless in this context, and vertex buffer is a highly used object that was probably intended to be as small and optimized as possible. Plus the associated mesh would already have a unique name, and I'm not aware of any cases where someone would need to save a uniquely named vertex buffer on its own without being part of a mesh (and even in that rare case, the asset path could be used to identify it just as effectively as its name, if not more effectively).

And since a large project may have 1000s or more of unique meshes saved throughout their models and scenes (each with multiple vertex buffers) this could amount to a small but non negligible increase in size for some existing projects' asset directories.

I'd have to see a good case showing where querying a unique name of a freshly loaded VertexBuffer is absolutely necessary, or a place in the jme code where there is a bug as a result of not saving/loading this. Otherwise, I'm unsure if the cost/benefit is worth it.

[yaRnMcDonuts]

Oops, I started typing my review after looking at the code and replied before seeing where you linked the existing issue.

So that does address my concern of whether this is necessary to fix a bug or an existing issue. (not necessarily a bug though, but still a case where someone has wanted this info for debugging)

I do think there still would be a way to write it so that it doesn't serialize the name if the name is equal to its default non-unique value (the value that is returned by the getName() method when name is null). That way it only serializes uniquely named VertexBuffers and won't waste memory serializing redundant default names.

[capdevon]

By default, the name field is null and therefore not serialized. This behavior aligns with expectations. If a user explicitly assigns a name to the VertexBuffer and chooses to serialize it, they would naturally expect that name to be preserved.

This PR seems preparatory to implementing this other change I would like to add. What are your thoughts on this?
#2356

[yaRnMcDonuts]

The name variable will also get set to a non-null default value anytime the getName() method is called:

jmonkeyengine/jme3-core/src/main/java/com/jme3/scene/VertexBuffer.java

Line 1199 in d1c8521

name = getClass().getSimpleName() + "(" + getBufferType().name() + ")";

So if a scene is opened in an editor that calls getName(), then suddenly all your meshes are saving a few extra redundant default Strings.

I'd say its okay for it to be saved when it is a uniquely set name sicne there is atleast some usecase for that, but not when name is equal to the non-unique default value that gets assigned when getName() is called. I'd just leave name unsaved in that case, and then it will always set itself back to that default value anyways.

[capdevon]

The name variable will also get set to a non-null default value anytime the getName() method is called:

jmonkeyengine/jme3-core/src/main/java/com/jme3/scene/VertexBuffer.java

Line 1199 in d1c8521

name = getClass().getSimpleName() + "(" + getBufferType().name() + ")";

This is an unusual case. As it stands, it causes confusion and complicates the code. I propose to modify the getName() method to be a pure getter. This means it retrieves information about the object's state without altering that state.

Edit:
Note: The VertexBuffer.getName() method is used only by GLRenderer class, when debug mode is on.

https://github.com/jMonkeyEngine/jmonkeyengine/blob/master/jme3-core/src/main/java/com/jme3/renderer/opengl/GLRenderer.java#L3222

        if (debug && caps.contains(Caps.GLDebug)) {
            if (vb.getName() != null) glext.glObjectLabel(GLExt.GL_BUFFER, vb.getId(), vb.getName());
        }

I don't have a strong opinion on this PR's necessity right now. Without using RenderDoc for debugging, I can't definitively say if it's indispensable. It's possible the name field is intentionally not serialized. However, serialization of the name field shouldn't significantly impact memory usage for assets. UTF-8 encoding uses one byte per character for the English alphabet, though this can vary for other character sets.

Regarding the API's design, I believe the getName() method should not alter the internal state of the VertexBuffer class. Using String.format is an excellent choice for this, as it optimizes performance during continuous invocations. Additionally, the name field should be serialized and deserialized when it's not null.

Even if we close this PR, users can still debug the necessary information by using this code snippet before running SimpleApplication in GLDebug mode with RenderDoc:

    private void debugVertexBuffers(Spatial subtree) {
        subtree.depthFirstTraversal(new SceneGraphVisitorAdapter() {
            @Override
            public void visit(Geometry geom) {
                for (IntMap.Entry<VertexBuffer> entry : geom.getMesh().getBuffers()) {
                    VertexBuffer buff = entry.getValue();
                    buff.setName(geom.getName() + "(" + buff.getBufferType().name() + ")");
                }
            }
        });
    }

Let's leave this PR open for a while so that we have time to reflect...

[yaRnMcDonuts]

Regarding the API's design, I believe the getName() method should not alter the internal state of the VertexBuffer class

I agree. While I do sometimes like to take this approach of declaring a variable in a getter when it makes sense, In this case I don't see any benefit and the getter should simply just return name or a default String if null (and not store it to the name field). That would be an effective solution to prevent the name from being saved when its redundant and not necessary.

[capdevon]

the getter should simply just return name or a default String if null (and not store it to the name field). That would be an effective solution to prevent the name from being saved when its redundant and not necessary.

Great—then the current version of this PR should be perfect for every need.