PhaseTracker 1.14 Upgrade #2476

gabizou · 2020-01-12T09:38:24Z

This leaves a lot to be desired currently as it's a WIP.

What's the update?

The PhaseTracker is essentially an adaptation of pairing server-focused changes occurring in a World to an Event factory with specific Cause population based on the processing of a IPhaseState. If that's enough to make you ask "huh??" then it might be recommended to read a little bit about Events and their causes. The PhaseTracker is a wedged implementation of logging BlockState, BlockEntity, Entity, and ItemStack changes within a World during specific operations, such as:

An Entity performing work during a Game tick
A Player interacting with a block
A Block "growing" into a different Block
etc.

Because of how intrinsically "wired" the system is to the engine, it needs hooks in various areas of the Game logic to say "hey, we're gonna be ticking an Entity here!" Currently, for stable-7 or Minecraft 1.12, it's doing this mostly well (sans some incompatibility with some mods and their design, no fault of theirs, just differences in how the system needs to be improved), but as it's shown, it's still very much tied to the game's core logic.

Ok, so update the hooks then?

Of course! How could I be so blind! Just update the tens of thousands of lines that make the system... Ok, it's not really tens of thousands, it's closer to maybe 20k, but short of the hooks, the actual logic processes need to be reviewed and re-analyzed with caution since 1.14's block logic has changed enough that some parts of the system are needing to be re-engineered.

Core concepts of the Update

PhaseTracker per Thread

With the PhaseTracker being so integral to many systems in the game logic, and to support client sided effects and events, along with asynchronous world generation events, the PhaseTracker is redesigned to have two primary instances (CLIENT and SERVER) while the purposed getInstance() will return a Thread appropriate variant. Because of this, we'll be able to appropriately associate individual PhaseTrackers to pooled PhaseContexts via PooledPhaseState. What this means is that the pools of recycled contexts will be per-thread, and furthermore be sanity checked to the PhaseTracker instance that created them. Uniquely, this will sanity check closures of PhaseContexts to avoid asynchronous closures without knowing which thread originally entered said state.

Splitting the `PhaseTracker` associated code from all Mixins into a separate package

This is mostly an organizational effort to verify "who does what and why". When looking at WorldServerMixin, it's difficult to read as to what this method is doing and why, except that it's interacting with a "proxy" for some reason. This, on top of several scattered areas where we have PhaseState entries, it becomes difficult to see what sections of code are to support the PhaseTracker, using the PhaseTracker for context population, or simply using the PhaseTracker for throwing events. Lastly, because testing of Sponge code without the PhaseTracker involvement and possible bugs/side effects, there's no "switch" to flip, except by removing the main hooks and hoping nothing else breaks in the process of those missing hooks.

Likewise, because the injections/redirects/etc. would now be in a separate Mixin bundle, we can too prefix those with tracker$ to further identify what methods are coming from where.

So here's a laundry list of things I need to have done for the update:

Update BlockTransaction to keep track of neighbor "shape updates" being requested, etc.
Update SpongeBlockProxy to be a simplified IWorld proxy so fewer mixins have to exist
This is up for debate, and only possible in 1.13+ because of the introduction of the super interface. Since we can implement IWorld, the contracts for various block methods only actually require an IWorld instance, allowing us to better track changes and TileEntity additions.
Review whether there's enough "hooks" in some mixins to split out into another mixing package/plugin
This would theoretically allow us to entirely disable the PhaseTracker without disabling events being thrown
We'd be able to improve visibility of what mixins/injections are for what purpose, when some are stubs or proxy-related injections.
Review BlockChangeFlag API exposure, we have a bit we can expand on for better plugin control

Signed-off-by: Gabriel Harris-Rouquette <gabizou@me.com>

…umenting the block physics process to re-re-re-re-re-rewrite the core of logging these changes. Signed-off-by: Gabriel Harris-Rouquette <gabizou@me.com>

…tarted WorldMixin and ServerWorldMixin. Signed-off-by: Gabriel Harris-Rouquette <gabizou@me.com>

src/main/java/org/spongepowered/common/event/tracking/PooledPhaseState.java

gabizou · 2020-01-12T09:43:47Z

src/main/java/org/spongepowered/common/registry/builtin/supplier/EventContextKeySupplier.java

+@SuppressWarnings("UnstableApiUsage")
+public class EventContextKeySupplier {
+
+    public static Stream<EventContextKey<?>> stream() {


@Zidane if you can, please tell me this is the correct way of doing it for Sponge provided types?

This would go into the Stream package and be named like the other stream classes are.

src/main/java/org/spongepowered/common/event/tracking/PooledPhaseState.java

Zidane · 2020-01-12T16:02:28Z

src/main/java/org/spongepowered/common/data/type/SpongeSpawnType.java

-    public SpongeSpawnType(CatalogKey key) {
+    private boolean isForced = false;
+
+    public SpongeSpawnType(CatalogKey key, String name) {


Delete name parameter unless it is a NamedCatalogType.

Zidane · 2020-01-12T16:02:40Z

src/main/java/org/spongepowered/common/event/SpongeEventContextKey.java

    }

-    public SpongeEventContextKey(String id, String name, Class<T> allowed) {
+    public SpongeEventContextKey(CatalogKey id, TypeToken<T> allowed) {


Zidane · 2020-01-12T16:03:43Z

src/main/java/org/spongepowered/common/event/tracking/BlockChangeFlagManager.java

+import java.util.Map;
+import java.util.StringJoiner;
+
+public final class BlockChangeFlagManager {


These are not catalogs, correct?

Zidane · 2020-01-12T16:06:35Z