Browse Source

[cpp] Enhance circular reference display with deterministic object identifiers

Replace "<circular>" with meaningful reference strings using a hybrid approach:
- Objects with names: <EventData-walk>, <BoneData-head>, <Animation-run>
- Objects without names: <TrackEntry-1>, <Bone-2>, <SliderTimeline-3>

Each serialized object now includes "refString" as its first field, enabling
easy navigation from circular references to full object definitions.
Mario Zechner 1 tháng trước cách đây
mục cha
commit
6f81d43faa

Những thai đổi đã bị hủy bỏ vì nó quá lớn
+ 307 - 105
spine-cpp/tests/SkeletonSerializer.h


Những thai đổi đã bị hủy bỏ vì nó quá lớn
+ 303 - 150
spine-libgdx/spine-libgdx-tests/src/com/esotericsoftware/spine/utils/SkeletonSerializer.java


+ 78 - 11
tests/src/generate-cpp-serializer.ts

@@ -78,7 +78,7 @@ function generatePropertyCode (property: Property, indent: string, enumMappings:
 			}
 			break;
 
-		case "enum":
+		case "enum": {
 			const enumName = property.enumName;
 			const enumMap = enumMappings[enumName];
 
@@ -99,8 +99,8 @@ function generatePropertyCode (property: Property, indent: string, enumMappings:
 				lines.push(`${indent}_json.writeValue(String::valueOf((int)${accessor}));`);
 			}
 			break;
-
-		case "array":
+		}
+		case "array": {
 			// In C++, arrays are never null - empty arrays (size() == 0) are equivalent to Java null
 			lines.push(`${indent}_json.writeArrayStart();`);
 			lines.push(`${indent}for (size_t i = 0; i < ${accessor}.size(); i++) {`);
@@ -113,7 +113,7 @@ function generatePropertyCode (property: Property, indent: string, enumMappings:
 			lines.push(`${indent}}`);
 			lines.push(`${indent}_json.writeArrayEnd();`);
 			break;
-
+		}
 		case "nestedArray":
 			// Nested arrays are always considered non-null in both Java and C++
 			lines.push(`${indent}_json.writeArrayStart();`);
@@ -148,7 +148,8 @@ function generateCppFromIR (ir: SerializerIR): string {
 	cppOutput.push('');
 	cppOutput.push('class SkeletonSerializer {');
 	cppOutput.push('private:');
-	cppOutput.push('    HashMap<void*, bool> _visitedObjects;');
+	cppOutput.push('    HashMap<void*, String> _visitedObjects;');
+	cppOutput.push('    int _nextId;');
 	cppOutput.push('    JsonWriter _json;');
 	cppOutput.push('');
 	cppOutput.push('public:');
@@ -161,6 +162,7 @@ function generateCppFromIR (ir: SerializerIR): string {
 		const cppParamType = transformType(method.paramType);
 		cppOutput.push(`    String ${method.name}(${cppParamType}* ${method.paramName}) {`);
 		cppOutput.push('        _visitedObjects.clear();');
+		cppOutput.push('        _nextId = 1;');
 		cppOutput.push('        _json = JsonWriter();');
 		cppOutput.push(`        ${method.writeMethodCall}(${method.paramName});`);
 		cppOutput.push('        return _json.getString();');
@@ -172,19 +174,46 @@ function generateCppFromIR (ir: SerializerIR): string {
 
 	// Generate write methods
 	for (const method of ir.writeMethods) {
-		const shortName = method.paramType.split('.').pop()!;
+		const shortName = method.paramType.split('.').pop();
 		const cppType = transformType(method.paramType);
 
 		// Custom writeSkin and writeSkinEntry implementations
 		if (method.name === 'writeSkin') {
 			cppOutput.push('    void writeSkin(Skin* obj) {');
 			cppOutput.push('        if (_visitedObjects.containsKey(obj)) {');
-			cppOutput.push('            _json.writeValue("<circular>");');
+			cppOutput.push('            _json.writeValue(_visitedObjects[obj]);');
 			cppOutput.push('            return;');
 			cppOutput.push('        }');
-			cppOutput.push('        _visitedObjects.put(obj, true);');
+
+			// Generate reference string for this object (only when first encountered)
+			// Only use name if there's a proper getName() method returning String
+			const nameGetter = method.properties.find(p =>
+				(p.kind === 'object' || p.kind === "primitive") &&
+				p.getter === 'getName()' &&
+				p.valueType === 'String'
+			);
+
+			if (nameGetter) {
+				// Use getName() if available and returns String
+				cppOutput.push('        String name = obj->getName();');
+				cppOutput.push('        String refString;');
+				cppOutput.push('        if (!name.isEmpty()) {');
+				cppOutput.push(`            refString.append("<${shortName}-").append(name).append(">");`);
+				cppOutput.push('        } else {');
+				cppOutput.push(`            refString.append("<${shortName}-").append(_nextId++).append(">");`);
+				cppOutput.push('        }');
+			} else {
+				// No suitable name getter - use numbered ID
+				cppOutput.push(`        String refString = String("<${shortName}-").append(_nextId++).append(">");`);
+			}
+			cppOutput.push('        _visitedObjects.put(obj, refString);');
+			cppOutput.push('');
 			cppOutput.push('');
 			cppOutput.push('        _json.writeObjectStart();');
+
+			cppOutput.push('        _json.writeName("refString");');
+			cppOutput.push('        _json.writeValue(refString);');
+
 			cppOutput.push('        _json.writeName("type");');
 			cppOutput.push('        _json.writeValue("Skin");');
 			cppOutput.push('');
@@ -230,6 +259,18 @@ function generateCppFromIR (ir: SerializerIR): string {
 			// Custom writeSkinEntry implementation
 			cppOutput.push('    void writeSkinEntry(Skin::AttachmentMap::Entry* obj) {');
 			cppOutput.push('        _json.writeObjectStart();');
+
+			// Generate refString using the name field if available
+			cppOutput.push('        String name = obj->_name;');
+			cppOutput.push('        String refString;');
+			cppOutput.push('        if (!name.isEmpty()) {');
+			cppOutput.push(`            refString.append("<${shortName}-").append(name).append(">");`);
+			cppOutput.push('        } else {');
+			cppOutput.push(`            refString.append("<${shortName}-").append(_nextId++).append(">");`);
+			cppOutput.push('        }');
+			cppOutput.push('        _json.writeName("refString");');
+			cppOutput.push('        _json.writeValue(refString);');
+
 			cppOutput.push('        _json.writeName("type");');
 			cppOutput.push('        _json.writeValue("SkinEntry");');
 			cppOutput.push('        _json.writeName("slotIndex");');
@@ -251,7 +292,7 @@ function generateCppFromIR (ir: SerializerIR): string {
 			if (method.subtypeChecks && method.subtypeChecks.length > 0) {
 				let first = true;
 				for (const subtype of method.subtypeChecks) {
-					const subtypeShortName = subtype.typeName.split('.').pop()!;
+					const subtypeShortName = subtype.typeName.split('.').pop();
 
 					if (first) {
 						cppOutput.push(`        if (obj->getRTTI().instanceOf(${subtypeShortName}::rtti)) {`);
@@ -271,14 +312,40 @@ function generateCppFromIR (ir: SerializerIR): string {
 			// Handle concrete types
 			// Add cycle detection
 			cppOutput.push('        if (_visitedObjects.containsKey(obj)) {');
-			cppOutput.push('            _json.writeValue("<circular>");');
+			cppOutput.push('            _json.writeValue(_visitedObjects[obj]);');
 			cppOutput.push('            return;');
 			cppOutput.push('        }');
-			cppOutput.push('        _visitedObjects.put(obj, true);');
+
+			// Generate reference string for this object (only when first encountered)
+			// Only use name if there's a proper getName() method returning String
+			const nameGetter = method.properties.find(p =>
+				(p.kind === 'object' || p.kind === "primitive") &&
+				p.getter === 'getName()' &&
+				p.valueType === 'String'
+			);
+
+			if (nameGetter) {
+				// Use getName() if available and returns String
+				cppOutput.push('        String name = obj->getName();');
+				cppOutput.push('        String refString;');
+				cppOutput.push('        if (!name.isEmpty()) {');
+				cppOutput.push(`            refString.append("<${shortName}-").append(name).append(">");`);
+				cppOutput.push('        } else {');
+				cppOutput.push(`            refString.append("<${shortName}-").append(_nextId++).append(">");`);
+				cppOutput.push('        }');
+			} else {
+				// No suitable name getter - use numbered ID
+				cppOutput.push(`        String refString = String("<${shortName}-").append(_nextId++).append(">");`);
+			}
+			cppOutput.push('        _visitedObjects.put(obj, refString);');
 			cppOutput.push('');
 
 			cppOutput.push('        _json.writeObjectStart();');
 
+			// Write reference string as first field for navigation
+			cppOutput.push('        _json.writeName("refString");');
+			cppOutput.push('        _json.writeValue(refString);');
+
 			// Write type field
 			cppOutput.push('        _json.writeName("type");');
 			cppOutput.push(`        _json.writeValue("${shortName}");`);

+ 34 - 11
tests/src/generate-java-serializer.ts

@@ -40,7 +40,7 @@ function generatePropertyCode (property: Property, indent: string, method?: Writ
 			}
 			break;
 
-		case "array":
+		case "array": {
 			// Special handling for Skin attachments - sort by slot index
 			const isSkinAttachments = method?.paramType === 'Skin' && property.name === 'attachments' && property.elementType === 'SkinEntry';
 			const sortedAccessor = isSkinAttachments ? 'sortedAttachments' : accessor;
@@ -76,8 +76,8 @@ function generatePropertyCode (property: Property, indent: string, method?: Writ
 				lines.push(`${indent}json.writeArrayEnd();`);
 			}
 			break;
-
-		case "nestedArray":
+		}
+		case "nestedArray": {
 			if (property.isNullable) {
 				lines.push(`${indent}if (${accessor} == null) {`);
 				lines.push(`${indent}    json.writeNull();`);
@@ -108,6 +108,7 @@ function generatePropertyCode (property: Property, indent: string, method?: Writ
 				lines.push(`${indent}json.writeArrayEnd();`);
 			}
 			break;
+		}
 	}
 
 	return lines;
@@ -134,11 +135,12 @@ function generateJavaFromIR (ir: SerializerIR): string {
 	javaOutput.push('import com.badlogic.gdx.utils.FloatArray;');
 	javaOutput.push('');
 	javaOutput.push('import java.util.Locale;');
-	javaOutput.push('import java.util.Set;');
-	javaOutput.push('import java.util.HashSet;');
+	javaOutput.push('import java.util.Map;');
+	javaOutput.push('import java.util.HashMap;');
 	javaOutput.push('');
 	javaOutput.push('public class SkeletonSerializer {');
-	javaOutput.push('    private final Set<Object> visitedObjects = new HashSet<>();');
+	javaOutput.push('    private final Map<Object, String> visitedObjects = new HashMap<>();');
+	javaOutput.push('    private int nextId = 1;');
 	javaOutput.push('    private JsonWriter json;');
 	javaOutput.push('');
 
@@ -146,6 +148,7 @@ function generateJavaFromIR (ir: SerializerIR): string {
 	for (const method of ir.publicMethods) {
 		javaOutput.push(`    public String ${method.name}(${method.paramType} ${method.paramName}) {`);
 		javaOutput.push('        visitedObjects.clear();');
+		javaOutput.push('        nextId = 1;');
 		javaOutput.push('        json = new JsonWriter();');
 		javaOutput.push(`        ${method.writeMethodCall}(${method.paramName});`);
 		javaOutput.push('        json.close();');
@@ -156,7 +159,7 @@ function generateJavaFromIR (ir: SerializerIR): string {
 
 	// Generate write methods
 	for (const method of ir.writeMethods) {
-		const shortName = method.paramType.split('.').pop()!;
+		const shortName = method.paramType.split('.').pop();
 		const className = method.paramType.includes('.') ? method.paramType : shortName;
 
 		javaOutput.push(`    private void ${method.name}(${className} obj) {`);
@@ -166,7 +169,7 @@ function generateJavaFromIR (ir: SerializerIR): string {
 			if (method.subtypeChecks && method.subtypeChecks.length > 0) {
 				let first = true;
 				for (const subtype of method.subtypeChecks) {
-					const subtypeShortName = subtype.typeName.split('.').pop()!;
+					const subtypeShortName = subtype.typeName.split('.').pop();
 					const subtypeClassName = subtype.typeName.includes('.') ? subtype.typeName : subtypeShortName;
 
 					if (first) {
@@ -186,15 +189,35 @@ function generateJavaFromIR (ir: SerializerIR): string {
 		} else {
 			// Handle concrete types
 			// Add cycle detection
-			javaOutput.push('        if (visitedObjects.contains(obj)) {');
-			javaOutput.push('            json.writeValue("<circular>");');
+			javaOutput.push('        if (visitedObjects.containsKey(obj)) {');
+			javaOutput.push('            json.writeValue(visitedObjects.get(obj));');
 			javaOutput.push('            return;');
 			javaOutput.push('        }');
-			javaOutput.push('        visitedObjects.add(obj);');
+			
+			// Generate reference string for this object (only when first encountered)
+			// Only use name if there's a proper getName() method returning String
+			const nameGetter = method.properties.find(p =>
+				(p.kind === 'object' || p.kind === "primitive") &&
+				p.getter === 'getName()' &&
+				p.valueType === 'String'
+			);
+
+			if (nameGetter) {
+				// Use getName() if available and returns String
+				javaOutput.push(`        String refString = obj.getName() != null ? "<${shortName}-" + obj.getName() + ">" : "<${shortName}-" + (nextId++) + ">";`);
+			} else {
+				// No suitable name getter - use numbered ID
+				javaOutput.push(`        String refString = "<${shortName}-" + (nextId++) + ">";`);
+			}
+			javaOutput.push('        visitedObjects.put(obj, refString);');
 			javaOutput.push('');
 
 			javaOutput.push('        json.writeObjectStart();');
 
+			// Write reference string as first field for navigation
+			javaOutput.push('        json.writeName("refString");');
+			javaOutput.push('        json.writeValue(refString);');
+
 			// Write type field
 			javaOutput.push('        json.writeName("type");');
 			javaOutput.push(`        json.writeValue("${shortName}");`);

+ 3 - 2
tests/src/headless-test-runner.ts

@@ -332,7 +332,7 @@ function executeCpp (args: TestArgs): string {
 		// Build using build.sh
 		log_action('Building C++ tests');
 		try {
-			execSync('./build.sh clean release', {
+			execSync('./build.sh clean debug', {
 				cwd: cppDir,
 				stdio: ['inherit', 'pipe', 'inherit']
 			});
@@ -593,7 +593,8 @@ function main (): void {
 	log_title('Spine Runtime Test');
 
 	// Clean output directory first
-	cleanOutputDirectory();
+	// TODO annoying during development of generators as it also smokes generator outputs
+	// cleanOutputDirectory();
 
 	if (args.files) {
 		// Auto-discovery mode: run tests for both JSON and binary files

+ 0 - 0
todos/work/2025-01-11-03-02-54-test-suite/task.md → todos/done/2025-01-11-03-02-54-test-suite.md


+ 22 - 0
todos/done/2025-01-21-18-49-54-logging-indentation.md

@@ -0,0 +1,22 @@
+# Improve bash script logging indentation for nested calls
+**Status:** Done
+**Agent PID:** 30228
+
+## Original Todo
+- clean up logging in spine-c/codegen, use chalk to do colored warnings/errors and make logging look very nice and informative (no emojis)
+
+## Description
+Implement automatic indentation for bash script logging when scripts call other scripts. Currently, when a script calls another script, both use the same logging functions from `formatters/logging/logging.sh`, making it difficult to distinguish the call hierarchy. The solution will detect script nesting level by analyzing the process tree and automatically indent log output from child scripts.
+
+## Implementation Plan
+- [x] Create process tree analysis utility to detect script nesting level
+- [x] Add nesting level detection that runs once when logging.sh is sourced
+- [x] Store indentation level in a local variable (SPINE_LOG_INDENT_SPACES) - **not exported**
+- [x] Modify log_action(), log_warn(), and log_detail() to use the pre-calculated local indentation
+- [x] Test with nested script calls (format.sh calling format-cpp.sh, etc.)
+- [x] Ensure backward compatibility and graceful fallback if detection fails
+- [x] Update documentation to explain the new indentation behavior
+- [x] Add script filename to log_title output for better clarity
+
+## Notes
+26 bash scripts in the codebase use logging.sh functionality, including build, test, format, setup, and publish scripts. All would benefit from proper indentation when calling each other.

+ 74 - 0
todos/done/2025-01-21-19-06-33-circular-reference-display-analysis.md

@@ -0,0 +1,74 @@
+# Circular Reference Display Analysis
+
+## Current Implementation
+
+The circular reference detection is implemented in:
+
+### 1. **Java SkeletonSerializer** 
+**File:** `spine-libgdx/spine-libgdx-tests/src/com/esotericsoftware/spine/utils/SkeletonSerializer.java`
+- Uses pattern: `if (visitedObjects.contains(obj)) { json.writeValue("<circular>"); return; }`
+- Found on 100+ lines throughout write methods
+
+### 2. **C++ SkeletonSerializer**
+**File:** `spine-cpp/tests/SkeletonSerializer.h`
+- Uses pattern: `if (_visitedObjects.containsKey(obj)) { _json.writeValue("<circular>"); return; }`
+- Similar implementation to Java version
+
+### 3. **Generator Scripts**
+- `tests/src/generate-java-serializer.ts` (lines 189-192)
+- `tests/src/generate-cpp-serializer.ts` (lines 182-183, 274-275)
+- Both inject the circular reference detection pattern
+
+### 4. **Test Runner**
+- `tests/src/headless-test-runner.ts` (line 409)
+- Handles "<circular>" values during JSON comparison
+
+## Object Identification Patterns
+
+### Name-based Identification
+Most Spine objects use `name` as primary identifier:
+```java
+// Event.java - toString returns the event's name
+public String toString() { return data.name; }
+
+// Attachment.java - toString returns attachment name  
+public String toString() { return name; }
+
+// Skeleton.java - toString returns skeleton data name or falls back
+public String toString() {
+    return data.name != null ? data.name : super.toString();
+}
+```
+
+### ID-based Identification
+TypeScript VertexAttachment uses auto-incrementing IDs:
+```typescript
+export abstract class VertexAttachment extends Attachment {
+    private static nextID = 0;
+    /** The unique ID for this attachment. */
+    id = VertexAttachment.nextID++;
+}
+```
+
+### Type-based Identification
+Serializers include explicit type information:
+```java
+json.writeName("type");
+json.writeValue("Animation");
+```
+
+## Current Limitations
+
+1. **No type information** - Can't quickly understand what type of object caused the circular reference
+2. **No unique identification** - Can't distinguish between multiple circular references of the same type  
+3. **No navigation aid** - Can't easily locate the original object definition in large object graphs
+
+## Enhancement Opportunities
+
+The existing infrastructure supports enhancement:
+- Consistent naming patterns across objects
+- Unique ID patterns could be expanded
+- Type information already captured in serializers
+- Object reference tracking infrastructure is mature
+
+Enhanced format could be: `<circular: Animation#"walk">` or `<circular: Bone#"head">` instead of just `<circular>`.

+ 35 - 0
todos/done/2025-01-21-19-06-33-circular-reference-display.md

@@ -0,0 +1,35 @@
+# Circular Reference Display Enhancement
+**Status:** Done
+**Agent PID:** 67832
+
+## Original Todo
+<circular> should include the type and a unique id for the object, so i can quckly understand what's been omitted navigate to it
+
+## Description
+Replace "<circular>" with deterministic object identifiers in Spine serializers. The implementation uses a hybrid approach: objects with name properties use "<ObjectType-name>" format (e.g. "<EventData-walk>"), while objects without names use numbered IDs "<ObjectType-1>", "<ObjectType-2>", etc. This ensures identical output across Java/C++ by using deterministic reference strings that are generated only when objects are first encountered.
+
+## Implementation Plan
+- [x] Update Java generator script - Modified `tests/src/generate-java-serializer.ts` to use Map<Object, String> visitedObjects with `nextId` counter, generating name-based or numbered reference strings
+- [x] Update C++ generator script - Modified `tests/src/generate-cpp-serializer.ts` to use HashMap<void*, String> with `_nextId` counter, matching Java logic with proper Spine String API usage
+- [x] Regenerate serializers - Generated updated Java and C++ SkeletonSerializer classes with new circular reference handling
+- [x] Fix deterministic IDs - Implemented hybrid system: `<Type-name>` for named objects, `<Type-N>` for unnamed objects, ensuring identical output across languages
+- [x] Update test runner - Test runner handles new circular reference format correctly
+- [x] Automated test: Serialization tests pass with no regressions
+- [x] User test: Circular references now show meaningful format like `<EventData-walk>` or `<Animation-1>` instead of `<circular>`
+- [x] Add reference string field: Include refString as first field in each object's JSON to enable quick navigation from circular references to full objects
+
+## Notes
+### Implementation Details:
+- **Circular Reference Logic**: Changed from simple boolean tracking to String-based reference storage
+- **Reference String Generation**: Only occurs when object is first encountered (after cycle detection check)
+- **Name Detection**: Uses `p.getter === 'getName()' && p.valueType === 'String'` to identify objects with meaningful names
+- **Deterministic Numbering**: Both Java and C++ use identical counter logic, ensuring same numbered IDs for unnamed objects
+- **Types with Names**: BoneData, EventData, IkConstraintData, PathConstraintData, PhysicsConstraintData, Animation, and attachment types
+- **Output Format**: 
+  - Named: `<EventData-walk>`, `<BoneData-head>`, `<Animation-run>`
+  - Unnamed: `<TrackEntry-1>`, `<Bone-2>`, `<SliderTimeline-3>`
+- **Navigation Enhancement**: Each serialized object includes `"refString"` as its first field, making it easy to jump from circular reference to full object definition
+
+### Commands:
+- `tests/generate-serializers.sh` -> generates the serializers
+- `tests/test.sh cpp -s spineboy -f` -> compiles and runs the tests, and outputs whether the generated json files are identical

+ 0 - 1
todos/todos.md

@@ -1,4 +1,3 @@
-- clean up logging in spine-c/codegen, use chalk to do colored warnings/errors and make logging look very nice and informative (no emojis)
 - spine-c/codegen type extractor should also report typedefs like typedef long long PropertyId; so primitive type to some name, and we need to handle that in the codegen
 - Generate language bindings in spine-c/codegen
     - Use CClassOrStruct, CEnum that get generated from spine-cpp-types.json and generate

Một số tệp đã không được hiển thị bởi vì quá nhiều tập tin thay đổi trong này khác