Skip to content

Expose APIs for manual serialization/deserialization#80

Closed
Unoqwy wants to merge 2 commits intoInfernalSuite:mainfrom
Unoqwy:feature/manual_serialization
Closed

Expose APIs for manual serialization/deserialization#80
Unoqwy wants to merge 2 commits intoInfernalSuite:mainfrom
Unoqwy:feature/manual_serialization

Conversation

@Unoqwy
Copy link

@Unoqwy Unoqwy commented Oct 18, 2023

  • Adds a new interface ActiveSlimeWorld that represents a live world, from which a snapshot can be taken at any time
  • Make loadWorld return an ActiveSlimeWorld to easily get the bound bukkit world
  • Adds a SlimeFormatAdapter interface for manual (de)serialization of slime worlds. The plugin provides a default implementation that calls the existing, unchanged, internal slime serializer.

@ComputerNerd100
Copy link
Member

I'm gonna keep this PR open for now and review it a bit more in depth later this week, however at first glance I don't think this is something we are likely to merge in soon as the functionality added here is something we're targeting in IWM and may be out of scope for ASPaper at the moment, but I'll dig into it more and have a think

Copy link
Member

@davidmayr davidmayr left a comment

Choose a reason for hiding this comment

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

In general, I'm not against the idea of exposing this API as users can already serialize to bytes like this by building a hacky custom slime loader with a callback or smth, so why not implement it properly with a nicer api.

It would require some changes and updates to the newest ASP version on the "develop" branch (please change the merge target to develop as well). If you are no longer interested in working on this since it has been a while since this was opened, please let me know.

* @return active slime world instance, or {@code null} if the provided world is not backed by a slime world
*/
@Nullable
ActiveSlimeWorld getActiveWorld(@NotNull World bukkitWorld);
Copy link
Member

Choose a reason for hiding this comment

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

Not necessary with the new SlimeWorldInstance API

* @param propertyMap A {@link SlimePropertyMap} with the world properties.
* @return A {@link SlimeWorld} that is empty, doesn't have a loader and is unregistered
*/
SlimeWorld createUnboundEmptyWorld(@NotNull String worldName, @NotNull SlimePropertyMap propertyMap);
Copy link
Member

Choose a reason for hiding this comment

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

Not necessary as the loader can be null in the normal createEmptyWorld method

/**
* A "running" slime world, bound to a loaded ("live") Minecraft world.
*/
public interface ActiveSlimeWorld extends SlimeWorld {
Copy link
Member

Choose a reason for hiding this comment

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

Not necessary with the new SlimeWorldInstance API

import java.util.function.Consumer;

public class DefaultSlimeFormatAdapter implements SlimeFormatAdapter {
private final @Nullable Consumer<String> infoLog;
Copy link
Member

Choose a reason for hiding this comment

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

why not just pass on the logger itself?

* @return Snapshot of the world, ready for serialization.
*/
@NotNull
SlimeWorld getSnapshot();
Copy link
Member

@davidmayr davidmayr Mar 21, 2025

Choose a reason for hiding this comment

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

cant the clone method be used here?

@CarmJos
Copy link

CarmJos commented Apr 3, 2025

I'm now developing a system for minigames & creative areas for other developers & players, and petty requiring interfaces like this.

(Currently I'm using jar dependency to using serialization & deserializations, which is not elegant at all.)

So, looking forward to see this pr to be merged.

@davidmayr
Copy link
Member

I'm now developing a system for minigames & creative areas for other developers & players, and petty requiring interfaces like this.

(Currently I'm using jar dependency to using serialization & deserializations, which is not elegant at all.)

So, looking forward to see this pr to be merged.

I think this PR is dead. I will try to implement this for the 1.21.5 release if I find the time to do so. If you want, you can also re-do this PR on the current "develop" branch with the changes I suggested, and we may be able to get this done faster

@CarmJos
Copy link

CarmJos commented Apr 3, 2025

I'm now developing a system for minigames & creative areas for other developers & players, and petty requiring interfaces like this.
(Currently I'm using jar dependency to using serialization & deserializations, which is not elegant at all.)
So, looking forward to see this pr to be merged.

I think this PR is dead. I will try to implement this for the 1.21.5 release if I find the time to do so. If you want, you can also re-do this PR on the current "develop" branch with the changes I suggested, and we may be able to get this done faster

Yeah I noticed that this pr first created in 2023, 2 yeasrs ago..

If so, I gonna redesign those new interfaces, and create a new pr for this in those days.

@Unoqwy
Copy link
Author

Unoqwy commented Apr 10, 2025

I think this PR is dead

Yeah I haven't used this in a couple years and forgot all about it. I'll leave the PR open so you can close it when you have a new one to replace it

davidmayr added a commit that referenced this pull request Apr 20, 2025
…#80

Signed-off-by: David Mayr <davidliebtkekse@gmail.com>
@davidmayr
Copy link
Member

This was restructured and implemented into API 4.0. Thanks for your contribution.

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