Open EpicPlayerA10 opened 4 months ago
Your Pull Request was automatically labelled as: "✨ Fix" Thank you for contributing to this project! ❤️
I’ll try and test and review this today, long standing bug but if it’s being called async that would make sense as to why it does that weird behavior, thanks for the PR!
A Slimefun preview build is available for testing! Commit: dd164ef3
https://preview-builds.walshy.dev/download/Slimefun/4197/dd164ef3
Note: This is not a supported build and is only here for the purposes of testing. Do not run this on a live server and do not report bugs anywhere but this PR!
Only other thing that may or may not be a problem is the removal of the exception catching and therein logging
The error will be logged by java anyway because the thrown exception from getHologram
will be propagated to the stacktrace. I think it is better to throw exception and stop execution of further code where getHologram
was called, than only printing error in the console and allowing further code execution while creating undefined behaviour.
Description
Fixes not disappearing holograms caused by delayed hologram placement when called from async ( https://github.com/Slimefun/Slimefun4/issues/3176#issuecomment-1025863596 )
Proposed changes
HologramsService#updateHologram
to allows calls only from sync code. Calls from async code should not be allowed as this on its own causes undefined behaviour. For example theHologramsService#removeHologram
function already does not allow async.HologramOwner#updateHologram
to allow calls from async code. ChecksisDeletedSoon
to be sure that the hologram owner still exists.Related Issues (if applicable)
3176
Checklist
Nonnull
andNullable
annotations to my methods to indicate their behaviour for null values