[neph1]

Summary

• Parsing a j3md with statements the SDK parser cannot handle used to throw raw exceptions (e.g. ArrayIndexOutOfBoundsException in DefineBlock.parse) which surfaced as the big "jMonkeyEngine SDK Exception Report" dialog.
• New MatDefParseException carries the offending line number and content, with a message that explains what is wrong and what was expected.
• All fileStructure leaf parsers (Define, MatParam, LightMode, ShaderFile, Input/OutputMapping, Definition, Condition) and the block name parsers (MaterialDef, Technique, ShaderNode) now throw MatDefParseException with a specific message instead of crashing or silently swallowing errors.
• DefineBlock tolerates the older/hand-written format where a define has no mapped material parameter (e.g. HAS_COLORMAP // Must be passed so that Unshaded.vert exports texCoord. from issue This means that there was an unexpected exception when parsing the j3md #650): the line is kept, the define stays unmapped (always set, matching engine behavior) and a warning with the line number is logged.
• ShaderFileBlock parses multi-language shader statements (VertexShader GLSL300 GLSL100: path.vert) correctly and exposes getLanguage()/getPath().
• EditableMatDefFile catches MatDefParseException, keeps whatever structure was parsed, marks itself loaded and notifies the user with a warning notification naming the file, line and problem instead of the exception report dialog. The visual editor shows the same explanation and still renders the parts that parsed.
• Added TechniqueBlock.getDefines() and unit tests covering the old define format, shader statements and the full j3md from issue This means that there was an unexpected exception when parsing the j3md #650.

Verification

• No JDK available in the sandbox, so compilation/tests could not be run locally. Brace balance and parse-logic simulation verified manually; unit tests added under jme3-materialeditor/test/unit (JUnit 5, matching existing test setup).

Closes #650

[neph1]

Tested locally. I can't find my complex shader nodes, but it works on the default created one. I've tried a few others and I don't see the big ugly exception.

[neph1]

One note: I can't go to the "Shader" tab without passing "Editor" first. But I think that's a separate issue.

[neph1]

@jaime-jmebot would you mind reviewing this?

[jaime-jmebot]

In string concatenation, the escaped quotes \" cause + lightMode + to be included literally inside the string message instead of evaluating the lightMode variable. Unescaping the double quotes around lightMode allows proper string concatenation.

Suggested change

	throw new MatDefParseException("Invalid light mode \" + lightMode + \", expected one of " + java.util.Arrays.toString(TechniqueDef.LightMode.values()), sta.getLineNumber(), sta.getLine(), ex);
	throw new MatDefParseException("Invalid light mode \"" + lightMode + "\", expected one of " + java.util.Arrays.toString(TechniqueDef.LightMode.values()), sta.getLineNumber(), sta.getLine(), ex);

[jaime-jmebot]

Splitting on single whitespace (\\s) produces empty string array entries when there are multiple consecutive spaces or tabs between MaterialDef and the material name (e.g., MaterialDef MyMaterial). Consequently, s[1].trim().isEmpty() evaluates to true and throws a MatDefParseException even when a valid name is present.

Using line.trim().split("\\s+") ensures whitespace runs are treated as a single delimiter.

Suggested change

	if (s.length < 2 || s[1].trim().isEmpty()) {
	throw new MatDefParseException("MaterialDef statement is missing a name, expected \"MaterialDef MyMaterial\"", getLineNumber(), getLine());
	}
	name = s[1];
	String[] s = line.trim().split("\\s+");
	if (s.length < 2) {
	throw new MatDefParseException("MaterialDef statement is missing a name, expected \"MaterialDef MyMaterial\"", getLineNumber(), getLine());
	}
	name = s[1];

[jaime-jmebot]

Similar to MatDefBlock, splitting on \\s will produce empty string entries if there is multiple consecutive whitespace or tabs between ShaderNode and the node name. Using line.trim().split("\\s+") prevents throwing a false MatDefParseException.

Suggested change

	if (s.length < 2 || s[1].trim().isEmpty()) {
	throw new MatDefParseException("ShaderNode statement is missing a name, expected \"ShaderNode MyNode\"", getLineNumber(), getLine());
	String[] s = line.trim().split("\\s+");
	if (s.length < 2) {
	throw new MatDefParseException("ShaderNode statement is missing a name, expected \"ShaderNode MyNode\"", getLineNumber(), getLine());
	}

[jaime-jmebot]

Splitting on \\s produces empty strings when multiple spaces or tabs separate Technique and the technique name. Using line.trim().split("\\s+") ensures robust tokenization across varying whitespace formatting.

Suggested change

	String[] s = line.split("\\s");
	if (s.length == 1) {
	name = "Default";
	} else if (s[1].trim().isEmpty()) {
	throw new MatDefParseException("Technique statement is missing a name, expected \"Technique MyTechnique\"", getLineNumber(), getLine());
	} else {
	String[] s = line.trim().split("\\s+");
	if (s.length == 1) {
	name = "Default";
	} else {
	name = s[1];
	}

[jaime-jmebot]

Hey @neph1, thanks for putting this together and asking for a review!

This is a great improvement to how the material definition editor handles parse failures. Replacing unhandled runtime exceptions with MatDefParseException significantly improves the user experience—giving informative warning dialogs in the IDE and rendering whatever partial material definition could be parsed, rather than presenting users with the generic exception reporter modal.

Technical Discussion

Here is what was analyzed across the changes:

• Error Handling & UX Flow (EditableMatDefFile, MatDefEditorlElement): Catching MatDefParseException specifically while retaining the partially parsed matDefStructure works very cleanly. It allows the editor and diagram to degrade gracefully when unsupported or partially invalid syntax is encountered, while warning the user about the exact line to check. The null guard on technique in initDiagramInternal also prevents follow-on NPEs when parsing fails before any technique can be constructed.
• Handling Unmapped Defines (DefineBlock): Tolerating defines without explicitly mapped material parameters (and keeping them unmapped with a log warning) directly addresses legacy/handwritten j3md files like the one reported in #650.
• Multi-language Shader Specifications (ShaderFileBlock): Accommodating statements like VertexShader GLSL300 GLSL100: path.vert is a welcome addition that accurately tracks the languages and shader paths.
• Test Suite (DefineBlockTest): Adding tests for single-language shaders, multi-language shaders, unmapped defines, and verifying the #650 scenario provides solid regression coverage.

I've left a few minor inline suggestions on small details:

1. In LightModeBlock, there is a minor string escaping issue where \" was used instead of unescaped quotes for string concatenation, which resulted in + lightMode + appearing literally in the exception message rather than printing the invalid light mode string.
2. In MatDefBlock, ShaderNodeBlock, and TechniqueBlock, splitting on single \\s instead of \\s+ causes empty array elements when multiple consecutive spaces or tabs are used between the keyword and name, triggering a parse exception on valid files. Using split("\\s+") handles arbitrary whitespace smoothly.

Once those small items are touched up, this should be ready to roll! Really nice work on improving the robustness of the material editor parser.