Complete JavaDocs for the API #78
Conversation
The JavaDocs for the module file are consumed by the JavaDocs tool and is effectively displayed as the index for the EventBus JavaDocs.
Expanded on the documentation for BusGroup, including a non-exhaustive simple example for the class JavaDoc and expanded explanations for the methods.
JetBrains annotations include a handful of useful documentation annotations that can improve readability in the source files (ApiStatus) and IDE support (Contract).
|
I plan on being finished this evening (Eastern Daylight Time). I've created this draft PR as a way to pace myself and view the work on GitHub as I complete it. |
|
Update: In CLASSIC Jonathan style, I've bitten off slightly more than I can chew, and I am about to fall asleep as I type this. However, I am more than halfway there. As you can see from the handful of the new issues and PRs I've made, I've acquainted myself with the API of EventBus 7 quite well. I'll push what I have and continue tomorrow. Like with everything else I've been doing with the toolchain, I want these docs to be held to the highest standard to which I can provide. So far, all that's left (aside from maybe a few more example snippets in existing packages) are the |
| * <strong>{@linkplain org.jetbrains.annotations.ApiStatus.NonExtendable must not be overridden}!</strong> It uses | ||
| * internals of {@link MutableEvent} in order to determine the monitoring state of the event. If overridden, your | ||
| * event will be unable to determine if it is actually monitoring! | ||
| */ | ||
| @Contract(pure = true) |
There was a problem hiding this comment.
This method isn't pure because it can return different values based on when it is executed (false when normal priority listeners are running, true when monitoring priority listeners are running). A method is only pure if it always returns the same value when given the same arguments.
| // https://mvnrepository.com/artifact/org.jspecify/jspecify | ||
| library('jspecify-annotations', 'org.jspecify', 'jspecify') version '1.0.0' | ||
| library 'jetbrains-annotations', 'org.jetbrains', 'annotations' version '26.0.2' |
There was a problem hiding this comment.
Missing mvnrepository link comment
There was a problem hiding this comment.
Please approve #73 first.
| * @deprecated Using this method with a cancellable event bus is not recommended, as it does not capture the event's | ||
| * cancellation state. Use {@link #post(Event)} instead. | ||
| */ | ||
| @Deprecated | ||
| @Override | ||
| T fire(T event); |
There was a problem hiding this comment.
I hinted at the possibility of ValueEvent in the future when Valhalla arrives, which could support grabbing the event's cancellation state from a fire() call. There may also be situations where you post the same event in different places and only care about the cancellation state in specific places.
There was a problem hiding this comment.
Ok, then I'll remove the deprecation from this method. Given the context of J21 though, I'd like to emphasize that the cancellation state cannot currently be returned as a result of #fire at this time.
| * @apiNote Similar to {@link net.minecraftforge.eventbus.api.bus.BusGroup#dispose()}, the posting of this event is a | ||
| * destructive action that will cause its resources to be freed. <strong>It must not be used after it is | ||
| * posted!</strong> |
There was a problem hiding this comment.
iirc this is enforced at runtime to make it a no-op, so "cannot be used" is clearer to me than "must not be used", as the latter implies an exception would be thrown. It's no-op instead of exception for performance reasons.
There was a problem hiding this comment.
ah it is no-op? I must've misread the internals then. I was under the assumption that disposing of a bus group is equivalent to free() in C.
| * import net.minecraftforge.eventbus.api.event.MutableEvent; | ||
| * import net.minecraftforge.eventbus.api.listener.Priority; | ||
| * | ||
| * public class MyCustomEvent extends MutableEvent { |
There was a problem hiding this comment.
I think we should put public final class in all our examples where relevant to encourage the practice of not allowing extending where not intended.
There was a problem hiding this comment.
ok, well MutableEvent does allow extensibility without enforcing inheritance of the event. I can make an example of that somewhere else (probably in MutableEvent itself).
| * BUS.addListener(MyCustomCancellableEvent::onMyCustomEvent); | ||
| * BUS.addListener(Priority.LOW, MyCustomCancellableEvent::alsoOnMyCustomEvent); // might cancel | ||
| * BUS.addListener(Priority.LOWEST + 1, true, MyCustomCancellableEvent::alwaysCancelMyEvent); // will always cancel | ||
| * BUS.addListener(MyCustomCancellableEvent::cannotRecieveEvent); // will never be called |
There was a problem hiding this comment.
The always cancelling listener in your example here is set to run after the cannotRecieveEvent listener (always cancelling is priority lowest + 1, cannotRecieveEvent is normal priority), so it does actually get called.
This PR adds complete JavaDocs to EventBus 7. These JavaDocs include explanations for type usage, use cases for the various characteristics, and non-exhaustive example snippets.
Additionally, this PR adds JetBrains annotations. The annotations in question are not
@Nullableor@NotNull, but rather documentation annotations such as the ones inApiStatusand IDE support annotations such as@Contract.The JavaDocs have been written to be best presented within a JavaDocs website, which would theoretically be on the MinecraftForge domain (i.e.
javadocs.minecraftforge.net), but they are also included as usual in thejavadoc.jarfile built by the project.Depends on the following PRs being addressed or merged first:
BusGroup#unregister(EventListener)for single listeners #77