Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ClientTicker.addAction is broken #34

Open
Johni0702 opened this issue Oct 31, 2019 · 0 comments
Open

ClientTicker.addAction is broken #34

Johni0702 opened this issue Oct 31, 2019 · 0 comments

Comments

@Johni0702
Copy link

Re-posting this as a separate issue since it seems to have been forgotten (ref).
This may or may not now be causing Johni0702/BetterPortals#395 (might be a mod re-submitting tasks in its handler or a broken ArrayDeque due to thread-unsafe access. at least in quark there doesn't appear to be the former).

wrt 34d2080:

This seems more like a workaround than a fix.
Why is the Runnable null in the first place? Is that expected behavior?
It looks like to me that passing null to addAction can never serve any purpose. So having addAction throw a NPE and then fixing the caller sounds like a better solution.

Actually, upon looking at Quark, I realized that this isn't ever getting called with null (might still want to add the check for good measure) but instead it's getting called from other threads and pendingActions is an ArrayDeque which isn't thread-safe.
So this is definitely not the right way to fix it.

While on the topic:
Why does this even exist in the first place? I.e. why not just use Minecraft.addScheduledTask?
As is this will additionally cause compatibility issues with BetterPortals because BP hooks Minecraft.addScheduledTask to determine which world a particular runnable belongs to and will then make sure it's run in that context. It doesn't hook the pendingActions list so everything will just be ran in the main world.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant