runAtEntityLater method broken on Paper only

[PikaMug]

Thank you for adding c720ba1471f9f60946ce7689d496a229df56b1e8 it works great on Folia!

However, it seems to have broken on Paper, with or without a fallback parameter. FoliaLib 0.3.3 executes the consumer, whereas 0.3.4 simply does not. Furthermore, if a fallback runnable is specified, that gets triggered instead (tested with ProjectileLaunchEvent where the fallback is triggered immediately on launching a Fireball via Player#launchProjectile).

Edit: It appears the Projectile#isValid check returns false on Paper? Perhaps #isDead would be preferable?

Java 17 with Paper version git-Paper-370 (MC: 1.20.4) (Implementing API version 1.20.4-R0.1-SNAPSHOT) (Git: 1fa48d1)

[Browsit]

We get a repeating StackOverflowError when spawning a custom mob which may be caused by this (did not occur on 0.3.3):

[17:35:53 ERROR]: Could not pass event MythicMobSpawnEvent to TestPlugin v1.0
java.lang.StackOverflowError: null
        at org.bukkit.craftbukkit.v1_20_R3.entity.CraftEntity.isValid(CraftEntity.java:389) ~[paper-1.20.4.jar:git-Paper-477]
        at org.browsit.testplugin.libs.folialib.impl.SpigotImplementation.runAtEntityLater(SpigotImplementation.java:236) ~[TestPlugin-1.0-SNAPSHOT.jar:?]

[TechnicallyCoded]

I'm currently away for vacation, sorry for the slow replies. I'll do my best to address this asap. If anyone wants to PR a fix in the meantime, feel free :)

[CJCrafter]

I found this issue when doing my PR for task chains... Please upvote #16 for a fix :+1:

[TechnicallyCoded]

I would never forget about massive issues... :o If the PR is accepted it will be fixed anyway, so I will just leave this open for the time being

[Loving11ish]

@TechnicallyCoded This is still an issue on 0.3.4 too!

[PikaMug]

@TechnicallyCoded This is still an issue on 0.3.4 too!

Already known. You can use the dev branch until 0.4.0 is out.

[Loving11ish]

@PikaMug I've not had to use a dev branch before using maven, how do I change to it please?

[Loving11ish]

@PikaMug I've not had to use a dev branch before using maven, how do I change to it please?

nvm, I figured it out anyway thanks me be a derp lol

[Loving11ish]

Ah dang it, still broken in the dev 0.4.0 version :/

Paper: 1.21-48-master@70b0e84 (2024-07-08T19:27:11Z) (Implementing API version 1.21-R0.1-SNAPSHOT)

Error: https://mclo.gs/AE8niSU

[TechnicallyCoded]

Ah dang it, still broken in the dev 0.4.0 version :/

Paper: 1.21-48-master@70b0e84 (2024-07-08T19:27:11Z) (Implementing API version 1.21-R0.1-SNAPSHOT)

Error: https://mclo.gs/AE8niSU

Will fix today if I can

[TechnicallyCoded]

Thanks to @Loving11ish spam pinging my DMs 40 times and scaring me into order, this is now fixed. Version 0.4.2 coming out (not yet on GH, but available on the repo already) fixes this issue and revamps asyncTeleport support while we're at it.

[PikaMug]

While I'm not getting any errors on 0.4.2 the consumer task is still not executed. Here is the code I'm now testing with on Paper (also applies to runAtEntityLater):

// Fireball is launched via PlayerInteractEvent#getPlayer#launchProjectile(Fireball.class)

@EventHandler
public void onProjectileLaunch(final ProjectileLaunchEvent event) {
    final Projectile projectile = event.getEntity();
    if (!(projectile instanceof Fireball)) {
        return;
    }
    foliaLib.getScheduler().runAtEntityTimer(projectile, task -> {
        System.out.println("This task should be executed"); // does not appear in console at all
    }, () -> {
        System.out.println("But only this fallback is triggered"); // appears in console once upon launch
    }, 2L, 2L);
}

[TechnicallyCoded]

Does it work as intended on Folia? Or is it broken on both?

[PikaMug]

@TechnicallyCoded Just checked, still works on Folia. Forking to check my theory that it should be using !Entity#isDead instead of Entity#isValid but will report back with whatever I find.

[PikaMug]

@TechnicallyCoded I've confirmed that checking whether the entity isDead works as expected on both platforms. As to why the isValid check is treated differently on Folia vs Paper, I cannot say. Let me know if you'd prefer I submit a PR.

[TechnicallyCoded]

The thing is, I am pretty sure isValid checks other stuff too, not just the death state (depending on the entity). So a Projectile instance check + isDead check could be a solution for the time being. This should be very well documented with comments of course, given how random it seems. If you want to PR that, it would probs go quicker. But lmk.