Cookie Api - Fabric Networking Api #3547

MrNavaStar · 2024-01-22T19:32:38Z

This Pr adds an api for the new 1.20.5 transfer packets and cookie system. Currently the api can be accessed through any of the network handlers (except handshake).

Transfer api:

public interface ServerTransferable {

    void transferToServer(String host, int port);

    boolean wasTransferred();
}

Cookie Api:

public interface ServerCookieStore {

    void setCookie(Identifier cookieId, byte[] cookie);

    CompletableFuture<byte[]> getCookie(Identifier cookieId);
}

...etworking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerTransferable.java

MrNavaStar · 2024-01-23T05:48:00Z

Hmmm, The injected interfaces don't seem to be working without a cast (testing in an external dev environment). Does any one see anything obviously wrong? (Could also be my dev env lol)

apple502j · 2024-01-23T05:56:51Z

Try run genSource on the networking project.

MrNavaStar · 2024-01-23T06:04:25Z

Yeah doesn't seem to fix it. Issue happens both in the test mod & in a full external environment

...etworking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerTransferable.java

...working-api-v1/src/main/java/net/fabricmc/fabric/mixin/networking/ClientConnectionMixin.java

...networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerCookieStore.java

...i-v1/src/main/java/net/fabricmc/fabric/mixin/networking/ServerCommonNetworkHandlerMixin.java

modmuss50 · 2024-01-23T10:03:03Z

...i-v1/src/testmod/java/net/fabricmc/fabric/test/networking/play/NetworkingPlayPacketTest.java

@@ -117,6 +139,13 @@ public void onInitialize() {
 				}
 			}
 		});
+
+		ServerLoginConnectionEvents.INIT.register((handler, server) -> {


Add the same test for configuration

Maybe we want to ensure that configuration can wait until the cookies have been returned.

Added the test. I'm not sure if we need to? A mod that wants that behaviour would probably just implement that in the rest of its configuration stuff

Login also needs testing.

I think its imporant that login and/or configuation can be bocked until the cookie has been returned.

What would the best way to hold the network stage be?

...i-v1/src/testmod/java/net/fabricmc/fabric/test/networking/play/NetworkingPlayPacketTest.java

modmuss50 · 2024-01-23T10:22:39Z

...working-api-v1/src/main/java/net/fabricmc/fabric/mixin/networking/ClientConnectionMixin.java

+	public void fabric_invokeCookieCallback(Identifier cookieId, byte[] cookie) {
+		CompletableFuture<byte[]> future = pendingCookieRequests.remove(cookieId);
+		if (future == null) return;
+		future.complete(cookie);


Question: What thread does this complete on? I would expect the network thread during config, and the main thread during play

I think it always runs the network thread, as its triggered by the packet response (unless packets process in the main thread in play? I didn't think they did)

fabric-networking-api-v1/src/main/resources/fabric.mod.json

...pi-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerConfigurationNetworking.java

...orking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerLoginNetworking.java

...working-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerPlayNetworking.java

...working-api-v1/src/main/java/net/fabricmc/fabric/mixin/networking/ClientConnectionMixin.java

modmuss50 · 2024-01-24T09:54:39Z

...etworking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/AbstractNetworkAddon.java

@@ -172,4 +178,25 @@ public final void handleDisconnect() {
 	 * @return whether the channel is reserved
 	 */
 	protected abstract boolean isReservedChannel(Identifier channelName);
+
+	public boolean triggerCookieFuture(Identifier cookieId, byte[] cookie) {


This looks like a prime target for some unit tests :)

how would you prime the underlying map with data so you can test this function?

...i-v1/src/testmod/java/net/fabricmc/fabric/test/networking/play/NetworkingPlayPacketTest.java

modmuss50 · 2024-01-24T10:44:11Z

...i-v1/src/testmod/java/net/fabricmc/fabric/test/networking/play/NetworkingPlayPacketTest.java

@@ -117,6 +139,13 @@ public void onInitialize() {
 				}
 			}
 		});
+
+		ServerLoginConnectionEvents.INIT.register((handler, server) -> {


Login also needs testing.

I think its imporant that login and/or configuation can be bocked until the cookie has been returned.

fabric-networking-api-v1/src/main/resources/fabric-networking-api-v1.accesswidener

...n/java/net/fabricmc/fabric/mixin/networking/accessor/ServerCommonNetworkHandlerAccessor.java

Co-authored-by: modmuss <modmuss50@gmail.com>

MrNavaStar · 2024-01-24T16:17:40Z

@modmuss50 Just wanted to put it out there, would a pre transfer event be useful? In case mods want to set cookies when another mod triggers a transfer? This may also work nicely with the new /transfer command

...pi-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerConfigurationNetworking.java

...etworking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/AbstractNetworkAddon.java

...c-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/ServerCookieStore.java

...i-v1/src/main/java/net/fabricmc/fabric/mixin/networking/ServerCommonNetworkHandlerMixin.java

...pi-v1/src/main/java/net/fabricmc/fabric/mixin/networking/ServerLoginNetworkHandlerMixin.java

MrNavaStar · 2024-04-10T14:54:44Z

Hey, sorry for being so slow to get back on this. Once my exams are done this week I should hopefully be able to finish this up. Thanks for your help!

Co-authored-by: modmuss <modmuss50@gmail.com>

…20.5

MrNavaStar added 5 commits January 22, 2024 12:28

initial work on transfer/cookie api

c602d62

remove injection on ServerPlayerEntity

3971da1

use built in way of storing 'isTransferred'

5a6b9dc

test cases - everything seems to be working ;)

4ff8ad1

fix a bug when multiple mods request the same cookie + api comments

24c3d4c

Shnupbups reviewed Jan 23, 2024

View reviewed changes

...etworking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerTransferable.java Outdated Show resolved Hide resolved

MrNavaStar added 2 commits January 22, 2024 18:48

typo

26f98e4

checkstyle

767891b

modmuss50 requested changes Jan 23, 2024

View reviewed changes

MrNavaStar added 7 commits January 23, 2024 16:16

fix race condition

14e7986

Remove ServerTransferable interface

7fd0ca2

use handler addon instead of client connection

7185ca3

Disconnect the client if they send an un-requested cookie

08cbc97

fix test mod

19a9c10

oops fix remove old code

3a6b9b1

checkstyle

d9f97fb

MrNavaStar requested a review from modmuss50 January 24, 2024 03:58

modmuss50 requested changes Jan 24, 2024

View reviewed changes

MrNavaStar and others added 4 commits January 24, 2024 06:55

Update fabric-networking-api-v1.accesswidener

9d223a9

Co-authored-by: modmuss <modmuss50@gmail.com>

silly fixes

5b793d4

Remove setCookie from network handler addon

b451875

Add constant for test cookie

14bfe7e

MrNavaStar requested a review from modmuss50 January 24, 2024 15:11

Merge branch 'FabricMC:1.20.5' into 1.20.5

edbbd81

modmuss50 requested changes Feb 18, 2024

View reviewed changes

LlamaLad7 reviewed Feb 19, 2024

View reviewed changes

...pi-v1/src/main/java/net/fabricmc/fabric/mixin/networking/ServerLoginNetworkHandlerMixin.java Outdated Show resolved Hide resolved

MrNavaStar and others added 20 commits April 16, 2024 00:04

initial work on transfer/cookie api

a84fa62

remove injection on ServerPlayerEntity

248f1f9

use built in way of storing 'isTransferred'

b1c5a90

test cases - everything seems to be working ;)

9f10a98

fix a bug when multiple mods request the same cookie + api comments

d3fe017

typo

ffde9ba

checkstyle

81fcdd8

fix race condition

d56e2c2

Remove ServerTransferable interface

92850cf

use handler addon instead of client connection

c0c3f67

Disconnect the client if they send an un-requested cookie

04cad10

fix test mod

613aaca

oops fix remove old code

b2fa72e

checkstyle

9f3cc48

Update fabric-networking-api-v1.accesswidener

0d01354

Co-authored-by: modmuss <modmuss50@gmail.com>

silly fixes

f85bdd6

Remove setCookie from network handler addon

4f0c9c3

Merge branch '1.20.5' of https://github.com/MrNavaStar/fabric into 1.…

a9f0452

…20.5

refactor

e95b2e0

Move getCookie to AbstractChanneledNetworkAddon

8f97b4c

MrNavaStar changed the title ~~Transfer / Cookie Api - Fabric Networking Api~~ Cookie Api - Fabric Networking Api Apr 16, 2024

MrNavaStar and others added 3 commits April 16, 2024 02:35

checkstlye + compile error

2c2c9b4

Merge branch 'FabricMC:1.20.5' into 1.20.5

b9ea0c7

not sure if this works

0574e03

MrNavaStar requested a review from modmuss50 April 17, 2024 01:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cookie Api - Fabric Networking Api #3547

Cookie Api - Fabric Networking Api #3547

MrNavaStar commented Jan 22, 2024 •

edited

MrNavaStar commented Jan 23, 2024 •

edited

apple502j commented Jan 23, 2024

MrNavaStar commented Jan 23, 2024

modmuss50 Jan 23, 2024

modmuss50 Jan 23, 2024

MrNavaStar Jan 24, 2024

modmuss50 Jan 24, 2024

MrNavaStar Jan 24, 2024

modmuss50 Jan 23, 2024

MrNavaStar Jan 24, 2024

modmuss50 Jan 24, 2024

MrNavaStar Jan 24, 2024

modmuss50 Jan 24, 2024

MrNavaStar commented Jan 24, 2024

MrNavaStar commented Apr 10, 2024

Cookie Api - Fabric Networking Api #3547

Are you sure you want to change the base?

Cookie Api - Fabric Networking Api #3547

Conversation

MrNavaStar commented Jan 22, 2024 • edited

MrNavaStar commented Jan 23, 2024 • edited

apple502j commented Jan 23, 2024

MrNavaStar commented Jan 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MrNavaStar commented Jan 24, 2024

MrNavaStar commented Apr 10, 2024

MrNavaStar commented Jan 22, 2024 •

edited

MrNavaStar commented Jan 23, 2024 •

edited