Asynchronous block remove

[LadyCailinBot]

CMDHELPER-3118 - Reported by EntryPoint

commandhelper generate error when i wrote set_block_at() in x_new_thread() look at this screenshot

code: http://pastebin.com/KXQXmFGa

error log: http://pastebin.com/V4NqpZRi

[LadyCailinBot]

Comment by PseudoKnight

This is a known risk when using threading. Only certain functions are threadsafe and almost none of the Minecraft ones are. The solution is to NEVER put set_block_at() inside a new thread. And in general do not use threads if you don't know the risks.

[LadyCailinBot]

Comment by LadyCailin

You can, however, use x_run_on_mainthread. That will fix the problem. This stacktrace shouldn't be printing like this, but x functions aren't really supported, so, that's ok. Either way, all I would be able to do is improve the error message. In this case, you need to change your code.

[LadyCailinBot]

Comment by PseudoKnight

Other async block placer plugins basically use a separate thread to calculate how many and which blocks they want to set every tick. Then they send that truncated list to the main thread (eg. x_run_on_main_thread) to set the blocks. They often disable block physics updates when setting the blocks, as that accounts for something like half of the cost of placing blocks.

[Pieter12345]

Is this issue still meaningful? The set_block_at() function has been deprecated since MC 1.13 and async calls to Bukkit related functions is API violation anyways. I'm guessing that the bug here is that it causes an error in core in CH, but I'm not sure whether this should be prevented or left as a user responsibility.

[PseudoKnight]

I have a proposal and a working prototype. We can use optimizeDynamic() in x_new_thread() to add warnings when using functions that return false from runAsync(). It traverses the children unless encountering a closure/iclosure. Works pretty well, but some functions have their runAsync() method configured incorrectly, so we'd have to make sure all false returns are accurate, at the very least.

There's a few other places where it should stop checking, like proc definitions and binds. This won't catch all cases where a function may be used off the main thread, but will help. (could use SA for better coverage?) Perhaps it might give people a misleading sense of confidence, though. It also doesn't catch run time problems, which are left up to the server.