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.

Sign up for GitHub

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

Jump to bottom

Added Villager Trades and basic movement #1263

Merged

SuperIceCN merged 11 commits into PowerNukkitX:master from Syodo-Network:master

Jul 13, 2023

Member

Buddelbubi commented Jun 26, 2023

No description provided.

Buddelbubi added 2 commits

June 26, 2023 12:51


          Added Villager Trades and basic movement

e2a49e3


          Added Villager Trades and basic movement

ff7e8a4

Fixed OutOfBound error in

Member

smartcmd commented Jun 26, 2023

Cool ❤️

smartcmd requested review from CoolLoong and SuperIceCN and removed request for CoolLoong

June 26, 2023 12:17

SuperIceCN requested a review from smartcmd

June 27, 2023 02:28

SuperIceCN requested changes

View reviewed changes

Member

SuperIceCN left a comment •

edited

Loading

A Problem is that Profession.class does not exist.

Also, please format your code:

Remove the unused imports
There should be space between expression tokens, e.g. tick % (10 * 10) == 0
There should be space between if/for/while, conditions and code blocks
No empty lines at the start and the end of methods.

Anyway, thank you very much for your contribution to PNX!

pom.xml Outdated

+                      <dependency>
+                          <groupId>org.projectlombok</groupId>
+                          <artifactId>lombok</artifactId>
+                          <version>1.18.22</version>

Member

SuperIceCN Jun 27, 2023

Why do we need to roll back the version from 1.18.24 to 1.18.22?

Member Author

Buddelbubi Jun 27, 2023

IntelliJ did that

Member Author

Buddelbubi Jun 27, 2023

You dont

src/main/java/cn/nukkit/entity/passive/EntityVillager.java Outdated

+                              ),
+                              Set.of(
+                                      new Behavior(new FlatRandomRoamExecutor(0.4f, 12, 40, true, 100, true, 10), new PassByTimeEvaluator(CoreMemoryTypes.LAST_BE_ATTACKED_TIME, 0, 100), 4, 1),
+                                      new Behavior(new EntityBreedingExecutor<>(EntityPig.class, 16, 100, 0.5f), entity -> entity.getMemoryStorage().get(CoreMemoryTypes.IS_IN_LOVE), 3, 1),

Member

SuperIceCN Jun 27, 2023

Why do we need EntityBreedingExecutor with EntityPig.class? Can we feed the villagers?

Member Author

Buddelbubi Jun 27, 2023

Uh. I focused on the functionality. I probably missed that

src/main/java/cn/nukkit/entity/passive/EntityVillager.java Outdated

+                                      new Behavior(new LookAtTargetExecutor(CoreMemoryTypes.NEAREST_PLAYER, 100), new ProbabilityEvaluator(4, 10), 1, 1, 100),
+                                      new Behavior(new FlatRandomRoamExecutor(0.2f, 12, 100, false, -1, true, 10), (entity -> true), 1, 1)
+                              ),
+                              Set.of(new NearestFeedingPlayerSensor(8, 0), new NearestPlayerSensor(8, 0, 20)),

Member

SuperIceCN Jun 27, 2023

If we use NearestFeedingPlayerSensor, the Entity must extends EntityAnimal. However it's not the case for villager now.

src/main/java/cn/nukkit/entity/passive/EntityVillager.java Outdated

+                                                  this.setTradeSeed(new Random().nextInt(Integer.MAX_VALUE));
+                                                  this.setProfession(profession.getIndex());
+                                                  this.applyProfession(profession);
+                                                  BlockVector3 blockVector = block.getLocation().asBlockVector3();

Member

SuperIceCN Jun 27, 2023

Using block.getFloorX/Y/Z instead.

src/main/java/cn/nukkit/entity/passive/EntityVillager.java Outdated

+                                          if (id == profession.getBlockid()) {
+                                              professionFound = true;
+                                              if (this.profession != profession.getIndex()) {
+                                                  this.setTradeSeed(new Random().nextInt(Integer.MAX_VALUE));

Member

SuperIceCN Jun 27, 2023

Using ThreadLocalRandom or NukkitRandom from the level of this villager instead, Random is globally synchronized and may block PNX, thereby affecting performance.

CoolLoong requested changes

View reviewed changes

pom.xml Show resolved Hide resolved

src/main/java/cn/nukkit/Server.java Show resolved Hide resolved

src/main/java/cn/nukkit/entity/passive/EntityVillager.java Outdated

                   public static final int NETWORK_ID = 115;
                   /**
                    * 代表交易配方
                    */
-                  public final ListTag<Tag> recipes = new ListTag<>("Recipes");
+                  @Getter
+                  public ListTag<Tag> recipes = new ListTag<>("Recipes");

Member

CoolLoong Jun 27, 2023

change to protected

src/main/java/cn/nukkit/entity/passive/EntityVillager.java Outdated

+                      return new BehaviorGroup(
+                              this.tickSpread,
+                              Set.of(
+                                      //用于刷新InLove状态的核心行为

Member

CoolLoong Jun 27, 2023

These behaviors do not match the villagers, please remove them

src/main/java/cn/nukkit/entity/passive/EntityVillager.java Outdated

+                                              //todo：Growth rate
+                                              all(
+                                                      new PassByTimeEvaluator(CoreMemoryTypes.ENTITY_SPAWN_TIME, 20 * 60 * 20, Integer.MAX_VALUE),
+                                                      entity -> entity instanceof EntityAnimal animal && animal.isBaby()

Member

CoolLoong Jun 27, 2023

please remove this

src/main/java/cn/nukkit/entity/passive/EntityVillager.java Outdated

+                              ),
+                              Set.of(
+                                      new Behavior(new FlatRandomRoamExecutor(0.4f, 12, 40, true, 100, true, 10), new PassByTimeEvaluator(CoreMemoryTypes.LAST_BE_ATTACKED_TIME, 0, 100), 4, 1),
+                                      new Behavior(new EntityBreedingExecutor<>(EntityPig.class, 16, 100, 0.5f), entity -> entity.getMemoryStorage().get(CoreMemoryTypes.IS_IN_LOVE), 3, 1),

Member

CoolLoong Jun 27, 2023

please remove this

src/main/java/cn/nukkit/network/process/processor/InventoryTransactionProcessor.java Outdated

                           }
                           if (playerHandle.getTradingTransaction().canExecute()) {
                               playerHandle.getTradingTransaction().execute();
+                              for(Inventory inventory : playerHandle.getTradingTransaction().getInventories()) {

Member

CoolLoong Jun 27, 2023

Suggest moving to TradeTransaction#execute

Member

CoolLoong commented Jul 3, 2023

@Buddelbubi You did not commit cn.nukkit.entity.data.profession.Profession

Buddelbubi added 5 commits

July 8, 2023 13:14


          Merge branch 'PowerNukkitX:master' into master

13e7664


          Did requested changes

402f639


          Oups. Actually committing...

8509ca8


          And again.. This program undid all my changes...

26e67ef


          Added Profession classes

e614078

Member

SuperIceCN commented Jul 9, 2023

May I ask if you have time to resolve the problems in this PR recently?

Member Author

Buddelbubi commented Jul 10, 2023

I already did. Did I miss some?


          Merge branch 'PowerNukkitX:master' into master

6208e3a

Mcayear requested review from CoolLoong and SuperIceCN

July 12, 2023 02:39


          Refactored code

9477bff

Member

CoolLoong commented Jul 12, 2023

thank you for your PR

CoolLoong requested changes

View reviewed changes

src/main/java/cn/nukkit/network/protocol/ProtocolInfo.java Outdated

@@ @@ -18,7 +18,7 @@ public interface ProtocolInfo { @@
                   /**
                    * Actual Minecraft: PE protocol version
                    */
-                  int CURRENT_PROTOCOL = dynamic(589);
+                  int CURRENT_PROTOCOL = dynamic(594);

Member

CoolLoong Jul 12, 2023

please wait #1312

Member

CoolLoong commented Jul 12, 2023

Other than that, there are no issue


          reverted protocol version

15891e5

Member Author

Buddelbubi commented Jul 12, 2023

Done

CoolLoong approved these changes

View reviewed changes

Member

CoolLoong commented Jul 12, 2023

smartcmd approved these changes

View reviewed changes


          changed lombok to 1.18.24 and provided

adad53e

SuperIceCN approved these changes

View reviewed changes

SuperIceCN merged commit 1432647 into PowerNukkitX:master

SuperIceCN mentioned this pull request

Release 1.20.10-r1 #1319

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet