Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1.20.2] Endec - Simplier Codec Alternative #184

Closed
wants to merge 31 commits into from

Conversation

Dragon-Seeker
Copy link
Member

@Dragon-Seeker Dragon-Seeker commented Oct 10, 2023

Endec is an implementation based on a previous attempt and Codec by @gliscowo. Such is an attempt at being a similar but different alternative Minecraft DFU system Codec.

Features:

  • Support for Networking by default (ByteBuf implementations like PacketByteBuf)
  • Less Functional Programming
  • Interop with Minecraft Codec

Such still requires refinement as such still lacks some of the various DFU implementations like MapCodec and possibly changes to general implementations.

@BasiqueEvangelist
Copy link
Contributor

code eck

@Dragon-Seeker Dragon-Seeker changed the title [1.20.2] Implement Simplier Codeck Alternative [1.20.2] Implement Simplier Codec Alternative Oct 10, 2023
- Adjust "any" methods to better reflect where they are located
- Fix error within writeAny for json and nbt
- Implement readAny NBT
- Add AW for forwarding Dyn Ops
- Fix errors within testing command
@AlphaMode
Copy link

Bode ck

Copy link
Contributor

@BasiqueEvangelist BasiqueEvangelist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

æ

Comment on lines 192 to 205
//Kinda mega cursed but...
static <e, T> Codeck<T> of(BiConsumer<Serializer<e>, T> encode, Function<Deserializer<e>, T> decode) {
return new Codeck<T>() {
@Override
public <E> void encode(Serializer<E> serializer, T value) {
encode.accept((Serializer<e>) serializer, value);
}

@Override
public <E> T decode(Deserializer<E> deserializer) {
return decode.apply((Deserializer<e>) deserializer);
}
};
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be a bit cleaner if encode and decode were bespoke interfaces that had the proper generics in them

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really would not want to split such into separate interfaces

src/main/java/io/wispforest/owo/serialization/Codeck.java Outdated Show resolved Hide resolved
return new MapCodeck<>(codeck, s -> s, s -> s);
}

public <R> MapCodeck<R, V> keyThen(Function<K, R> toFunc, Function<R, K> fromFunc) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this method in the Public API? IntelliJ might complain about using it outside of owo if the impl package is made internal

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible, such is still up in the air about how the folder structure should be done.


protected final Deque<Supplier<NbtElement>> stack = new ArrayDeque<>();

private boolean unsafe = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't really make it unsafe, though. All it does is make it lax on format errors

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would call such unsafe but I am up for a better name

src/main/java/io/wispforest/owo/serialization/Codeck.java Outdated Show resolved Hide resolved
src/main/java/io/wispforest/owo/serialization/Codeck.java Outdated Show resolved Hide resolved
src/main/java/io/wispforest/owo/serialization/Codeck.java Outdated Show resolved Hide resolved
…lization using Attributes

- Add Variable Length Ints
- Adjust networking test
- Fix improper encode and decode for String
- Use VAR_INT codeck for enum
src/main/java/io/wispforest/owo/serialization/Codeck.java Outdated Show resolved Hide resolved
StructField.of("z", StructCodeck.INT, BlockPos::getX),
BlockPos::new
),
SerializationAttribute.HUMAN_READABLE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this result in hard to edit NBT?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, I have no clue how such would be represented, to say the least. It was mostly just a guess

Comment on lines 3 to 5
/**
* Interface that indicates that the object has an end operation to such
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO it would be nicer to just make an interface that extends AutoCloseable and just makes close not throw anything, kind of like InfallibleCloseable in gadget.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly a good idea as such exists just as a way to prevent issues with calling close method and end is just used due to what glisco used within her version.


@Override
default <V> void writeOptional(Codeck<V> codeck, Optional<V> optional) {
optional.ifPresentOrElse(v -> codeck.encode(this, v), this::empty);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't NbtEnds disappear after serialization, making deserialization use the default field value instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unknown, keeping such until fully confirm that it is not required as I don't want to just discard such just yet

}

return error
? DataResult.error(exception != null ? exception::getMessage : () -> "The corresponding Format for the given DynamicOps could not be located!")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't really be a problem, as if you get a funky DynamicOps you can always just NbtOps or JsonOps and then convert everything to the needed dynamic ops

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True but I rather have this error instead to properly map such due to how conversion between formats can cause a lose of information potentially

SELF_DESCRIBING(),
BINARY(),
HUMAN_READABLE(),
COMPRESSED();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As in compressed with gzip/whatever? Why would a Codeck want to care about this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As in the the data should be as small as possible but still hold onto some form of structure as in not intended to be human readable but still can be read in a sense based on order or something

SerializationAttribute.HUMAN_READABLE)
.orElse(Codeck.LONG.then(ChunkPos::new, ChunkPos::toLong));

Codeck<BitSet> BITSET = Codeck.LONG_ARRAY.then(BitSet::valueOf, BitSet::toLongArray);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

won't this lose the precise length of the bitset?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unknown but such was copied from PacketBufSerilizer which calls writeBitSet and readBitSet using a long array within PacketByteBuf

Dragon-Seeker and others added 2 commits October 23, 2023 16:44
@Dragon-Seeker Dragon-Seeker changed the title [1.20.2] Implement Simplier Codec Alternative [1.20.2] Endec - Simplier Codec Alternative Oct 24, 2023
Copy link
Contributor

@BasiqueEvangelist BasiqueEvangelist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ant & Dec

Comment on lines 41 to 42
private static final Endec<Map<Identifier, Integer>> RESPONSE_SERIALIZER =
(Endec<Map<Identifier, Integer>>) (Object) ReflectionEndecBuilder.createMapSerializer(Map.class, Identifier.class, Integer.class);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be updated to use a proper MapEndec

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly


<E> T decode(Deserializer<E> deserializer);

default <E> E encode(Supplier<Serializer<E>> serializerCreator, T value){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does this take a supplier if it just invokes it once anyways

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is an easy way of constructing a supplier of the desired serializer type and returning the value held by such serializer as the return.

Comment on lines 9 to 11
boolean hasNext();

E next();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't Iterator have these methods anyways? 🤨

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did such as a note to what is being implemented exactly and needs to be implemented


public class StructField<S, F> {

public MutableObject<F> defaultValue = null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this a MutableObject?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Such acts as a way of telling if a given value is defaulted. Such allows for the default value of null as setting the mutable object to be a instance of what ever defaulted value wanted.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What
You mean like it can either be null (required), or non-null, containing the actual default value?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, with the default value allowing for either a value of the given type or null

Dragon-Seeker and others added 4 commits November 7, 2023 17:23
- Add KeyField
- Adjust StructField to be a extenstion of KeyField
- Add various helper method ontop of KeyField and StructField
- Adjust Nbt and Json Serializers for Prefix support
 - remove compressed and binary attributes as they both serve little purpose
 - update SelfDescribedDeserializer#readAny to accept a Serializer as a visitor to construct data instead of converting to intermediate types
 - reimplement JsonEndec and NbtEndec on top of updated readAny(...) specification
 - experimental implementation for Endec.ofCodec
Comment on lines 57 to 79
var value = primitive.getAsBigDecimal();

try {
var asLong = value.longValueExact();

if ((byte) asLong == asLong) {
visitor.writeByte(element.getAsByte());
} else if ((short) asLong == asLong) {
visitor.writeShort(element.getAsShort());
} else if ((int) asLong == asLong) {
visitor.writeInt(element.getAsInt());
} else {
visitor.writeLong(asLong);
}
} catch (ArithmeticException bruh /* quite cringe java moment, why use an exception for this */) {
var asDouble = value.doubleValue();

if ((float) asDouble == asDouble) {
visitor.writeFloat(element.getAsFloat());
} else {
visitor.writeDouble(asDouble);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couldn't you just use .getAsNumber and then check the type of the returned value?

…-serializer method ordering, remove binary nbt format
Dragon-Seeker and others added 12 commits November 18, 2023 18:24
This is for the few that have been left behind with Mojang's implementation of Codec and lack the ability to handle any of their 17 field objects.
Also add support for nested generic calls with Map, List and Set on the getGeneric method
 - separate non-primitive endecs into BuiltInEndecs
 - use wildcard generics on (De)Serializer parameters
 - implement Endec.dispatchedStruct for DFU parity
…e (always require a default value), refactor StructField to be independent of now-different KeyedEndec
@Dragon-Seeker Dragon-Seeker deleted the 1.20.2-Codeck branch November 21, 2023 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants