API8 SpV: The Dedicated Server is Not Available error-spam loading datapack

[theboomer]

API-8 Win7, Java 8_281, spongevanilla-universal jar built with tyhdefu fix commit https://github.com/SpongePowered/Sponge/commit/8c84a888258402e4d541bea25b1675f9694c40ab march 9 (Previous many days of builds prevented making a new server, and slowed producing a reproducible way to generate error to post here)

Problem: The spongevanilla 1.16.5 server is unable to load files from my datapacks, due to the dedicated server not available. Conversation with gabizou in discord #plugins channel march 7 PST/march 8 EST

To reproduce, and simplify one of my datapacks to a few files only, I stripped my mob-death-rewards datapack down to include just two mob kill triggers and execution, and the appropriate load/tick based files. The datapack is here to download: www.prennet.com/mobrewards.zip

To reproduce the error: Create a brand new server with universal jar file /bat file, and run/fix eula/rerun etc to get a working baseline spongevanilla server, and stop. Then move the mobrewards datapack folder/file structures from the zip file into the world datapack folder, and then run.

Errors generated will be spewn for each file like so

[tyhdefu]

I can reproduce this. Appears to be an issue with commands being registered too early (before the server is available). Here is a full log

[zml2008]

This will be a bit of a fun balancing act for us. With 1.16, Mojang loads data packs before a server instance even exists, and does all command parsing at that point. To accommodate this, we have to load commands before the server is available.

We'll need to figure out a few things to make this play nicely with our commands system:

1. Do we want to ignore permissions checks altogether for datapack loads? Or shift permissions checks to execution time? To make this work, we would have to represent function execution in command context somehow -- either as a context on an existing subject, or as a subject of its own.
2. Are there issues with the server holding on to the CommandContext from parse time onwards? We should make sure any information that could become stale is not held. I think trying to copy Brigadier's parse-time vs execution-time split will cause more problem than it solves. Instead, we'll have to make sure that any Sponge-specific parsing is only performed at execution time (though I think this is already mostly the case).

[gabizou]

1. Do we want to ignore permissions checks altogether for datapack loads?

We can wrap data pack loads with a PhaseState and check there (Intermediary PermissionService by way of PhaseTracker being the bootstrapped permission service?)

[dualspiral]

We can wrap data pack loads with a PhaseState and check there (Intermediary PermissionService by way of PhaseTracker being the bootstrapped permission service?)

No, because the permission service is server scoped so there'd be nothing to bootstrap on to.

[theboomer]

Its been a while before I could test this out, but the recent builds (spongevanilla-1.17-9.0.0-RC659-universal) still throw these errors.
Is the temp fix not incorporated into the server project , or has it rebroken since?

[Faithcaio]

I think the issue is coming from CommandSourceStackMixin_API then. There is no Subject in the cause and it then picks the system subject instead of forwarding CommandSource.NULL from ServerFunctionLibrary

[dualspiral]

I think the issue is coming from CommandSourceStackMixin_API then.

Uhhh, no? It's doing everything it's meant to be doing. ServerFunctionLibrary perhaps should be adding the null source to the cause, which itself may need some mixing into. Even there, there are other considerations - and it's really a pain because of how early it wants to parse the nodes...

There are a lot of nuances to this issue, of which that is only one part. It's not a quick fix. If it was, it would have been done already.