Add new 1.14 Merchant, Raid and Villager related features. #2068 #2069 #2073

i509VCB · 2019-12-18T12:34:52Z

This adds some new events (#2068):

RingBellEvent (Needs to have a way to get a list of all villagers which will hear the bell ring).

VillagerEvent.ProfessionChange
VillagerEvent.ProfessionLevelUp
VillagerEvent.Panic

TradeMerchantEvent

RaidEvent.Start
RaidEvent.StartWave
RaidEvent.End

Other additions:

Adds a new Key to get a Villager's Profession level.

Add a way to reference all the Raids within a world, inside of ServerWorld. (#2069)

src/main/java/org/spongepowered/api/event/entity/living/trader/VillagerEvent.java

src/main/java/org/spongepowered/api/event/item/merchant/TradeMerchantEvent.java

src/main/java/org/spongepowered/api/event/entity/living/trader/VillagerEvent.java

Zidane · 2019-12-19T03:14:47Z

src/main/java/org/spongepowered/api/event/entity/living/trader/VillagerEvent.java

+		 * @return The {@link Profession} level.
+		 */
+		default int getCurrentProfessionLevel() {
+			this.getVillager().professionLevel().get();


This needs to be passed to the event factory in the impl, never default and point to the source like this.

Right, and especially since a plugin can change the profession level value between event listeners.

Mind you that this would technically be possible to be part of ChangeValueEvent as a sub class LevelUpProfession that is based on the Keys.PROFESSION_LEVEL. I'll be making adjustments to event-impl-gen to allow annotating Keys fields to generate an event based on it.

src/main/java/org/spongepowered/api/event/raid/RaidEvent.java

src/main/java/org/spongepowered/api/world/server/ServerWorld.java

gabizou

Well done, only went through functional aspects of the PR, will do another review of documentation a bit later, but a cursory look was fine.

gabizou · 2019-12-19T03:21:36Z

src/main/java/org/spongepowered/api/event/block/entity/RingBellEvent.java

+	 *
+	 * @return A list of Entities that will get the glowing effect.
+	 */
+	List<Entity> entitiesToGlow();


This might be better to mimic AffectEntityEvent where there's the immutable list of entities originally, versus the now mutable list that'll be used.

gabizou · 2019-12-19T03:22:27Z

src/main/java/org/spongepowered/api/event/entity/living/trader/VillagerEvent.java

+		 * @return The {@link Profession} level.
+		 */
+		default int getCurrentProfessionLevel() {
+			this.getVillager().professionLevel().get();


Right, and especially since a plugin can change the profession level value between event listeners.

gabizou · 2019-12-19T03:24:16Z

src/main/java/org/spongepowered/api/event/entity/living/trader/VillagerEvent.java

+		 * @return The {@link Profession} level.
+		 */
+		default int getCurrentProfessionLevel() {
+			this.getVillager().professionLevel().get();


Mind you that this would technically be possible to be part of ChangeValueEvent as a sub class LevelUpProfession that is based on the Keys.PROFESSION_LEVEL. I'll be making adjustments to event-impl-gen to allow annotating Keys fields to generate an event based on it.

src/main/java/org/spongepowered/api/event/entity/living/trader/VillagerEvent.java

src/main/java/org/spongepowered/api/event/item/merchant/TradeMerchantEvent.java

src/main/java/org/spongepowered/api/world/server/ServerWorld.java

ST-DDT · 2019-12-19T23:27:30Z

Do not force push your changes as it complicates the review process.

i509VCB · 2019-12-19T23:37:34Z

Do not force push your changes as it complicates the review process.

Will note that.

gabizou

Run a licenseFormat and it looks good to me.

src/main/java/org/spongepowered/api/raid/Wave.java

Zidane · 2019-12-29T18:27:31Z

src/main/java/org/spongepowered/api/event/item/merchant/TradeWithMerchantEvent.java

+	 */
+	Merchant getMerchant();
+
+	/**


It would be nice to have the original TradeOffer here.

I'm not sure whether the TradeOffer should be settable in this event.

@ST-DDT Would you care to share with the class why?

It's hard to explain. I dont see a reason for that. You are in the middle of the trade. Why would the trade change at this point. If this were something like SelectTradeOfferEvent it would make sense, but not while actually doing it.

Edit: Is the server expected to verify that the requirements for the old or the new trade offer is met? What happens if the player no longer meets the requirement will the event be cancelled afterwards? If not how will the missing resources be withdrawn?

Considering that case you mentioned, I am not too sure if you can make the client forcefully select a TradeOffer. Maybe it would be best to make the TradeOffer not settable during the event.

src/main/java/org/spongepowered/api/event/entity/living/trader/VillagerEvent.java

src/main/java/org/spongepowered/api/event/item/merchant/TradeWithMerchantEvent.java

src/main/java/org/spongepowered/api/world/server/ServerWorld.java

ST-DDT · 2020-01-02T08:45:08Z

Only these last minor issues otherwise it looks good to me.

src/main/java/org/spongepowered/api/event/entity/living/trader/VillagerEvent.java

ST-DDT reviewed Dec 18, 2019

View reviewed changes

src/main/java/org/spongepowered/api/event/entity/living/trader/VillagerEvent.java Outdated Show resolved Hide resolved

gabizou changed the title ~~[DO NOT MERGE ME] [Draft] Add new 1.14 Merchant, Raid and Villager related features. #2068 #2069~~ Add new 1.14 Merchant, Raid and Villager related features. #2068 #2069 Dec 18, 2019

i509VCB force-pushed the feature/new_114_events branch 2 times, most recently from 589345b to 699e3a1 Compare December 19, 2019 03:05

i509VCB marked this pull request as ready for review December 19, 2019 03:09

i509VCB requested a review from gabizou as a code owner December 19, 2019 03:09

Zidane reviewed Dec 19, 2019

View reviewed changes

src/main/java/org/spongepowered/api/event/item/merchant/TradeMerchantEvent.java Outdated Show resolved Hide resolved

Zidane reviewed Dec 19, 2019

View reviewed changes

src/main/java/org/spongepowered/api/event/entity/living/trader/VillagerEvent.java Show resolved Hide resolved

Zidane reviewed Dec 19, 2019

View reviewed changes

src/main/java/org/spongepowered/api/event/raid/RaidEvent.java Outdated Show resolved Hide resolved

Zidane reviewed Dec 19, 2019

View reviewed changes

src/main/java/org/spongepowered/api/world/server/ServerWorld.java Outdated Show resolved Hide resolved

gabizou requested changes Dec 19, 2019

View reviewed changes

i509VCB force-pushed the feature/new_114_events branch 2 times, most recently from 0270d0d to 1932c3d Compare December 19, 2019 21:13

gabizou requested changes Dec 27, 2019

View reviewed changes

i509VCB force-pushed the feature/new_114_events branch from 1932c3d to 84c6f6c Compare December 29, 2019 04:44

ST-DDT reviewed Dec 29, 2019

View reviewed changes

src/main/java/org/spongepowered/api/raid/Wave.java Outdated Show resolved Hide resolved

Zidane reviewed Dec 29, 2019

View reviewed changes

src/main/java/org/spongepowered/api/event/entity/living/trader/VillagerEvent.java Outdated Show resolved Hide resolved

i509VCB force-pushed the feature/new_114_events branch from 84c6f6c to 0209444 Compare January 2, 2020 07:12

ST-DDT reviewed Jan 2, 2020

View reviewed changes

src/main/java/org/spongepowered/api/event/item/merchant/TradeWithMerchantEvent.java Outdated Show resolved Hide resolved

src/main/java/org/spongepowered/api/world/server/ServerWorld.java Outdated Show resolved Hide resolved

Zidane reviewed Jan 2, 2020

View reviewed changes

src/main/java/org/spongepowered/api/event/entity/living/trader/VillagerEvent.java Show resolved Hide resolved

Zidane reviewed Jan 2, 2020

View reviewed changes

src/main/java/org/spongepowered/api/event/entity/living/trader/VillagerEvent.java Outdated Show resolved Hide resolved

Add new 1.14 Events related to Raids, Merchants and Villagers.

ad726f9

i509VCB force-pushed the feature/new_114_events branch from 0209444 to ad726f9 Compare January 2, 2020 17:21

Zidane merged commit 3d691e8 into SpongePowered:api-8 Jan 2, 2020

i509VCB mentioned this pull request Jan 2, 2020

[1.14] Add a mechanism to get raids which are in the ServerWorld #2069

Closed

i509VCB mentioned this pull request Jan 2, 2020

[1.14] New Villager and Mechant related Events #2068

Closed

renovate bot mentioned this pull request Mar 4, 2022

chore(deps): Update dependency org.spongepowered:spongeapi to v8 - autoclosed KyoriPowered/adventure-platform#85

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new 1.14 Merchant, Raid and Villager related features. #2068 #2069 #2073

Add new 1.14 Merchant, Raid and Villager related features. #2068 #2069 #2073

i509VCB commented Dec 18, 2019 •

edited

Loading

Zidane Dec 19, 2019

gabizou Dec 19, 2019

gabizou Dec 19, 2019

gabizou left a comment

gabizou Dec 19, 2019

gabizou Dec 19, 2019

gabizou Dec 19, 2019

ST-DDT commented Dec 19, 2019

i509VCB commented Dec 19, 2019

gabizou left a comment

Zidane Dec 29, 2019

ST-DDT Dec 29, 2019

Zidane Dec 29, 2019

ST-DDT Dec 29, 2019 •

edited

Loading

i509VCB Dec 29, 2019

ST-DDT commented Jan 2, 2020 •

edited

Loading

Add new 1.14 Merchant, Raid and Villager related features. #2068 #2069 #2073

Add new 1.14 Merchant, Raid and Villager related features. #2068 #2069 #2073

Conversation

i509VCB commented Dec 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gabizou left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ST-DDT commented Dec 19, 2019

i509VCB commented Dec 19, 2019

gabizou left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ST-DDT Dec 29, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ST-DDT commented Jan 2, 2020 • edited Loading

i509VCB commented Dec 18, 2019 •

edited

Loading

ST-DDT Dec 29, 2019 •

edited

Loading

ST-DDT commented Jan 2, 2020 •

edited

Loading