add allowHomonymObjectName option for OBJModel.ModelSettings #9047
Conversation
rename only works on the `o` .obj command/keyword now when allowHomonymObjectName is true and rename happens just plus char '_' and a unique number id, so you can just get all homonym parts by call filter and use methods like startWith with the homonym part as parameter
| public record ModelSettings(@NotNull ResourceLocation modelLocation, | ||
| boolean automaticCulling, boolean shadeQuads, boolean flipV, | ||
| boolean emissiveAmbient, @Nullable String mtlOverride) | ||
| boolean emissiveAmbient, @Nullable String mtlOverride, boolean allowHomonymObjectName) | ||
| { } |
There was a problem hiding this comment.
This is a breaking change because any callers to the previous constructor (without the new parameter) will break.
Please add a new constructor to this record which calls the canonical constructor with an appropriate default value for the new parameter, mark that new constructor as deprecated for removal (@Deprecated(forRemoval = true, since = "1.19.2"), and add a javadoc with a @deprecated tag directing users to the canonical constructor (which has the new parameter).
| @@ -299,6 +316,10 @@ static ObjModel parse(ObjTokenizer tokenizer, ModelSettings settings, Map<String | |||
| } | |||
| } | |||
| } | |||
| if (!allowHomonymObjectName && hasHomonymObjectNameAppeared) | |||
| { | |||
| LOGGER.error("find homonym object in obj model:" + settings.modelLocation + ", use allow_homonym_object_name=true can rename them for you"); | |||
There was a problem hiding this comment.
Some error message cleanup, as well as using logger parameter substitution instead of manual string concatenation:
| LOGGER.error("find homonym object in obj model:" + settings.modelLocation + ", use allow_homonym_object_name=true can rename them for you"); | |
| LOGGER.error("Found homonym object in ObjModel: {}; use the 'allow_homonym_object_name' option to automatically resolve homonyms", settings.modelLocation); |
| hasHomonymObjectNameAppeared = true; | ||
| if (allowHomonymObjectName) | ||
| { | ||
| int suffixNum = 0; |
There was a problem hiding this comment.
I wonder if we should begin this at 1 instead, so the names would be: object, object_1, object_2.
| @@ -57,6 +57,7 @@ public ObjModel read(JsonObject jsonObject, JsonDeserializationContext deseriali | |||
| boolean flipV = GsonHelper.getAsBoolean(jsonObject, "flip_v", false); | |||
| boolean emissiveAmbient = GsonHelper.getAsBoolean(jsonObject, "emissive_ambient", true); | |||
| String mtlOverride = GsonHelper.getAsString(jsonObject, "mtl_override", null); | |||
| boolean allowHomonymObjectName = GsonHelper.getAsBoolean(jsonObject,"allow_homonym_object_name",false); | |||
There was a problem hiding this comment.
Formatting: spaces after commas.
| boolean allowHomonymObjectName = GsonHelper.getAsBoolean(jsonObject,"allow_homonym_object_name",false); | |
| boolean allowHomonymObjectName = GsonHelper.getAsBoolean(jsonObject, "allow_homonym_object_name", false); |
| @@ -86,7 +87,7 @@ public ObjModel read(JsonObject jsonObject, JsonDeserializationContext deseriali | |||
| deprecationWarningsBuilder.put("materialLibraryOverride", "mtl_override"); | |||
| } | |||
|
|
|||
| return loadModel(new ObjModel.ModelSettings(new ResourceLocation(modelLocation), automaticCulling, shadeQuads, flipV, emissiveAmbient, mtlOverride), deprecationWarningsBuilder.build()); | |||
| return loadModel(new ObjModel.ModelSettings(new ResourceLocation(modelLocation), automaticCulling, shadeQuads, flipV, emissiveAmbient, mtlOverride,allowHomonymObjectName), deprecationWarningsBuilder.build()); | |||
There was a problem hiding this comment.
Formatting: spaces after commas.
| return loadModel(new ObjModel.ModelSettings(new ResourceLocation(modelLocation), automaticCulling, shadeQuads, flipV, emissiveAmbient, mtlOverride,allowHomonymObjectName), deprecationWarningsBuilder.build()); | |
| return loadModel(new ObjModel.ModelSettings(new ResourceLocation(modelLocation), automaticCulling, shadeQuads, flipV, emissiveAmbient, mtlOverride, allowHomonymObjectName), deprecationWarningsBuilder.build()); |
sciwhiz12
removed
the
Triage
|
|
This PR adds the ability for the OBJ Loder to load .obj file with the same part name
this PR follows #8624, but for Minecraft version 1.19.2 and a small difference.
I don't know how to change an existing GitHub PR's source/target branch, so I just open a new one
Why need this?
for modeling software such as BlockBench which allows homonym cube names. When its model is exported as a .obj model file. The name subsequent to the .obj format command/keyword
owill duplicateDue to the current OBJ Loader implementation, the collection of the parts is a
Map<String, ModelGroup> parts, so the latter will override the former. As a result, only the last part will remainImplementation
add an option bool for
OBJModel.SettingcalledallowHomonymObjectNamewhich is false by default.when reading from json file, its key is
allow_homonym_object_namewe now will check whether the same part name occurs before, if it is and the option is true, we re-name it by plusing a char
_and a non-negative growth number after the original name until no-repeat occurs.In a similar way to another modeling software called Blender, it can correctly load .obj file with homonym object names
re-name only works on the
o.obj command/keyword nowAddition
When finding any part name that occurs before, we use a bool to record this. When load end we print an error message that indicates this and prompts this option when the option is false as this is usually not expected behavior.