Merge pull request #223 from aether-framework/bugfix/117-118-119-120-121-122-123-api-bugfixes

Remove unused methods and improve functionality in core classes

Author: Splatcrafter

aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/dynamic/TaggedDynamic.java

@@ -179,4 +179,68 @@ public TypeReference type() {
     public Dynamic<?> value() {
         return this.value;
     }
+
+    /**
+     * Indicates whether some other object is "equal to" this one.
+     *
+     * <p>Two {@code TaggedDynamic} instances are considered equal if they have the same type reference and the same
+     * dynamic value.
+     * This means that both the type and the data must match for two tagged dynamics to be considered equal.</p>
+     *
+     * <h4>Example</h4>
+     * <pre>{@code
+     * TaggedDynamic tagged1 = new TaggedDynamic(typeRef, dynamicValue);
+     * TaggedDynamic tagged2 = new TaggedDynamic(typeRef, dynamicValue);
+     *
+     * // tagged1 and tagged2 are equal because they have the same type and value
+     * boolean isEqual = tagged1.equals(tagged2); // true
+     * }</pre>
+     *
+     * @param obj the reference object with which to compare
+     * @return {@code true} if this object is the same as the obj argument; {@code false} otherwise
+     */
+    @Override
+    public boolean equals(final Object obj) {
+        if (this == obj) {
+            return true;
+        }
+        if (obj == null || getClass() != obj.getClass()) {
+            return false;
+        }
+
+        final TaggedDynamic that = (TaggedDynamic) obj;
+        return this.type.equals(that.type) && this.value.equals(that.value);
+    }
+
+    /**
+     * Returns a hash code value for the object.
+     *
+     * <p>The hash code is computed based on both the type reference and the dynamic value. This ensures that equal
+     * {@code TaggedDynamic} instances will have the same hash code, which is important for correct behavior in
+     * hash-based collections.</p>
+     *
+     * @return a hash code value for this object
+     */
+    @Override
+    public int hashCode() {
+        int result = this.type.hashCode();
+        result = 31 * result + this.value.hashCode();
+        return result;
+    }
+
+    /**
+     * Returns a string representation of the object.
+     *
+     * <p>The string representation includes the type reference and the dynamic value, providing a human-readable
+     * summary of the tagged dynamic's contents. This can be useful for debugging and logging purposes.</p>
+     *
+     * @return a string representation of the object
+     */
+    @Override
+    public String toString() {
+        return "TaggedDynamic{" +
+                "type=" + this.type +
+                ", value=" + this.value +
+                '}';
+    }
 }

aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/fix/Fixes.java

@@ -121,7 +121,7 @@ private Fixes() {
      * @param rewrite    the function that transforms the typed value; must not be {@code null}
      * @return a type rewrite rule that applies the transformation to matching types; never {@code null}
      * @throws NullPointerException if any parameter is {@code null}
-     * @see #fixTypeEverywhere(String, Type, Function)
+     * @see #fixTypeEverywhere(String, DynamicOps, Type, Function)
      */
     @NotNull
     public static TypeRewriteRule fixTypeEverywhereTyped(@NotNull final String name,
@@ -156,22 +156,44 @@ public String toString() {
     /**
      * Creates a rule that transforms the dynamic representation of a type.
      *
-     * @param name    the fix name
-     * @param type    the type to transform
-     * @param rewrite the dynamic transformation function
-     * @return a type rewrite rule
+     * <p>This method creates a {@link TypeRewriteRule} that matches values of the specified
+     * type and transforms them by encoding to {@link Dynamic}, applying the rewrite function, and decoding back. The
+     * rule only matches types with the same {@link de.splatgames.aether.datafixers.api.TypeReference} as the target
+     * type.</p>
+     *
+     * <h4>Example</h4>
+     * <pre>{@code
+     * TypeRewriteRule rule = Fixes.fixTypeEverywhere(
+     *     "addTimestamp",
+     *     GsonOps.INSTANCE,
+     *     playerType,
+     *     dynamic -> dynamic.set("timestamp", dynamic.createLong(0L))
+     * );
+     * }</pre>
+     *
+     * @param <T>     the dynamic type
+     * @param name    the fix name; must not be {@code null}
+     * @param ops     the dynamic ops for encoding/decoding; must not be {@code null}
+     * @param type    the type to transform; must not be {@code null}
+     * @param rewrite the dynamic transformation function; must not be {@code null}
+     * @return a type rewrite rule; never {@code null}
+     * @throws NullPointerException if any parameter is {@code null}
+     * @see #fixTypeEverywhereTyped(String, Type, Type, Function)
      */
     @NotNull
-    public static TypeRewriteRule fixTypeEverywhere(@NotNull final String name,
-                                                    @NotNull final Type<?> type,
-                                                    @NotNull final Function<Dynamic<?>, Dynamic<?>> rewrite) {
+    public static <T> TypeRewriteRule fixTypeEverywhere(@NotNull final String name,
+                                                        @NotNull final DynamicOps<T> ops,
+                                                        @NotNull final Type<?> type,
+                                                        @NotNull final Function<Dynamic<?>, Dynamic<?>> rewrite) {
         Preconditions.checkNotNull(name, "name must not be null");
+        Preconditions.checkNotNull(ops, "ops must not be null");
         Preconditions.checkNotNull(type, "type must not be null");
         Preconditions.checkNotNull(rewrite, "rewrite must not be null");
 
         return new TypeRewriteRule() {
             @NotNull
             @Override
+            @SuppressWarnings({"unchecked", "rawtypes"})
             public Optional<Typed<?>> rewrite(@NotNull final Type<?> inputType,
                                               @NotNull final Typed<?> input) {
                 Preconditions.checkNotNull(inputType, "inputType must not be null");
@@ -180,9 +202,11 @@ public Optional<Typed<?>> rewrite(@NotNull final Type<?> inputType,
                     return Optional.empty();
                 }
 
-                // We need to get DynamicOps from somewhere - this requires the caller to provide it
-                // For now, we'll use a simplified approach
-                return Optional.of(input);
+                final DataResult<Dynamic<T>> encodeResult = input.encode(ops);
+                return encodeResult.flatMap(dynamic -> {
+                    final Dynamic<?> transformed = rewrite.apply(dynamic);
+                    return ((Type) type).read(transformed);
+                }).map(newValue -> new Typed<>((Type) type, newValue)).result();
             }
 
             @Override

aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/optic/Finder.java

@@ -179,8 +179,10 @@ public Dynamic<?> set(@NotNull final Dynamic<?> root,
      * Creates a finder that navigates to an element by index in a list/array structure.
      *
      * <p>The returned finder extracts and modifies the element at the specified
-     * index position. If the index is out of bounds (negative or beyond the list size), {@link #get} returns
-     * {@code null} and {@link #set} returns the root unchanged.</p>
+     * index position. A negative {@code index} is rejected immediately by throwing
+     * {@link IllegalArgumentException} at construction time. If the index is non-negative
+     * but beyond the list size, {@link #get} returns {@code null} and {@link #set} returns
+     * the root unchanged.</p>
      *
      * <h4>Example</h4>
      * <pre>{@code
@@ -195,16 +197,21 @@ public Dynamic<?> set(@NotNull final Dynamic<?> root,
      * Dynamic<?> updated = firstScoreFinder.set(data, data.createInt(100));
      * // updated: {"scores": [100, 92, 78]}
      *
-     * // Out of bounds access
+     * // Out-of-range (positive) index — get returns null, set returns root unchanged
      * Finder<?> outOfBounds = scoresFinder.then(Finder.index(10));
      * Dynamic<?> missing = outOfBounds.get(data);  // null
+     *
+     * // Negative index — throws IllegalArgumentException immediately
+     * Finder.index(-1);  // throws IllegalArgumentException
      * }</pre>
      *
-     * @param index the zero-based index of the element to focus on
+     * @param index the zero-based index of the element to focus on; must not be negative
+     * @throws IllegalArgumentException if {@code index} is negative
      * @return a finder that navigates to the element at the specified index, never {@code null}
      */
     @NotNull
     static Finder<Object> index(final int index) {
+        Preconditions.checkArgument(index >= 0, "index must not be negative, got: %s", index);
         return new Finder<>() {
             @NotNull
             @Override
@@ -232,7 +239,7 @@ public String id() {
                     return root;
                 }
                 final var list = listResult.result().orElseThrow().toList();
-                if (index < 0 || index >= list.size()) {
+                if (index >= list.size()) {
                     return root;
                 }
                 final List<Dynamic<Object>> newList = new ArrayList<>();

aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/optic/Iso.java

@@ -384,13 +384,6 @@ public A modify(@NotNull final B source,
                 Preconditions.checkNotNull(modifier, "modifier must not be null");
                 return from(modifier.apply(to(source)));
             }
-
-            @NotNull
-            @Override
-            public <C, D> Optic<B, A, C, D> compose(@NotNull final Optic<T, S, C, D> other) {
-                Preconditions.checkNotNull(other, "other must not be null");
-                throw new UnsupportedOperationException("Compose on reversed iso");
-            }
         };
     }
 

aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/rewrite/Rules.java

@@ -1568,9 +1568,13 @@ public static <T> TypeRewriteRule flattenField(@NotNull final DynamicOps<T> ops,
             Dynamic<T> result = typedDynamic.remove(fieldName);
             final var entries = entriesResult.result().orElse(java.util.stream.Stream.empty()).toList();
             for (final var entry : entries) {
-                final String key = entry.first().asString().result().orElse(null);
+                final Dynamic<?> keyDynamic = entry.first();
+                final Dynamic<T> value = entry.second();
+                if (keyDynamic == null || value == null) {
+                    continue;
+                }
+                final String key = keyDynamic.asString().result().orElse(null);
                 if (key != null) {
-                    final Dynamic<T> value = entry.second();
                     result = result.set(key, value);
                 }
             }

aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/schema/Schema.java

@@ -94,7 +94,8 @@
 public class Schema {
     private final DataVersion version;
     private final Schema parent;
-    private TypeRegistry types;
+    private volatile TypeRegistry types;
+    private TypeRegistry buildingTypes;
 
     /**
      * Creates a new schema for the specified version with the given types.
@@ -161,10 +162,17 @@ public Schema parent() {
      */
     @NotNull
     public TypeRegistry types() {
-        if (this.types == null) {
-            this.types = this.buildTypes();
+        TypeRegistry result = this.types;
+        if (result == null) {
+            synchronized (this) {
+                result = this.types;
+                if (result == null) {
+                    result = this.buildTypes();
+                    this.types = result;
+                }
+            }
         }
-        return this.types;
+        return result;
     }
 
     /**
@@ -178,18 +186,22 @@ public TypeRegistry types() {
     @NotNull
     private TypeRegistry buildTypes() {
         final TypeRegistry registry = this.createTypeRegistry();
-        this.types = registry;
+        this.buildingTypes = registry;
 
         // Inherit types from parent if present
         if (this.parent != null) {
-            // Copy types from parent
-            this.parent.types();
-            // Parent types are already registered in parent's registry
-            // For now, we don't copy - subclass must re-register all types it needs
+            final TypeRegistry parentTypes = this.parent.types();
+            for (final TypeReference ref : parentTypes.references()) {
+                final Type<?> parentType = parentTypes.get(ref);
+                if (parentType != null) {
+                    registry.register(parentType);
+                }
+            }
         }
 
         // Let subclass register types
         this.registerTypes();
+        this.buildingTypes = null;
 
         return registry;
     }
@@ -232,8 +244,9 @@ protected void registerTypes() {
      */
     protected final void registerType(@NotNull final Type<?> type) {
         Preconditions.checkNotNull(type, "type must not be null");
-        Preconditions.checkState(this.types != null, "Cannot register types before types() is called");
-        this.types.register(type);
+        final TypeRegistry registry = this.buildingTypes;
+        Preconditions.checkState(registry != null, "Cannot register types outside of registerTypes()");
+        registry.register(type);
     }
 
     /**
@@ -267,13 +280,14 @@ protected final void registerType(@NotNull final TypeReference reference,
                                       @NotNull final TypeTemplate template) {
         Preconditions.checkNotNull(reference, "reference must not be null");
         Preconditions.checkNotNull(template, "template must not be null");
-        Preconditions.checkState(this.types != null, "Cannot register types before types() is called");
+        final TypeRegistry registry = this.buildingTypes;
+        Preconditions.checkState(registry != null, "Cannot register types outside of registerTypes()");
 
         // Apply the template with an empty family to get the concrete type
         final Type<?> templateType = template.apply(TypeFamily.empty());
 
         // Wrap the template type with the reference
-        this.types.register(new TemplateBasedType<>(reference, templateType));
+        registry.register(new TemplateBasedType<>(reference, templateType));
     }
 
     /**

aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/type/Typed.java

@@ -384,7 +384,8 @@ public <T> DataResult<Typed<A>> updateDynamic(@NotNull final DynamicOps<T> ops,
      * @param ops    the dynamic operations for encoding, must not be {@code null}
      * @param finder the finder that locates the desired sub-value, must not be {@code null}
      * @param <T>    the underlying data format type
-     * @return a {@link DataResult} containing the found dynamic value or {@code null} if not found, never {@code null}
+     * @return a {@link DataResult} containing the found dynamic value on success, or an error result if the path
+     *         was not found; never {@code null}
      * @throws NullPointerException if {@code ops} or {@code finder} is {@code null}
      * @see #updateAt(DynamicOps, Finder, Function)
      */
@@ -393,12 +394,12 @@ public <T> DataResult<Dynamic<T>> getAt(@NotNull final DynamicOps<T> ops,
                                             @NotNull final Finder<?> finder) {
         Preconditions.checkNotNull(ops, "ops must not be null");
         Preconditions.checkNotNull(finder, "finder must not be null");
-        return encode(ops).map(dynamic -> {
+        return encode(ops).flatMap(dynamic -> {
             final Dynamic<?> found = finder.get(dynamic);
             if (found == null) {
-                return null;
+                return DataResult.error("Path not found: " + finder);
             }
-            return found.convert(ops);
+            return DataResult.success(found.convert(ops));
         });
     }
 

aether-datafixers-api/src/test/java/de/splatgames/aether/datafixers/api/optic/FinderTest.java

@@ -32,6 +32,7 @@
 import java.util.Map;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 /**
  * Unit tests for {@link Finder}.
@@ -167,6 +168,14 @@ void idReturnsDescriptiveIdentifier() {
 
             assertThat(finder.id()).isEqualTo("index[3]");
         }
+
+        @Test
+        @DisplayName("index() rejects negative index")
+        void indexRejectsNegativeIndex() {
+            assertThatThrownBy(() -> Finder.index(-1))
+                    .isInstanceOf(IllegalArgumentException.class)
+                    .hasMessageContaining("must not be negative");
+        }
     }
 
     @Nested

aether-datafixers-api/src/test/java/de/splatgames/aether/datafixers/api/optic/IsoTest.java

@@ -196,6 +196,26 @@ void reverseAddsReverseToId() {
             assertThat(unwrapIso.reverse().id()).isEqualTo("wrapper.unwrap.reverse");
         }
 
+        @Test
+        @DisplayName("reverse() supports compose()")
+        void reverseSupportsCompose() {
+            // reversed: String -> Wrapper<String>
+            final Iso<String, String, Wrapper<String>, Wrapper<String>> reversed = unwrapIso.reverse();
+
+            // another iso: Wrapper<String> -> String (extract value and uppercase)
+            final Iso<Wrapper<String>, Wrapper<String>, String, String> extractUpper = Iso.of(
+                    "wrapper.upper",
+                    w -> w.value().toUpperCase(),
+                    s -> new Wrapper<>(s.toLowerCase())
+            );
+
+            // compose: String -> Wrapper<String> -> String (uppercased)
+            final Iso<String, String, String, String> composed = reversed.compose(extractUpper);
+
+            assertThat(composed.to("hello")).isEqualTo("HELLO");
+            assertThat(composed.from("HELLO")).isEqualTo("hello");
+        }
+
         @Test
         @DisplayName("double reverse returns to original")
         void doubleReverseReturnsToOriginal() {