[WIP] Refactor and implement ChangeDataHolderEvent.ValueChange #1605
Conversation
Aaron1011
requested review from
Deamon5550,
Faithcaio and
gabizou
as code owners
| @@ -122,6 +124,11 @@ public boolean supports(Class<? extends DataManipulator<?, ?>> holderClass) { | |||
| .orElseGet(DataTransactionResult::failNoData); | |||
| } | |||
|
|
|||
| @Override | |||
| public <E> Optional<ChangeDataHolderEvent.ValueChange> offerWithEvent(Key<? extends BaseValue<E>> key, E value, Cause cause) { | |||
| throw new UnsupportedOperationException(); | |||
| @Override | ||
| public <R> Optional<R> getDefault(Key<? extends BaseValue<R>> key) { | ||
| return DataUtil.getNbtProcessor(this.getDataType(), key) | ||
| .flatMap(processor -> processor.readFrom(this.data).map(v -> v.getDefault())); |
There was a problem hiding this comment.
multi-line the nested mapping. In the event there's an exception, we want to be able to pinpoint which lambda is having an issue.
| @@ -97,6 +98,8 @@ | |||
| */ | |||
| DataTransactionResult set(DataHolder dataHolder, M manipulator, MergeFunction function); | |||
|
|
|||
| //void setWithEvent(DataHolder container, M manipulator, Cause cause); | |||
There was a problem hiding this comment.
Intended to be used later maybe? comments welcome.
| import org.spongepowered.api.data.DataTransactionResult; | ||
| import org.spongepowered.api.data.key.Key; | ||
| import org.spongepowered.api.data.manipulator.DataManipulator; | ||
| import org.spongepowered.api.data.manipulator.ImmutableDataManipulator; | ||
| import org.spongepowered.api.data.value.BaseValue; | ||
| import org.spongepowered.api.data.value.ValueContainer; | ||
| import org.spongepowered.api.data.value.immutable.ImmutableValue; | ||
| import org.spongepowered.api.event.cause.Cause; | ||
| import org.spongepowered.api.event.data.ChangeDataHolderEvent; |
| this.player = (Player) player; | ||
| } | ||
|
|
||
| @Inject(method = "addStats(Lnet/minecraft/item/ItemFood;Lnet/minecraft/item/ItemStack;)V", at = @At(value = "HEAD")) |
There was a problem hiding this comment.
This should be a redirect if you're modifying the cause stack, this way you can wrap it in a try with autocloseable frames.
| import java.util.Optional; | ||
|
|
||
| @Plugin(id = "food-change-test", authors = "Aaron1011") | ||
| public class FoodChangeTest { |
There was a problem hiding this comment.
If this is going to constantly print to the player, should make it a toggle to join in (reduces the amount of commenting when debugging implementation).
| @@ -25,6 +25,8 @@ | |||
| package org.spongepowered.test; | |||
|
|
|||
| import org.slf4j.Logger; | |||
| import org.spongepowered.api.data.key.Keys; | |||
|
Going to close this for now due to SpongePowered/SpongeAPI#1728 and #1670 which provide a method of implementing and listening to the event. If you'd like to make other changes, open a new PR with those in relation to what is now in the API and implementation. |
SpongeAPI | SpongeCommon | SpongeForge
See the SpongeAPI PR for more details.
This is the basic pattern for firing a
ChangeDataHolderEvent.ValueChangeevent for a given key:DataParameter, etc), callofferWithEventon the appropriateDataHolder, putting any relevant objects into theCauseValueProcessorfor the key in question, bypass the setter you redirected before, if applicable. This ensures that setting the value does not end up recursing infinitely between your redirected setter and the value processor.Basically, the idea is to ensure that all changes done by mods/vanilla/forge end up firing an event, while API changes (e.g. anything going through the value processor) does not. The decision as to whether or not to fire an event for a plugin change is handled on the API side, and is not the implementation's concern.