Impl ServerTickEvent and RenderTickEvent

[GammaWhiskey]

Contained herein are my efforts to eliminate every TODO from the TickEvent class! A few things of note here:

• Fabric API's START_SERVER_TICK event, which is what I've used to implement ServerTickEvent (trying not to reinvent the wheel here), fires slightly later than the START phase of Forge's ServerTickEvent. Here's what I mean:

    protected void tick(BooleanSupplier booleanSupplier) {
        long l = Util.getMeasuringTimeNano();
        // Forge fires here
        ++this.ticks;
        // Fabric API fires here
        this.tickWorlds(booleanSupplier);
        ...

If this is problematic, I could create a custom inject to match Forge. (This issue does not exist with Fabric API's END_SERVER_TICK, which I've also used, as both Forge and Fabric API fire their respective events at the very end of the method.)

• As Fabric API has no render tick events, I wrote mixins to cover that territory. This about sums up the results of that endeavor:

      ...
      if (!this.skipGameRender) {
          // RenderTickEvent START phase fires here with both stock Forge and the mixin
          this.profiler.swap("gameRenderer");
          this.gameRenderer.render(this.paused ? this.pausedTickDelta : this.renderTickCounter.tickDelta, l, tick);
          this.profiler.swap("toasts");
          this.toastManager.draw();
          // RenderTickEvent END phase fires here with the mixin
          this.profiler.pop();
          // RenderTickEvent END phase fires here with stock Forge
      }
      // Could potentially mixin another (!this.skipGameRender) conditional here as a workaround
      this.profiler.endTick();
      ...

  Frankly, I'm not quite sure how to mixin where Forge patches it in. However, if the event firing before this.profiler.pop() is problematic, then the workaround specified above would theoretically accomplish the same thing - albeit in a way that isn't as clean. (How important is the profiler in relation to this event?)

[kitlith]

Ehhh. I wouldn't be too torn up if the profiler results were slightly thrown off.

However, you can inject to after a method call (and any variable assignment with the return value) with INVOKE_ASSIGN. Does that help?

[GammaWhiskey]

Ehhh. I wouldn't be too torn up if the profiler results were slightly thrown off.

However, you can inject to after a method call (and any variable assignment with the return value) with INVOKE_ASSIGN. Does that help?

I was under the impression that INVOKE_ASSIGN could only be used if the method returned a value, and DisableableProfiler.pop() returns void.

[kitlith]

https://jenkins.liteloader.com/view/Other/job/Mixin/javadoc/index.html?org/spongepowered/asm/mixin/injection/Inject.html says that it merely has special handling for invokes that return and immediately assign, not that it can only be used in that case.

Even if it couldn't be used, you could use 'shift' to shift the injection point to after the method call.

EDIT: apparently i copied the wrong link, I need to fix that.

[GammaWhiskey]

Huh. That seems to contradict the Mixin wiki:

Like BeforeInvoke, this injection point searches for invoke instructions within the target instruction list and returns instructions which match the criteria. However the method targetted must return a value.

Plus, whenever I set it to use INVOKE_ASSIGN, it fails. Either way, I can just shift the injection point, so I'll do that!

[kitlith]

Huh, I didn't see that. Fair enough!

[rikka0w0]

if ++this.ticks; is not public, then implementing Forge's ServerTickEvent with Fabric API's START_SERVER_TICK should be fine. I don't think there are any mod uses tick as a timestamp or care about its state.

[TheGlitch76]

At worst it could cause an off-by-one error, but having that cause a genuine issue is very unlikely.

[TheGlitch76]

Thank you for the in-depth PR description by the way, it was very helpful!

[GammaWhiskey]

Thank you for the in-depth PR description by the way, it was very helpful!

I always aspire to be helpful. 👈😉👈