-
-
Notifications
You must be signed in to change notification settings - Fork 129
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
[1.20.5] Remove uses of Event.Result
and update events to have their own Result classes
#588
Conversation
Last commit published: 84838162c2031b1ddcd2fbb11504245e6b1e6518. PR PublishingThe artifacts published by this PR:
Repository DeclarationIn order to use the artifacts published by the PR, add the following repository to your buildscript: repositories {
maven {
name 'Maven for PR #588' // https://github.com/neoforged/NeoForge/pull/588
url 'https://prmaven.neoforged.net/NeoForge/pr588'
content {
includeModule('net.neoforged', 'testframework')
includeModule('net.neoforged', 'neoforge')
}
}
} MDK installationIn order to setup a MDK using the latest PR version, run the following commands in a terminal. mkdir NeoForge-pr588
cd NeoForge-pr588
curl -L https://prmaven.neoforged.net/NeoForge/pr588/net/neoforged/neoforge/20.6.108-beta-pr-588-rem-result/mdk-pr588.zip -o mdk.zip
jar xf mdk.zip
rm mdk.zip || del mdk.zip To test a production environment, you can download the installer from here. |
8100056
to
f881ee0
Compare
5d3ebfa
to
ea50771
Compare
Event.Result
Event.Result
and update events to have their own Result classes
5240b78
to
c4db42d
Compare
Still 92 compile errors. My original list is missing a few entries. |
Remaining events:
|
eb723ae
to
57d55d6
Compare
Minor refactor, merging EntityItemPickupEvent and PlayerEvent$ItemPickupEvent. These events are actually two phases of the same operation, and are now joined as a Pre/Post event.
This event is just legacy code, it was superseded by RightClickItem when that was introduced.
14839ea
to
9f66784
Compare
Also reverts a name change for CriticalHitEvent
What
This PR is a work-in-progress for removing all uses of
Event.Result
from existing events. Changes to an individual event will be grouped into a single commit until the first pass is completed.Why
Uses of the abstracted
Event.Result
enum induces the following readability issues:ALLOW
/DEFAULT
/DENY
for any result-based event, even when this is not a very descriptive name.@HasResult
annotation are, and it is unexpected that thegetResult
/setResult
methods exist on all children ofEvent
.a. This is the same reason why
@Cancellable
was removed in Replace@Cancelable
byICancellableEvent
interface & related changes Bus#20There are also technical challenges with continuing to support the annotation-based model, since it means we must run a transformer to inject real implementations of
getResult
/setResult
based on the presence of the annotation.Progress
The following events need to be updated:
MouseButtonPressed.Post
MouseButtonReleased.Post
renamed toLivingPackSizeEvent
SpawnClusterSizeEvent
MobEffectEvent.Applicable
(Fixed a bug where this wouldn't fire for withers)MobSpawnEvent.SpawnPlacementCheck
MobSpawnEvent.PositionCheck
(Fixed a bug where theBaseSpawner
wasn't getting passed through when applicable)renamed toMobSpawnEvent.AllowDespawn
MobDespawnEvent
removed. Supplanted by other events.ZombieEvent.SummonAidEvent
CriticalHitEvent
(This and the below event have been fused toSleepingLocationCheckEvent
CanContinueSleepingEvent
)SleepingTimeCheckEvent
EntityMobGriefingEvent
BlockEvent.CreateFluidSourceEvent
CropGrowEvent.Pre
renamed toSaplingGrowTreeEvent
BlockGrowFeatureEvent
Notes
Depends on neoforged/Bus#30 - this PR will not build locally or in CI unless using a version of Bus with this PR merged.