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.

Sign up for GitHub

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

Add serializer to EntitySnapshot #7509

Open
wants to merge 1 commit into
base: dev/feature
Choose a base branch
from
Open

Add serializer to EntitySnapshot #7509

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 34 additions & 1 deletion src/main/java/ch/njol/skript/classes/data/BukkitClasses.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import ch.njol.skript.util.BlockUtils;
import ch.njol.skript.util.PotionEffectUtils;
import ch.njol.skript.util.StringMode;
import ch.njol.util.StringUtils;
import ch.njol.yggdrasil.Fields;
import io.papermc.paper.world.MoonPhase;
import org.bukkit.*;
Expand Down Expand Up @@ -60,6 +59,7 @@
import org.bukkit.util.Vector;
import org.jetbrains.annotations.Nullable;

import java.io.NotSerializableException;
import java.io.StreamCorruptedException;
import java.util.*;
import java.util.Map.Entry;
Expand Down Expand Up @@ -1493,6 +1493,39 @@ public String toVariableNameString(EntitySnapshot snapshot) {
return toString(snapshot, 0);
}
})
.serializer(new Serializer<EntitySnapshot>() {

@Override
public Fields serialize(EntitySnapshot snapshot) throws NotSerializableException {
Fields fields = new Fields();
fields.putPrimitive("snapshot", snapshot.getAsString());
return fields;
}

@Override
public void deserialize(EntitySnapshot o, Fields f)
throws StreamCorruptedException, NotSerializableException {
assert false; // canBeInstantiated is false
}

@Override
public EntitySnapshot deserialize(Fields fields) throws StreamCorruptedException, NotSerializableException {
EntityFactory factory = Bukkit.getEntityFactory();
String input = fields.getPrimitive("snapshot", String.class);
return factory.createEntitySnapshot(input);
Copy link
Contributor

Choose a reason for hiding this comment

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

My ONLY worry here is that the server (CraftBukkit/Paper/Whatever), doesn't handle data changes for entity snapshots.
Mojang lately has been changing NBT fields on entities quite a bit.
A user could save their snapshot in MC 1.21.4, and then load it in a future version (let's say 1.21.5), and any changed tags will get ignored.... the returned entity snapshot would have different values than the user originally had "saved", and they may have no clue what is going on.

Exhibit A:
(from a 1.21.5 snapshot)
Screenshot 2025-01-22 at 2 33 21 PM

In this instance, if you saved a Zombie with some cool armor (in 1.21.4), he's going to load (in 1.21.5) as a boring old, unprotected Zombie.

Copy link
Member

Choose a reason for hiding this comment

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

To add onto this. As per the javadocs, it states that the NBT string provided by getAsString should NOT be relied be upon for serialization.
image

Copy link
Contributor

Choose a reason for hiding this comment

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

My assumption would probably be because exactly this, Bukkit/CB/Paper doesn't run this thru DataFixerUpper (maybe it should).

}

@Override
public boolean mustSyncDeserialization() {
return false;
}

@Override
protected boolean canBeInstantiated() {
return false;
}

})
);
}

Expand Down
Loading