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

"Epic" issue about network node and world representation desyncs #3424

Closed
raoulvdberge opened this issue Dec 5, 2022 · 5 comments
Closed
Labels
bug Something isn't working
Milestone

Comments

@raoulvdberge
Copy link
Collaborator

raoulvdberge commented Dec 5, 2022

A long time ago... I made an architectural decision for RS that has worked good for us for years. Unfortunately, since mods and Minecraft are evolving, this decision is quite problematic now.

This issue is alleviated in RS 2, but needs (temporary) fixing/workaround in RS 1.

Blockstate mismatch

Network(Node) mismatch

Confusing behavior

@raoulvdberge
Copy link
Collaborator Author

Some work for this has been done already on: 885d46c

raoulvdberge added a commit that referenced this issue Dec 10, 2022
Could be in some other places as well but let's see what this does.
raoulvdberge added a commit that referenced this issue Dec 20, 2022
Could be in some other places as well but let's see what this does.
@refinedmods refinedmods deleted a comment from Gnuelch Feb 16, 2023
@refinedmods refinedmods locked and limited conversation to collaborators Feb 16, 2023
@raoulvdberge raoulvdberge added this to the v1.12.4 milestone Nov 4, 2023
@raoulvdberge
Copy link
Collaborator Author

raoulvdberge commented Nov 5, 2023

Note to addon developers

If you are using NetworkNodeBlockEntity

Starting from RS v1.12.4 for MC 1.20.1 (and version v1.11.7 for MC 1.19.2, v1.10.6 for MC 1.18.2) it is strongly advised to pass the network node class type to the constructor.

For example:

    public DetectorBlockEntity(BlockPos pos, BlockState state) {
-       super(RSBlockEntities.DETECTOR.get(), pos, state, SPEC);
+       super(RSBlockEntities.DETECTOR.get(), pos, state, SPEC, DetectorNetworkNode.class);
    }

In order to avoid crashing bugs at runtime when a block entity mismatches with its network node representation, RS needs to know the class type.

Both constructors will keep existing to ensure compatibility, but please move to this new format to avoid crashes (see this bug report for a nice list).

When can I use this API?

Now!

  • For MC 1.20.1: v1.12.4
  • For MC 1.19.2: v1.11.7
  • For MC 1.18.2: v1.10.6

If you are not using NetworkNodeBlockEntity

If you have a custom implementation of INetworkNodeProxy, remember to cast safely when you are retrieving the node from the network node manager.
Like so:

    @Override
    @Nonnull
    @SuppressWarnings("unchecked")
    public N getNode() {
        INetworkNodeManager manager = API.instance().getNetworkNodeManager((ServerLevel) level);

        try {
            INetworkNode node = manager.getNode(worldPosition);

            if (node == null) {
                LOGGER.warn("Expected a node @ {} but couldn't find it, creating a new one...", worldPosition);
                node = createAndSetNode(manager);
            }

-           return (N) node;
+           return networkNodeClass.cast(node);
        } catch (ClassCastException e) {
            LOGGER.warn("Node @ {} got desynced with it's block entity container, recreating", worldPosition, e);
            return (N) createAndSetNode(manager);
        }
    }

@raoulvdberge
Copy link
Collaborator Author

Fixed in 9b30bec

Will be fixed for RS v1.12.4 for MC 1.20.1 and will probably be backported to MC 1.19.2.

@raoulvdberge
Copy link
Collaborator Author

This is now backported to MC 1.19.2 in version v1.11.7.

@raoulvdberge
Copy link
Collaborator Author

This is now backported to MC 1.18.2 in version v1.10.6.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant