CraftAttack/spawn
5
0
Fork 0
Code Issues 3 Pull Requests Packages Projects Releases Wiki Activity

master-customAdvancements #4

Merged
MineTec merged 3 commits from master-customAdvancements into master 2024-10-05 16:40:52 +00:00
Conversation 8 Commits 3 Files Changed 8 +10 -10
2 changed files with 10 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit e3b07aa62f - Show all commits

9
src/main/java/eu/mhsl/craftattack/spawn/appliances/customAdvancements/CustomAdvancements.java
View File

@ -30,11 +30,12 @@ public class CustomAdvancements extends Appliance {
} }
try { try {
NamespacedKey namespacedKey = Objects.requireNonNull(NamespacedKey.fromString("custom_advancements:craftattack/" + advancementName)); NamespacedKey namespacedKey = Objects.requireNonNull(NamespacedKey.fromString("custom_advancements:craftattack/" + advancementName), "NamespacedKey is invalid!");
Advancement advancement = Objects.requireNonNull(Bukkit.getAdvancement(namespacedKey)); Advancement advancement = Objects.requireNonNull(Bukkit.getAdvancement(namespacedKey), "The advancement does not exist!");
player.getAdvancementProgress(advancement).awardCriteria("criteria"); player.getAdvancementProgress(advancement).awardCriteria("criteria");
} catch (Exception e) { } catch (Exception e) {
Main.logger().info("Custom Advancements Datapack not found!"); Main.logger().info("Advancement " + advancementName + " not found! (is Custom Advancements data pack loaded?)");
Pupsi marked this conversation as resolved Outdated
MineTec commented 2024-09-29 16:52:19 +00:00
Outdated
Review
Copy Link

die Fehlermeldung kann irreführend sein. Es sollte zumindest im Fehler ersichtlich sein welches advancement fehlt.

die Fehlermeldung kann irreführend sein. Es sollte zumindest im Fehler ersichtlich sein welches advancement fehlt.
MineTec commented 2024-09-29 17:05:45 +00:00
Outdated
Review
Copy Link

mein Vorschlag hier wäre du throwst nach dem custom fehler einfach nochmal die exception throw e und packst in die Objects.requireNonNull das angefragte rein. Dann ist alles immer ersichtlich.

mein Vorschlag hier wäre du throwst nach dem custom fehler einfach nochmal die exception `throw e` und packst in die Objects.requireNonNull das angefragte rein. Dann ist alles immer ersichtlich.
throw e;
} }
} }
@ -46,7 +47,7 @@ public class CustomAdvancements extends Appliance {
grantedAdvancements.forEach(pendingAdvancement -> grantAdvancement(pendingAdvancement.advancement(), player.getUniqueId())); grantedAdvancements.forEach(pendingAdvancement -> grantAdvancement(pendingAdvancement.advancement(), player.getUniqueId()));
} }
Pupsi marked this conversation as resolved
MineTec commented 2024-09-29 16:53:08 +00:00
Review
Copy Link

sollte private sein, wenn grantAdvancement entscheidet ob es pending ist oder nicht

sollte private sein, wenn grantAdvancement entscheidet ob es pending ist oder nicht
public void addPendingAdvancement(UUID receiver, String advancement) { private void addPendingAdvancement(UUID receiver, String advancement) {
pendingAdvancements.add(new PendingAdvancement(receiver, advancement)); pendingAdvancements.add(new PendingAdvancement(receiver, advancement));
} }

11
src/main/java/eu/mhsl/craftattack/spawn/appliances/customAdvancements/listener/CustomAdvancementsListener.java
View File

@ -30,12 +30,11 @@ public class CustomAdvancementsListener extends ApplianceListener<CustomAdvancem
if(!(event.getView().getPlayer() instanceof Player player)) return; if(!(event.getView().getPlayer() instanceof Player player)) return;
if(result.getType() == Material.RED_SHULKER_BOX) { if(result.getType() == Material.RED_SHULKER_BOX) {
// getAppliance().grantAdvancement(Advancements.fleischerchest, player.getUniqueId()); getAppliance().grantAdvancement(Advancements.fleischerchest, player.getUniqueId());
Pupsi marked this conversation as resolved Outdated
MineTec commented 2024-09-29 16:49:45 +00:00
Outdated
Review
Copy Link

?

?
getAppliance().addPendingAdvancement(player.getUniqueId(), Advancements.fleischerchest); return;
} else if( }
Pupsi marked this conversation as resolved Outdated
MineTec commented 2024-09-29 16:57:35 +00:00
Outdated
Review
Copy Link

das else if konstrukt ist nicht schön...

ich tendiere sogar für jeweils einen eigenen handler pro advancement...

vielleicht einfach zwei einzelstehnede ifs mit quasi early return?

das else if konstrukt ist nicht schön... ich tendiere sogar für jeweils einen eigenen handler pro advancement... vielleicht einfach zwei einzelstehnede ifs mit quasi early return?
result.getItemMeta().itemName().equals(Component.text("98fdf0ae-c3ab-4ef7-ae25-efd518d600de"))
&& result.getItemMeta().getEnchantmentGlintOverride() if(result.getItemMeta().itemName().equals(Component.text("98fdf0ae-c3ab-4ef7-ae25-efd518d600de"))) {
Pupsi marked this conversation as resolved Outdated
MineTec commented 2024-09-29 16:56:36 +00:00
Outdated
Review
Copy Link

der check nach enchantmentGlintOverride raus, itemName reicht aus

(ggf kommt das glint raus, da zurzeit ein bugs mit overrideGlint auf köpfen besteht)

der check nach enchantmentGlintOverride raus, itemName reicht aus (ggf kommt das glint raus, da zurzeit ein bugs mit overrideGlint auf köpfen besteht)
) {
getAppliance().grantAdvancement(Advancements.craftPixelblock, player.getUniqueId()); getAppliance().grantAdvancement(Advancements.craftPixelblock, player.getUniqueId());
} }
} }